Attachment 237938 Details for Bug 131349 – Updated according review

[patch] Updated according review

bug-131349-20140911090327.patch (text/plain), 10.64 KB, created by youenn fablet on 2014-09-11 00:03:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2014-09-11 00:03:33 PDT

Size: 10.64 KB

patch

obsolete

>Subversion Revision: 173424
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 96763a0f428caaeb2f8ff9f3077b9b75f97c6f25..3ade8b3bb27790588f0b1a22b10ba05898545f99 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2014-09-10  Youenn Fablet  <youenn.fablet@crf.canon.fr>
>+
>+        [WK2] Authentication dialog is displayed for cross-origin XHR
>+        https://bugs.webkit.org/show_bug.cgi?id=131349
>+
>+        Reviewed by Alexey Proskuryakov.
>+
>+        * WebCore.exp.in: Export of isAllowedToAskUserForCredentials.
>+        * loader/ResourceLoader.cpp:
>+        (WebCore::ResourceLoader::isAllowedToAskUserForCredentials): Replacing clientCredentialPolicy method. Returns true if credentials can be requested to the user.
>+        (WebCore::ResourceLoader::didReceiveAuthenticationChallenge): Updated to use isAllowedToAskUserForCredentials.
>+        * loader/ResourceLoader.h: Removing clientCredentialPolicy method and adding isAllowedToAskUserForCredentials method.
>+
> 2014-09-08  Mihnea Ovidenie  <mihnea@adobe.com>
> 
>         [CSSRegions]Incorrect layout for multicol element transformed into region
>diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
>index 28dc470bbd57441bcd6056ed62c72bd5fbcdc51a..e63f01c86779bc5ca1961c892044bf07128f4d9a 100644
>--- a/Source/WebKit2/ChangeLog
>+++ b/Source/WebKit2/ChangeLog
>@@ -1,3 +1,19 @@
>+2014-09-10  Youenn Fablet  <youenn.fablet@crf.canon.fr>
>+
>+        [WK2] Authentication dialog is displayed for cross-origin XHR
>+        https://bugs.webkit.org/show_bug.cgi?id=131349
>+
>+        Reviewed by Alexey Proskuryakov.
>+
>+        Precomputing client credential policy in the Web Process before sending the resource load task to the Network Process.
>+
>+        * NetworkProcess/NetworkResourceLoader.cpp:
>+        (WebKit::NetworkResourceLoader::didReceiveAuthenticationChallenge): Added an ASSERT to ensure that credential policy is never set to DoNotAskClientForCrossOriginCredentials.
>+        * WebProcess/Network/WebResourceLoadScheduler.cpp:
>+        (WebKit::WebResourceLoadScheduler::scheduleLoad): Precomputing client credential policy to handle the case of cross-origin requests.
>+        * WebProcess/Network/WebResourceLoader.cpp:
>+        (WebKit::WebResourceLoader::willSendRequest): Added a TODO to check whether redirections need a specific handling.
>+
> 2014-09-08  Antti Koivisto  <antti@apple.com>
> 
>         Pass certificate info as part of ResourceResponse
>diff --git a/Source/WebCore/WebCore.exp.in b/Source/WebCore/WebCore.exp.in
>index 7262e736e0db67410edab2e8a4925b46af9b59f6..8d1fd406ffdc4f95cbe00bfbfcfaef52e8c82a89 100644
>--- a/Source/WebCore/WebCore.exp.in
>+++ b/Source/WebCore/WebCore.exp.in
>@@ -1693,6 +1693,7 @@ __ZNK7WebCore14ResourceBuffer4sizeEv
> __ZNK7WebCore14ResourceBuffer7isEmptyEv
> __ZNK7WebCore14ResourceHandle10connectionEv
> __ZNK7WebCore14ResourceLoader11frameLoaderEv
>+__ZNK7WebCore14ResourceLoader32isAllowedToAskUserForCredentialsEv
> __ZNK7WebCore14ScrollableArea13scrolledToTopEv
> __ZNK7WebCore14ScrollableArea14scrollAnimatorEv
> __ZNK7WebCore14ScrollableArea14scrolledToLeftEv
>diff --git a/Source/WebCore/loader/ResourceLoader.cpp b/Source/WebCore/loader/ResourceLoader.cpp
>index 7aa4d87171d97589edc5a820ab90a38a857504bf..2c81fa34c66cb0ddc33f8f4627325ce3c010e5bf 100644
>--- a/Source/WebCore/loader/ResourceLoader.cpp
>+++ b/Source/WebCore/loader/ResourceLoader.cpp
>@@ -538,6 +538,11 @@ bool ResourceLoader::shouldUseCredentialStorage()
>     return frameLoader()->client().shouldUseCredentialStorage(documentLoader(), identifier());
> }
> 
>+bool ResourceLoader::isAllowedToAskUserForCredentials() const
>+{
>+    return m_options.clientCredentialPolicy() == AskClientForAllCredentials || (m_options.clientCredentialPolicy() == DoNotAskClientForCrossOriginCredentials && m_frame->document()->securityOrigin()->canRequest(originalRequest().url()));
>+}
>+
> void ResourceLoader::didReceiveAuthenticationChallenge(const AuthenticationChallenge& challenge)
> {
>     ASSERT(m_handle->hasAuthenticationChallenge());
>@@ -547,7 +552,7 @@ void ResourceLoader::didReceiveAuthenticationChallenge(const AuthenticationChall
>     Ref<ResourceLoader> protect(*this);
> 
>     if (m_options.allowCredentials() == AllowStoredCredentials) {
>-        if (m_options.clientCredentialPolicy() == AskClientForAllCredentials || (m_options.clientCredentialPolicy() == DoNotAskClientForCrossOriginCredentials && m_frame->document()->securityOrigin()->canRequest(originalRequest().url()))) {
>+        if (isAllowedToAskUserForCredentials()) {
>             frameLoader()->notifier().didReceiveAuthenticationChallenge(this, challenge);
>             return;
>         }
>diff --git a/Source/WebCore/loader/ResourceLoader.h b/Source/WebCore/loader/ResourceLoader.h
>index 2bf75378f5434b93475e962ff3668d2b931b2ae6..e37f0c2c8c5b51832d341b980b30607ac52df5b2 100644
>--- a/Source/WebCore/loader/ResourceLoader.h
>+++ b/Source/WebCore/loader/ResourceLoader.h
>@@ -122,10 +122,11 @@ public:
>     bool shouldSendResourceLoadCallbacks() const { return m_options.sendLoadCallbacks() == SendCallbacks; }
>     void setSendCallbackPolicy(SendCallbackPolicy sendLoadCallbacks) { m_options.setSendLoadCallbacks(sendLoadCallbacks); }
>     bool shouldSniffContent() const { return m_options.sniffContent() == SniffContent; }
>-    ClientCredentialPolicy clientCredentialPolicy() const { return m_options.clientCredentialPolicy(); }
>+    WEBCORE_EXPORT bool isAllowedToAskUserForCredentials() const;
> 
>     bool reachedTerminalState() const { return m_reachedTerminalState; }
> 
>+
>     const ResourceRequest& request() const { return m_request; }
> 
>     void setDataBufferingPolicy(DataBufferingPolicy);
>diff --git a/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp
>index 1bfb6b771a3bc05e51088710ec35ea1b10e39a0d..3381813e9a9e5329381b9df87715925055295868 100644
>--- a/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp
>@@ -385,10 +385,9 @@ bool NetworkResourceLoader::shouldUseCredentialStorage(ResourceHandle* handle)
> void NetworkResourceLoader::didReceiveAuthenticationChallenge(ResourceHandle* handle, const AuthenticationChallenge& challenge)
> {
>     ASSERT_UNUSED(handle, handle == m_handle);
>+    // NetworkResourceLoader does not know whether the request is cross origin, so Web process computes an applicable credential policy for it.
>+    ASSERT(m_clientCredentialPolicy != DoNotAskClientForCrossOriginCredentials);
> 
>-    // FIXME (http://webkit.org/b/115291): Since we go straight to the UI process for authentication we don't get WebCore's
>-    // cross-origin check before asking the client for credentials.
>-    // Therefore we are too permissive in the case where the ClientCredentialPolicy is DoNotAskClientForCrossOriginCredentials.
>     if (m_clientCredentialPolicy == DoNotAskClientForAnyCredentials) {
>         challenge.authenticationClient()->receivedRequestToContinueWithoutCredential(challenge);
>         return;
>diff --git a/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp b/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp
>index c55802f3da73c27482768b5fcf8e3a5e6216679c..71c88898d40cc5866e660649054309672b180a2f 100644
>--- a/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp
>+++ b/Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp
>@@ -169,7 +169,7 @@ void WebResourceLoadScheduler::scheduleLoad(ResourceLoader* resourceLoader, Cach
>     loadParameters.contentSniffingPolicy = contentSniffingPolicy;
>     loadParameters.allowStoredCredentials = allowStoredCredentials;
>     // If there is no WebFrame then this resource cannot be authenticated with the client.
>-    loadParameters.clientCredentialPolicy = (webFrame && webPage) ? resourceLoader->clientCredentialPolicy() : DoNotAskClientForAnyCredentials;
>+    loadParameters.clientCredentialPolicy = (webFrame && webPage && resourceLoader->isAllowedToAskUserForCredentials()) ? AskClientForAllCredentials : DoNotAskClientForAnyCredentials;
>     loadParameters.shouldClearReferrerOnHTTPSToHTTPRedirect = shouldClearReferrerOnHTTPSToHTTPRedirect;
>     loadParameters.isMainResource = resource && resource->type() == CachedResource::MainResource;
>     loadParameters.defersLoading = resourceLoader->defersLoading();
>diff --git a/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp b/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp
>index 47045a80505f13df7144716a509e353871010dbf..bcc8dab1637b034311e863488a0ed71e5eafc7aa 100644
>--- a/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp
>+++ b/Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp
>@@ -89,6 +89,7 @@ void WebResourceLoader::willSendRequest(const ResourceRequest& proposedRequest,
>     ResourceRequest newRequest = proposedRequest;
>     if (m_coreLoader->documentLoader()->applicationCacheHost()->maybeLoadFallbackForRedirect(m_coreLoader.get(), newRequest, redirectResponse))
>         return;
>+    // FIXME: Do we need to update NetworkResourceLoader clientCredentialPolicy in case loader policy is DoNotAskClientForCrossOriginCredentials?
>     m_coreLoader->willSendRequest(newRequest, redirectResponse);
>     
>     if (!m_coreLoader)
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ccc5cce843bbe9439252cb7981be9af2283f092f..29f6d0b878c4fc689b684d1b5e95bbbb08a67c6f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2014-09-10  Youenn Fablet  <youenn.fablet@crf.canon.fr>
>+
>+        [WK2] Authentication dialog is displayed for cross-origin XHR
>+        https://bugs.webkit.org/show_bug.cgi?id=131349
>+
>+        Reviewed by Alexey Proskuryakov.
>+
>+        * platform/mac-wk2/TestExpectations: Unskipped tests.
>+
> 2014-09-08  Mihnea Ovidenie  <mihnea@adobe.com>
> 
>         [CSSRegions]Incorrect layout for multicol element transformed into region
>diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations
>index 6698074ea6d59375baadac87f2ccca5347f4f787..ad6a36eed5ebe4373fb6fa36b0eef41a3db90441 100644
>--- a/LayoutTests/platform/mac-wk2/TestExpectations
>+++ b/LayoutTests/platform/mac-wk2/TestExpectations
>@@ -341,10 +341,6 @@ webkit.org/b/125996 platform/mac/accessibility/search-when-element-starts-in-tab
> 
> webkit.org/b/127960 [ MountainLion ] http/tests/security/cross-origin-plugin-private-browsing-toggled.html [ Pass Failure ]
> 
>-webkit.org/b/131349 http/tests/xmlhttprequest/access-control-preflight-credential-async.html [ Failure ]
>-webkit.org/b/131349 http/tests/xmlhttprequest/cross-origin-no-authorization.html [ Failure ]
>-webkit.org/b/131349 http/tests/xmlhttprequest/cross-origin-no-credential-prompt.html [ Failure ]
>-
> webkit.org/b/134550 [ Mavericks ] http/tests/cache/iframe-304-crash.html [ Pass Failure ]
> 
> # Subpixel wrong cliprect on WK2

Actions: View | Formatted Diff | Diff

Attachments on bug 131349: 228814 | 232423 | 237689 | 237847 | 237938 | 237949