Attachment 349575 Details for Bug 189318 – FIX

[patch] FIX

189318.diff (text/plain), 17.43 KB, created by Basuke Suzuki on 2018-09-12 13:15:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-09-12 13:15:59 PDT

Size: 17.43 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 608c3d645bf..bf267e3c1f3 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2018-09-11  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl] Respond with requested authentication scheme for authentication challenge.
>+        https://bugs.webkit.org/show_bug.cgi?id=189318
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/wincairo/TestExpectations:
>+
> 2018-09-10  Per Arne Vollan  <pvollan@apple.com>
> 
>         [Windows] Layout Test webanimations/accelerated-transition-interrupted-on-composited-element.html is failing
>diff --git a/LayoutTests/platform/wincairo/TestExpectations b/LayoutTests/platform/wincairo/TestExpectations
>index 775149fc910..f73c640be15 100644
>--- a/LayoutTests/platform/wincairo/TestExpectations
>+++ b/LayoutTests/platform/wincairo/TestExpectations
>@@ -915,10 +915,7 @@ http/tests/websocket/tests/hybi/contentextensions [ Failure ]
> http/tests/websocket/tests/hybi/contentextensions/block-cookies.php [ Pass Failure ]
> http/tests/websocket/tests/hybi/contentextensions/block-cookies-worker.php [ Pass Failure ]
> http/tests/websocket/tests/hybi/deflate-frame-parameter.html [ Failure ]
>-http/tests/websocket/tests/hybi/httponly-cookie.pl [ Failure ]
> http/tests/websocket/tests/hybi/inspector [ Skip ]
>-http/tests/websocket/tests/hybi/secure-cookie-insecure-connection.pl [ Failure ]
>-http/tests/websocket/tests/hybi/secure-cookie-secure-connection.pl [ Failure ]
> http/tests/websocket/tests/hybi/upgrade-simple-ws.html [ Pass Failure ]
> http/tests/websocket/tests/hybi/websocket-allowed-setting-cookie-as-third-party.html [ Pass Failure ]
> http/tests/websocket/tests/hybi/websocket-blocked-from-setting-cookie-as-third-party.html [ Failure ]
>@@ -938,23 +935,15 @@ http/tests/xmlhttprequest/access-control-preflight-credential-async.html [ Failu
> http/tests/xmlhttprequest/access-control-preflight-credential-sync.html [ Failure ]
> http/tests/xmlhttprequest/access-control-preflight-not-successful.html [ Failure ]
> http/tests/xmlhttprequest/auth-reject-protection-space.html [ Failure ]
>-http/tests/xmlhttprequest/basic-auth-default.html [ Failure ]
> http/tests/xmlhttprequest/basic-auth-nopassword.html [ Failure ]
> http/tests/xmlhttprequest/basic-auth-nouser.html [ Failure ]
> http/tests/xmlhttprequest/cross-origin-authorization-with-embedder.html [ Failure ]
>-http/tests/xmlhttprequest/cross-origin-authorization.html [ Failure ]
> http/tests/xmlhttprequest/cross-origin-cookie-storage.html [ Failure ]
> http/tests/xmlhttprequest/cross-origin-no-authorization.html [ Failure ]
>-http/tests/xmlhttprequest/logout.html [ Failure ]
>-http/tests/xmlhttprequest/null-auth.php [ Failure ]
>-http/tests/xmlhttprequest/re-login-async.html [ Failure ]
>-http/tests/xmlhttprequest/re-login.html [ Failure ]
>-http/tests/xmlhttprequest/redirect-credentials-responseURL.html [ Failure ]
> http/tests/xmlhttprequest/redirect-cross-origin-post-sync.html [ Failure ]
> http/tests/xmlhttprequest/redirect-cross-origin-sync.html [ Failure ]
> http/tests/xmlhttprequest/redirect-cross-origin-tripmine.html [ Failure ]
> http/tests/xmlhttprequest/redirections-and-user-headers.html [ Failure ]
>-http/tests/xmlhttprequest/remember-bad-password.html [ Failure ]
> http/tests/xmlhttprequest/response-access-on-error.html [ Failure ]
> http/tests/xmlhttprequest/upload-onload-event.html [ Failure ]
> http/tests/xmlhttprequest/upload-onloadend-event-after-load.html [ Failure ]
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 593804b6fda..6aaa9cccb58 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,47 @@
>+2018-09-11  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl] Respond with requested authentication scheme for authentication challenge.
>+        https://bugs.webkit.org/show_bug.cgi?id=189318
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Curl port depends on libcurl's authentication handling by enabling CURLAUTH_ANY. With this
>+        mode, the round-trip communication between the client and the server is handled by libcurl
>+        internally. That's okay for many cases. But when initial request has a credentials
>+        (i.e. XMLHttpRequest), there's no valid chance to store credential to the storage because
>+        the returned response is not 401.
>+
>+        Passes following tests:
>+        - http/tests/websocket/tests/hybi/httponly-cookie.pl [ Failure ]
>+        - http/tests/websocket/tests/hybi/secure-cookie-insecure-connection.pl [ Failure ]
>+        - http/tests/websocket/tests/hybi/secure-cookie-secure-connection.pl [ Failure ]
>+        - http/tests/xmlhttprequest/basic-auth-default.html [ Failure ]
>+        - http/tests/xmlhttprequest/cross-origin-authorization.html [ Failure ]
>+        - http/tests/xmlhttprequest/logout.html [ Failure ]
>+        - http/tests/xmlhttprequest/null-auth.php [ Failure ]
>+        - http/tests/xmlhttprequest/re-login-async.html [ Failure ]
>+        - http/tests/xmlhttprequest/re-login.html [ Failure ]
>+        - http/tests/xmlhttprequest/redirect-credentials-responseURL.html [ Failure ]
>+        - http/tests/xmlhttprequest/remember-bad-password.html [ Failure ]
>+
>+        * platform/network/ResourceHandle.h:
>+        * platform/network/curl/CurlContext.cpp:
>+        (WebCore::CurlHandle::setHttpAuthUserPass):
>+        (WebCore::CurlHandle::enableHttpAuthentication): Deleted.
>+        * platform/network/curl/CurlContext.h:
>+        * platform/network/curl/CurlRequest.cpp:
>+        (WebCore::CurlRequest::setAuthenticationScheme):
>+        (WebCore::CurlRequest::setupTransfer):
>+        * platform/network/curl/CurlRequest.h:
>+        * platform/network/curl/ResourceHandleCurl.cpp:
>+        (WebCore::ResourceHandle::start):
>+        (WebCore::ResourceHandle::didReceiveAuthenticationChallenge):
>+        (WebCore::ResourceHandle::receivedCredential):
>+        (WebCore::ResourceHandle::getCredential):
>+        (WebCore::ResourceHandle::restartRequestWithCredential):
>+        (WebCore::ResourceHandle::platformLoadResourceSynchronously):
>+        (WebCore::ResourceHandle::continueAfterWillSendRequest):
>+
> 2018-09-10  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         [WTF] Add Markable<T, Traits>
>diff --git a/Source/WebCore/platform/network/ResourceHandle.h b/Source/WebCore/platform/network/ResourceHandle.h
>index 90b0fda53b5..01a86e359b1 100644
>--- a/Source/WebCore/platform/network/ResourceHandle.h
>+++ b/Source/WebCore/platform/network/ResourceHandle.h
>@@ -263,8 +263,8 @@ private:
> 
>     bool shouldRedirectAsGET(const ResourceRequest&, bool crossOrigin);
> 
>-    std::optional<std::pair<String, String>> getCredential(ResourceRequest&, bool);
>-    void restartRequestWithCredential(const String& user, const String& password);
>+    std::optional<Credential> getCredential(const ResourceRequest&, bool);
>+    void restartRequestWithCredential(const ProtectionSpace&, const Credential&);
> 
>     void handleDataURL();
> #endif
>diff --git a/Source/WebCore/platform/network/curl/CurlContext.cpp b/Source/WebCore/platform/network/curl/CurlContext.cpp
>index 64b838c9d5d..3d393e3ede0 100644
>--- a/Source/WebCore/platform/network/curl/CurlContext.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlContext.cpp
>@@ -514,15 +514,11 @@ void CurlHandle::enableAllowedProtocols()
>     curl_easy_setopt(m_handle, CURLOPT_PROTOCOLS, allowedProtocols);
> }
> 
>-void CurlHandle::enableHttpAuthentication(long option)
>-{
>-    curl_easy_setopt(m_handle, CURLOPT_HTTPAUTH, option);
>-}
>-
>-void CurlHandle::setHttpAuthUserPass(const String& user, const String& password)
>+void CurlHandle::setHttpAuthUserPass(const String& user, const String& password, long authType)
> {
>     curl_easy_setopt(m_handle, CURLOPT_USERNAME, user.utf8().data());
>     curl_easy_setopt(m_handle, CURLOPT_PASSWORD, password.utf8().data());
>+    curl_easy_setopt(m_handle, CURLOPT_HTTPAUTH, authType);
> }
> 
> void CurlHandle::setCACertPath(const char* path)
>diff --git a/Source/WebCore/platform/network/curl/CurlContext.h b/Source/WebCore/platform/network/curl/CurlContext.h
>index 5c126af1101..c314e5278b1 100644
>--- a/Source/WebCore/platform/network/curl/CurlContext.h
>+++ b/Source/WebCore/platform/network/curl/CurlContext.h
>@@ -251,8 +251,7 @@ public:
>     void enableAcceptEncoding();
>     void enableAllowedProtocols();
> 
>-    void enableHttpAuthentication(long);
>-    void setHttpAuthUserPass(const String&, const String&);
>+    void setHttpAuthUserPass(const String&, const String&, long authType = CURLAUTH_ANY);
> 
>     void setCACertPath(const char*);
>     void setSslVerifyPeer(VerifyPeer);
>diff --git a/Source/WebCore/platform/network/curl/CurlRequest.cpp b/Source/WebCore/platform/network/curl/CurlRequest.cpp
>index 726992aa3d2..3784c6f888b 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.cpp
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.cpp
>@@ -57,6 +57,31 @@ void CurlRequest::invalidateClient()
>     m_messageQueue = nullptr;
> }
> 
>+void CurlRequest::setAuthenticationScheme(ProtectionSpaceAuthenticationScheme scheme)
>+{
>+    switch (scheme) {
>+    case ProtectionSpaceAuthenticationSchemeHTTPBasic:
>+        m_authType = CURLAUTH_BASIC;
>+        break;
>+
>+    case ProtectionSpaceAuthenticationSchemeHTTPDigest:
>+        m_authType = CURLAUTH_DIGEST;
>+        break;
>+
>+    case ProtectionSpaceAuthenticationSchemeNTLM:
>+        m_authType = CURLAUTH_NTLM;
>+        break;
>+
>+    case ProtectionSpaceAuthenticationSchemeNegotiate:
>+        m_authType = CURLAUTH_NEGOTIATE;
>+        break;
>+
>+    default:
>+        m_authType = CURLAUTH_ANY;
>+        break;
>+    }
>+}
>+
> void CurlRequest::setUserPass(const String& user, const String& password)
> {
>     ASSERT(isMainThread());
>@@ -183,8 +208,7 @@ CURL* CurlRequest::setupTransfer()
>     }
> 
>     if (!m_user.isEmpty() || !m_password.isEmpty()) {
>-        m_curlHandle->enableHttpAuthentication(CURLAUTH_ANY);
>-        m_curlHandle->setHttpAuthUserPass(m_user, m_password);
>+        m_curlHandle->setHttpAuthUserPass(m_user, m_password, m_authType);
>     }
> 
>     m_curlHandle->setHeaderCallbackFunction(didReceiveHeaderCallback, this);
>diff --git a/Source/WebCore/platform/network/curl/CurlRequest.h b/Source/WebCore/platform/network/curl/CurlRequest.h
>index e4cc4a234fd..09db94750c4 100644
>--- a/Source/WebCore/platform/network/curl/CurlRequest.h
>+++ b/Source/WebCore/platform/network/curl/CurlRequest.h
>@@ -34,6 +34,7 @@
> #include "CurlSSLVerifier.h"
> #include "FileSystem.h"
> #include "NetworkLoadMetrics.h"
>+#include "ProtectionSpace.h"
> #include "ResourceRequest.h"
> #include <wtf/MessageQueue.h>
> #include <wtf/Noncopyable.h>
>@@ -66,6 +67,7 @@ public:
>     virtual ~CurlRequest() = default;
> 
>     void invalidateClient();
>+    WEBCORE_EXPORT void setAuthenticationScheme(ProtectionSpaceAuthenticationScheme);
>     WEBCORE_EXPORT void setUserPass(const String&, const String&);
> 
>     void start();
>@@ -161,6 +163,7 @@ private:
>     ResourceRequest m_request;
>     String m_user;
>     String m_password;
>+    unsigned long m_authType { CURLAUTH_ANY };
>     bool m_shouldSuspend { false };
>     bool m_enableMultipart { false };
> 
>diff --git a/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp b/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>index c42adc378a6..ec3e56ac39a 100644
>--- a/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>+++ b/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>@@ -83,8 +83,10 @@ bool ResourceHandle::start()
> 
>     d->m_curlRequest = createCurlRequest(WTFMove(request));
> 
>-    if (auto credential = getCredential(d->m_firstRequest, false))
>-        d->m_curlRequest->setUserPass(credential->first, credential->second);
>+    if (auto credential = getCredential(d->m_firstRequest, false)) {
>+        d->m_curlRequest->setUserPass(credential->user(), credential->password());
>+        d->m_curlRequest->setAuthenticationScheme(ProtectionSpaceAuthenticationSchemeHTTPBasic);
>+    }
> 
>     d->m_curlRequest->start();
> 
>@@ -228,7 +230,7 @@ void ResourceHandle::didReceiveAuthenticationChallenge(const AuthenticationChall
>             urlToStore = challenge.failureResponse().url();
>         CredentialStorage::defaultCredentialStorage().set(partition, credential, challenge.protectionSpace(), urlToStore);
> 
>-        restartRequestWithCredential(credential.user(), credential.password());
>+        restartRequestWithCredential(challenge.protectionSpace(), credential);
> 
>         d->m_user = String();
>         d->m_pass = String();
>@@ -253,7 +255,7 @@ void ResourceHandle::didReceiveAuthenticationChallenge(const AuthenticationChall
>                     CredentialStorage::defaultCredentialStorage().set(partition, credential, challenge.protectionSpace(), challenge.failureResponse().url());
>                 }
> 
>-                restartRequestWithCredential(credential.user(), credential.password());
>+                restartRequestWithCredential(challenge.protectionSpace(), credential);
>                 return;
>             }
>         }
>@@ -288,7 +290,7 @@ void ResourceHandle::receivedCredential(const AuthenticationChallenge& challenge
>         }
>     }
> 
>-    restartRequestWithCredential(credential.user(), credential.password());
>+    restartRequestWithCredential(challenge.protectionSpace(), credential);
> 
>     clearAuthentication();
> }
>@@ -330,13 +332,15 @@ void ResourceHandle::receivedChallengeRejection(const AuthenticationChallenge&)
>     ASSERT_NOT_REACHED();
> }
> 
>-std::optional<std::pair<String, String>> ResourceHandle::getCredential(ResourceRequest& request, bool redirect)
>+std::optional<Credential> ResourceHandle::getCredential(const ResourceRequest& request, bool redirect)
> {
>     // m_user/m_pass are credentials given manually, for instance, by the arguments passed to XMLHttpRequest.open().
>-    String partition = request.cachePartition();
>+    Credential credential { d->m_user, d->m_pass, CredentialPersistenceNone };
> 
>     if (shouldUseCredentialStorage()) {
>-        if (d->m_user.isEmpty() && d->m_pass.isEmpty()) {
>+        String partition = request.cachePartition();
>+
>+        if (credential.isEmpty()) {
>             // <rdar://problem/7174050> - For URLs that match the paths of those previously challenged for HTTP Basic authentication, 
>             // try and reuse the credential preemptively, as allowed by RFC 2617.
>             d->m_initialCredential = CredentialStorage::defaultCredentialStorage().get(partition, request.url());
>@@ -345,25 +349,17 @@ std::optional<std::pair<String, String>> ResourceHandle::getCredential(ResourceR
>             // before sending a request. This makes it possible to implement logout by sending an
>             // XMLHttpRequest with known incorrect credentials, and aborting it immediately (so that
>             // an authentication dialog doesn't pop up).
>-            CredentialStorage::defaultCredentialStorage().set(partition, Credential(d->m_user, d->m_pass, CredentialPersistenceNone), request.url());
>+            CredentialStorage::defaultCredentialStorage().set(partition, credential, request.url());
>         }
>     }
> 
>-    String user = d->m_user;
>-    String password = d->m_pass;
>-
>-    if (!d->m_initialCredential.isEmpty()) {
>-        user = d->m_initialCredential.user();
>-        password = d->m_initialCredential.password();
>-    }
>-
>-    if (user.isEmpty() && password.isEmpty())
>-        return std::nullopt;
>+    if (!d->m_initialCredential.isEmpty())
>+        return d->m_initialCredential;
> 
>-    return std::pair<String, String>(user, password);
>+    return std::nullopt;
> }
> 
>-void ResourceHandle::restartRequestWithCredential(const String& user, const String& password)
>+void ResourceHandle::restartRequestWithCredential(const ProtectionSpace& protectionSpace, const Credential& credential)
> {
>     ASSERT(isMainThread());
> 
>@@ -374,15 +370,19 @@ void ResourceHandle::restartRequestWithCredential(const String& user, const Stri
>     d->m_curlRequest->cancel();
> 
>     d->m_curlRequest = createCurlRequest(WTFMove(previousRequest), RequestStatus::ReusedRequest);
>-    d->m_curlRequest->setUserPass(user, password);
>+    d->m_curlRequest->setAuthenticationScheme(protectionSpace.authenticationScheme());
>+    d->m_curlRequest->setUserPass(credential.user(), credential.password());
>     d->m_curlRequest->start();
> }
> 
> void ResourceHandle::platformLoadResourceSynchronously(NetworkingContext* context, const ResourceRequest& request, StoredCredentialsPolicy storedCredentialsPolicy, ResourceError& error, ResourceResponse& response, Vector<char>& data)
> {
>     ASSERT(isMainThread());
>+    ASSERT(!request.isEmpty());
> 
>     SynchronousLoaderClient client;
>+    client.setAllowStoredCredentials(storedCredentialsPolicy == StoredCredentialsPolicy::Use);
>+
>     bool defersLoading = false;
>     bool shouldContentSniff = true;
>     bool shouldContentEncodingSniff = true;
>@@ -396,6 +396,12 @@ void ResourceHandle::platformLoadResourceSynchronously(NetworkingContext* contex
> 
>     auto requestCopy = handle->firstRequest();
>     handle->d->m_curlRequest = handle->createCurlRequest(WTFMove(requestCopy));
>+
>+    if (auto credential = handle->getCredential(handle->d->m_firstRequest, false)) {
>+        handle->d->m_curlRequest->setUserPass(credential->user(), credential->password());
>+        handle->d->m_curlRequest->setAuthenticationScheme(ProtectionSpaceAuthenticationSchemeHTTPBasic);
>+    }
>+
>     handle->d->m_curlRequest->start();
> 
>     do {
>@@ -511,7 +517,7 @@ void ResourceHandle::continueAfterWillSendRequest(ResourceRequest&& request)
>     d->m_curlRequest = createCurlRequest(WTFMove(request));
> 
>     if (shouldForwardCredential && credential)
>-        d->m_curlRequest->setUserPass(credential->first, credential->second);
>+        d->m_curlRequest->setUserPass(credential->user(), credential->password());
> 
>     d->m_curlRequest->start();
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 189318: 349489 | 349575 | 349828 | 349899