Attachment 349693 Details for Bug 189601 – PATCH

[patch] PATCH

189601.diff (text/plain), 6.28 KB, created by Basuke Suzuki on 2018-09-13 13:25:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-09-13 13:25:59 PDT

Size: 6.28 KB

patch

obsolete

>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index b33404c0fba..7fbbf6b2477 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-09-13  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl][WebKit] Bug fix for continuously retrying with empty credentials.
>+        https://bugs.webkit.org/show_bug.cgi?id=189601
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added stop condition for empty credentials passed by client.
>+
>+        * NetworkProcess/curl/NetworkDataTaskCurl.cpp:
>+        (WebKit::NetworkDataTaskCurl::curlDidReceiveResponse):
>+        (WebKit::NetworkDataTaskCurl::invokeDidReceiveResponse):
>+        (WebKit::NetworkDataTaskCurl::tryHttpAuthentication):
>+        (WebKit::NetworkDataTaskCurl::tryProxyAuthentication):
>+        * NetworkProcess/curl/NetworkDataTaskCurl.h:
>+
> 2018-09-10  Tim Horton  <timothy_horton@apple.com>
> 
>         Try to fix the build after r235850
>diff --git a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
>index f7d49c282b1..a14dd1261bf 100644
>--- a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
>+++ b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
>@@ -172,22 +172,7 @@ void NetworkDataTaskCurl::curlDidReceiveResponse(CurlRequest& request, const Cur
>         return;
>     }
> 
>-    didReceiveResponse(ResourceResponse(m_response), [this, protectedThis = makeRef(*this)](PolicyAction policyAction) {
>-        if (m_state == State::Canceling || m_state == State::Completed)
>-            return;
>-
>-        switch (policyAction) {
>-        case PolicyAction::Use:
>-            if (m_curlRequest)
>-                m_curlRequest->completeDidReceiveResponse();
>-            break;
>-        case PolicyAction::Ignore:
>-            break;
>-        case PolicyAction::Download:
>-            notImplemented();
>-            break;
>-        }
>-    });
>+    invokeDidReceiveResponse();
> }
> 
> void NetworkDataTaskCurl::curlDidReceiveBuffer(CurlRequest&, Ref<SharedBuffer>&& buffer)
>@@ -237,6 +222,26 @@ bool NetworkDataTaskCurl::shouldRedirectAsGET(const ResourceRequest& request, bo
>     return false;
> }
> 
>+void NetworkDataTaskCurl::invokeDidReceiveResponse()
>+{
>+    didReceiveResponse(ResourceResponse(m_response), [this, protectedThis = makeRef(*this)](PolicyAction policyAction) {
>+        if (m_state == State::Canceling || m_state == State::Completed)
>+            return;
>+
>+        switch (policyAction) {
>+        case PolicyAction::Use:
>+            if (m_curlRequest)
>+                m_curlRequest->completeDidReceiveResponse();
>+            break;
>+        case PolicyAction::Ignore:
>+            break;
>+        case PolicyAction::Download:
>+            notImplemented();
>+            break;
>+        }
>+    });
>+}
>+
> void NetworkDataTaskCurl::willPerformHTTPRedirection()
> {
>     static const int maxRedirects = 20;
>@@ -353,14 +358,23 @@ void NetworkDataTaskCurl::tryHttpAuthentication(AuthenticationChallenge&& challe
>             return;
>         }
> 
>-        if (disposition == AuthenticationChallengeDisposition::UseCredential && !credential.isEmpty()) {
>+        if (disposition == AuthenticationChallengeDisposition::UseCredential && (!credential.isEmpty() || !m_didChallengeEmptyCredentialForAuth)) {
>+            // When "isAllowedToAskUserForCredentials" is false, an empty credential, which might cause
>+            // an infinite authentication loop. To avoid such infinite loop, a HTTP authentication with empty
>+            // user and password is processed only once.
>+            if (credential.isEmpty())
>+                m_didChallengeEmptyCredentialForAuth = true;
>+
>             if (m_storedCredentialsPolicy == StoredCredentialsPolicy::Use) {
>                 if (credential.persistence() == CredentialPersistenceForSession || credential.persistence() == CredentialPersistencePermanent)
>                     m_session->networkStorageSession().credentialStorage().set(m_partition, credential, challenge.protectionSpace(), challenge.failureResponse().url());
>             }
>+
>+            restartWithCredential(credential);
>+            return;
>         }
> 
>-        restartWithCredential(credential);
>+        invokeDidReceiveResponse();
>     });
> }
> 
>@@ -376,11 +390,19 @@ void NetworkDataTaskCurl::tryProxyAuthentication(WebCore::AuthenticationChalleng
>             return;
>         }
> 
>-        CurlContext::singleton().setProxyUserPass(credential.user(), credential.password());
>-        CurlContext::singleton().setDefaultProxyAuthMethod();
>+        if (disposition == AuthenticationChallengeDisposition::UseCredential && (!credential.isEmpty() || !m_didChallengeEmptyCredentialForProxyAuth)) {
>+            if (credential.isEmpty())
>+                m_didChallengeEmptyCredentialForProxyAuth = true;
>+
>+            CurlContext::singleton().setProxyUserPass(credential.user(), credential.password());
>+            CurlContext::singleton().setDefaultProxyAuthMethod();
>+
>+            auto requestCredential = m_curlRequest ? Credential(m_curlRequest->user(), m_curlRequest->password(), CredentialPersistenceNone) : Credential();
>+            restartWithCredential(requestCredential);
>+            return;
>+        }
> 
>-        auto requestCredential = m_curlRequest ? Credential(m_curlRequest->user(), m_curlRequest->password(), CredentialPersistenceNone) : Credential();
>-        restartWithCredential(requestCredential);
>+        invokeDidReceiveResponse();
>     });
> }
> 
>diff --git a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h
>index fcc4d34e4d6..aa0ba6cfded 100644
>--- a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h
>+++ b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h
>@@ -69,6 +69,8 @@ private:
>     void curlDidComplete(WebCore::CurlRequest&) override;
>     void curlDidFailWithError(WebCore::CurlRequest&, const WebCore::ResourceError&) override;
> 
>+    void invokeDidReceiveResponse();
>+
>     bool shouldRedirectAsGET(const WebCore::ResourceRequest&, bool crossOrigin);
>     void willPerformHTTPRedirection();
> 
>@@ -85,6 +87,8 @@ private:
>     WebCore::ResourceResponse m_response;
>     unsigned m_redirectCount { 0 };
>     unsigned m_authFailureCount { 0 };
>+    bool m_didChallengeEmptyCredentialForAuth { false };
>+    bool m_didChallengeEmptyCredentialForProxyAuth { false };
> };
> 
> } // namespace WebKit

Actions: View | Formatted Diff | Diff

Attachments on bug 189601: 349693