NetworkLoadChecker should check cached redirections
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 May 2018 19:09:08 +0000 (19:09 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 May 2018 19:09:08 +0000 (19:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185849

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

* web-platform-tests/service-workers/service-worker/redirected-response.https-expected.txt:
* web-platform-tests/service-workers/service-worker/registration-security-error.https-expected.txt:

Source/WebCore:

Covered by rebased tests.

* loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::willSendRequestInternal):
       Log the case of a redirection with fetch error mode.

Source/WebKit:

* NetworkProcess/NetworkLoadChecker.cpp:
(WebKit::NetworkLoadChecker::checkRedirection):
Set the resource error url as done by WebCore SubresourceLoader.
* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::retrieveCacheEntry):
Pass the resource request to dispatchWillSendRedirectedRequest now needs it.
(WebKit::NetworkResourceLoader::willSendRedirectedRequest):
Make sure that m_networkLoad is not null before cancelling it since we might be checking a cached redirection.
(WebKit::NetworkResourceLoader::continueWillSendRedirectedRequest):
Ensure the redirect response is coming from the Network before adding it to the cache.
(WebKit::NetworkResourceLoader::dispatchWillSendRequestForCacheEntry):
Call willSendRedirectedRequest to make sure the cached redirect is validated.
* NetworkProcess/NetworkResourceLoader.h:

LayoutTests:

* TestExpectations:
* http/tests/fetch/redirectmode-and-preload-expected.txt:
* http/tests/fetch/redirectmode-and-preload.html:
Removed tests that mix manual/error redirect mode with no-cors since this is no longer a valid possibility.
* http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt:
* http/tests/xmlhttprequest/access-control-and-redirects-expected.txt:
* platform/mac-wk1/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt: Removed.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@232121 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/http/tests/fetch/redirectmode-and-preload-expected.txt
LayoutTests/http/tests/fetch/redirectmode-and-preload.html
LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt
LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-expected.txt
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/redirected-response.https-expected.txt
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/registration-security-error.https-expected.txt
LayoutTests/platform/mac-wk1/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt [deleted file]
Source/WebCore/ChangeLog
Source/WebCore/loader/SubresourceLoader.cpp
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
Source/WebKit/NetworkProcess/NetworkResourceLoader.h

index f991073..6509129 100644 (file)
@@ -1,3 +1,18 @@
+2018-05-23  Youenn Fablet  <youenn@apple.com>
+
+        NetworkLoadChecker should check cached redirections
+        https://bugs.webkit.org/show_bug.cgi?id=185849
+
+        Reviewed by Chris Dumez.
+
+        * TestExpectations:
+        * http/tests/fetch/redirectmode-and-preload-expected.txt:
+        * http/tests/fetch/redirectmode-and-preload.html:
+        Removed tests that mix manual/error redirect mode with no-cors since this is no longer a valid possibility.
+        * http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt:
+        * http/tests/xmlhttprequest/access-control-and-redirects-expected.txt:
+        * platform/mac-wk1/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt: Removed.
+
 2018-05-23  Nan Wang  <n_wang@apple.com>
 
         AX: setValue on contenteditable should preserve whitespace
index 62808f7..edbf694 100644 (file)
@@ -362,6 +362,8 @@ webkit.org/b/179881 imported/w3c/web-platform-tests/encoding/eof-shift_jis.html
 imported/w3c/web-platform-tests/encoding/legacy-mb-tchinese/big5/big5-encode-form-errors-extBa.html [ Skip ]
 [ Debug ] imported/w3c/web-platform-tests/encoding/legacy-mb-tchinese/big5/big5-encode-form-errors-extBb.html [ Skip ]
 
+# redirect-chain.php is generating some random number that appear in console error logging.
+http/tests/cache/disk-cache/redirect-chain-limits.html [ DumpJSConsoleLogInStdErr ]
 
 # Content encoding sniffing is only supported by CFNetwork.
 http/tests/xmlhttprequest/gzip-content-type-no-content-encoding.html [ Skip ]
index 4ab5519..0b8c04c 100644 (file)
@@ -1,13 +1,6 @@
-CONSOLE MESSAGE: line 19: No-Cors mode requires follow redirect mode
-CONSOLE MESSAGE: line 19: Not allowed to request resource
-CONSOLE MESSAGE: line 19: Fetch API cannot load http://127.0.0.1:8000/fetch/resources/redirect-with-cache.php?enableCaching&url=http://localhost:8000/security/resources/allow-if-origin.php?allowCache&origin=http%3A%2F%2F127.0.0.1%3A8000&name=alert-fail.js&contentType=text/ascii due to access control checks.
-CONSOLE MESSAGE: line 32: No-Cors mode requires follow redirect mode
-CONSOLE MESSAGE: line 32: Not allowed to request resource
-CONSOLE MESSAGE: line 32: Fetch API cannot load http://127.0.0.1:8000/fetch/resources/redirect-with-cache.php?enableCaching&url=http://localhost:8000/security/resources/allow-if-origin.php?allowCache&origin=http%3A%2F%2F127.0.0.1%3A8000&name=alert-fail.js&contentType=text/ascii due to access control checks.
+CONSOLE MESSAGE: Not allowed to follow a redirection while loading http://127.0.0.1:8000/fetch/resources/redirect-with-cache.php?enableCaching&url=http://localhost:8000/security/resources/allow-if-origin.php?allowCache&origin=http%3A%2F%2F127.0.0.1%3A8000&name=alert-fail.js&contentType=text/ascii
 CONSOLE MESSAGE: Fetch API cannot load http://127.0.0.1:8000/fetch/resources/redirect-with-cache.php?enableCaching&url=http://localhost:8000/security/resources/allow-if-origin.php?allowCache&origin=http%3A%2F%2F127.0.0.1%3A8000&name=alert-fail.js&contentType=text/ascii due to access control checks.
 
-PASS Fetch should check for redirections even if resource is preloaded (same fetch options except for redirect mode) 
 PASS Fetch should check for redirections even if resource is preloaded (different fetch mode, different redirect mode) 
-PASS Fetch should check for redirections even if resource is preloaded (same fetch options except for redirect mode) 
 PASS Fetch should check for redirections even if resource is preloaded (different fetch mode, different redirect mode) 
 
index 8dbb008..8bcd9b4 100644 (file)
@@ -15,9 +15,6 @@
 function startTests()
 {
     var preloadUrl = "./resources/redirect-with-cache.php?enableCaching&url=http://localhost:8000/security/resources/allow-if-origin.php?allowCache&origin=http%3A%2F%2F127.0.0.1%3A8000&name=alert-fail.js&contentType=text/ascii";
-    promise_test(function(test) {
-        return promise_rejects(test,new TypeError(), fetch(preloadUrl, {redirect: "manual", mode: "no-cors", credentials: "include"}));
-    }, "Fetch should check for redirections even if resource is preloaded (same fetch options except for redirect mode)");
 
     promise_test(function(test) {
         return fetch(preloadUrl, {redirect: "manual", mode: "cors", credentials: "include"}).then((response) => {
@@ -29,10 +26,6 @@ function startTests()
     }, "Fetch should check for redirections even if resource is preloaded (different fetch mode, different redirect mode)");
 
     promise_test(function(test) {
-        return promise_rejects(test, new TypeError(), fetch(preloadUrl, {redirect: "error", mode: "no-cors", credentials: "include"}));
-    }, "Fetch should check for redirections even if resource is preloaded (same fetch options except for redirect mode)");
-
-    promise_test(function(test) {
         return promise_rejects(test, new TypeError(), fetch(preloadUrl, {redirect: "error", mode: "cors", credentials: "include"}));
     }, "Fetch should check for redirections even if resource is preloaded (different fetch mode, different redirect mode)");
 }
index 593f6c6..b7fe744 100644 (file)
@@ -1,5 +1,5 @@
 CONSOLE MESSAGE: Cross-origin redirection to http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi denied by Cross-Origin Resource Sharing policy: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
-CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi due to access control checks.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi due to access control checks.
 CONSOLE MESSAGE: Cross-origin redirection to foo://bar.cgi denied by Cross-Origin Resource Sharing policy: URL is either a non-HTTP URL or contains credentials.
 CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=foo://bar.cgi&%20%20access-control-allow-origin=http://127.0.0.1:8000 due to access control checks.
 CONSOLE MESSAGE: Preflight response is not successful
index ceca468..2f32d7b 100644 (file)
@@ -1,11 +1,11 @@
 CONSOLE MESSAGE: line 25: Cross-origin redirection to http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi denied by Cross-Origin Resource Sharing policy: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
-CONSOLE MESSAGE: line 25: XMLHttpRequest cannot load http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi due to access control checks.
+CONSOLE MESSAGE: line 25: XMLHttpRequest cannot load http://localhost:8000/resources/redirect.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi due to access control checks.
 CONSOLE MESSAGE: Cross-origin redirection to http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi denied by Cross-Origin Resource Sharing policy: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
-CONSOLE MESSAGE: XMLHttpRequest cannot load http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi due to access control checks.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/resources/redirect.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi due to access control checks.
 CONSOLE MESSAGE: line 25: Cross-origin redirection to http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi denied by Cross-Origin Resource Sharing policy: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
-CONSOLE MESSAGE: line 25: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi due to access control checks.
+CONSOLE MESSAGE: line 25: XMLHttpRequest cannot load http://localhost:8000/resources/redirect.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi due to access control checks.
 CONSOLE MESSAGE: Cross-origin redirection to http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi denied by Cross-Origin Resource Sharing policy: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
-CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi due to access control checks.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/resources/redirect.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi due to access control checks.
 Tests that redirects between origins are never allowed, even when access control is involved.
 
 Per the spec, these test cases should be allowed, but cross-origin redirects are currently unsupported in WebCore.
index cd78e6a..5d92bcc 100644 (file)
@@ -1,3 +1,13 @@
+2018-05-23  Youenn Fablet  <youenn@apple.com>
+
+        NetworkLoadChecker should check cached redirections
+        https://bugs.webkit.org/show_bug.cgi?id=185849
+
+        Reviewed by Chris Dumez.
+
+        * web-platform-tests/service-workers/service-worker/redirected-response.https-expected.txt:
+        * web-platform-tests/service-workers/service-worker/registration-security-error.https-expected.txt:
+
 2018-05-22  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-text] Import WPT test suite
index a44bdd4..6c65207 100644 (file)
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: Redirections are not allowed
+CONSOLE MESSAGE: Not allowed to follow a redirection while loading https://localhost:9443/service-workers/service-worker/resources/redirect.py?Redirect=https%3A%2F%2Flocalhost%3A9443%2Fservice-workers%2Fservice-worker%2Fresources%2Fsimple.txt%3F&error
 CONSOLE MESSAGE: Fetch API cannot load https://localhost:9443/service-workers/service-worker/resources/redirect.py?Redirect=https%3A%2F%2Flocalhost%3A9443%2Fservice-workers%2Fservice-worker%2Fresources%2Fsimple.txt%3F&error due to access control checks.
 CONSOLE MESSAGE: Response served by service worker has redirections
 CONSOLE MESSAGE: Fetch API cannot load https://localhost:9443/service-workers/service-worker/resources/simple.txt? due to access control checks.
@@ -8,6 +8,8 @@ CONSOLE MESSAGE: Response served by service worker is opaque redirect
 CONSOLE MESSAGE: Fetch API cannot load https://localhost:9443/service-workers/service-worker/resources/redirect.py?Redirect=https%3A%2F%2Flocalhost%3A9443%2Fservice-workers%2Fservice-worker%2Fresources%2Fsimple.txt%3F&original-redirect-mode=follow&sw=manual due to access control checks.
 CONSOLE MESSAGE: Response served by service worker is opaque redirect
 CONSOLE MESSAGE: Fetch API cannot load https://localhost:9443/service-workers/service-worker/resources/redirect.py?Redirect=https%3A%2F%2Flocalhost%3A9443%2Fservice-workers%2Fservice-worker%2Fresources%2Fsimple.txt%3F&original-redirect-mode=error&sw=manual due to access control checks.
+CONSOLE MESSAGE: Not allowed to follow a redirection while loading https://localhost:9443/service-workers/service-worker/dummy?url=https%3A%2F%2Flocalhost%3A9443%2Fservice-workers%2Fservice-worker%2Fresources%2Fsimple.txt%3F&original-redirect-mode=error&sw=gen
+CONSOLE MESSAGE: Fetch API cannot load https://localhost:9443/service-workers/service-worker/dummy?url=https%3A%2F%2Flocalhost%3A9443%2Fservice-workers%2Fservice-worker%2Fresources%2Fsimple.txt%3F&original-redirect-mode=error&sw=gen due to access control checks.
 
 PASS initialize global state (service worker registration and caches) 
 PASS mode: "follow", non-intercepted request, no server redirect 
index fae5ef2..0eebcb2 100644 (file)
@@ -3,7 +3,7 @@ PASS Registering same scope as the script directory without the last slash
 PASS Registration scope outside the script directory 
 PASS Registering scope outside domain 
 PASS Registering script outside domain 
-FAIL Registering redirected script assert_throws: Registration of redirected script should fail. function "function () { throw e }" threw object "TypeError: Script URL https://localhost:9443/service-workers/service-worker/resources/redirect.py?Redirect=%2Fservice-workers%2Fservice-worker%2Fresources%2Fregistration-worker.js fetch resulted in error: Redirections are not allowed" that is not a DOMException SecurityError: property "code" is equal to undefined, expected 18
+FAIL Registering redirected script assert_throws: Registration of redirected script should fail. function "function () { throw e }" threw object "TypeError: Script URL https://localhost:9443/service-workers/service-worker/resources/redirect.py?Redirect=%2Fservice-workers%2Fservice-worker%2Fresources%2Fregistration-worker.js fetch resulted in error: Not allowed to follow a redirection while loading https://localhost:9443/service-workers/service-worker/resources/redirect.py?Redirect=%2Fservice-workers%2Fservice-worker%2Fresources%2Fregistration-worker.js" that is not a DOMException SecurityError: property "code" is equal to undefined, expected 18
 PASS Scope including parent-reference and not under the script directory 
 PASS Script URL including consecutive slashes 
 FAIL Script URL is same-origin filesystem: URL assert_throws: Registering a script which has same-origin filesystem: URL should fail with SecurityError. function "function () { throw e }" threw object "TypeError: serviceWorker.register() must be called with a script URL whose protocol is either HTTP or HTTPS" that is not a DOMException SecurityError: property "code" is equal to undefined, expected 18
diff --git a/LayoutTests/platform/mac-wk1/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt b/LayoutTests/platform/mac-wk1/http/tests/xmlhttprequest/access-control-and-redirects-async-expected.txt
deleted file mode 100644 (file)
index b7fe744..0000000
+++ /dev/null
@@ -1,34 +0,0 @@
-CONSOLE MESSAGE: Cross-origin redirection to http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi denied by Cross-Origin Resource Sharing policy: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
-CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi due to access control checks.
-CONSOLE MESSAGE: Cross-origin redirection to foo://bar.cgi denied by Cross-Origin Resource Sharing policy: URL is either a non-HTTP URL or contains credentials.
-CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=foo://bar.cgi&%20%20access-control-allow-origin=http://127.0.0.1:8000 due to access control checks.
-CONSOLE MESSAGE: Preflight response is not successful
-CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=true&%20%20url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&%20%20access-control-allow-origin=* due to access control checks.
-CONSOLE MESSAGE: Request header field x-webkit is not allowed by Access-Control-Allow-Headers.
-CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi due to access control checks.
-Tests that asynchronous XMLHttpRequests handle redirects according to the CORS standard.
-
-Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi without credentials
-Expecting success: false
-PASS: 0
-Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=http://127.0.0.1:8000 without credentials
-Expecting success: true
-PASS: PASS: Cross-domain access allowed.
-
-Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=http://username:password@localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=http://127.0.0.1:8000 without credentials
-Expecting success: true
-PASS: PASS: Cross-domain access allowed.
-
-Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?url=foo://bar.cgi&  access-control-allow-origin=http://127.0.0.1:8000 without credentials
-Expecting success: false
-PASS: 0
-Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=true&  url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=* without credentials
-Expecting success: false
-PASS: 0
-Testing http://localhost:8000/xmlhttprequest/resources/redirect-cors.php?redirect-preflight=false&  url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi&  access-control-allow-origin=*&  access-control-allow-headers=x-webkit without credentials
-Expecting success: false
-PASS: 0
-Testing resources/redirect-cors.php?url=http://127.0.0.1:8000/xmlhttprequest/resources/get.txt without credentials
-Expecting success: true
-PASS: PASS
-
index efaebac..df196cb 100644 (file)
@@ -1,3 +1,16 @@
+2018-05-23  Youenn Fablet  <youenn@apple.com>
+
+        NetworkLoadChecker should check cached redirections
+        https://bugs.webkit.org/show_bug.cgi?id=185849
+
+        Reviewed by Chris Dumez.
+
+        Covered by rebased tests.
+
+        * loader/SubresourceLoader.cpp:
+        (WebCore::SubresourceLoader::willSendRequestInternal):
+       Log the case of a redirection with fetch error mode.
+
 2018-05-23  Nan Wang  <n_wang@apple.com>
 
         AX: setValue on contenteditable should preserve whitespace
index f547035..66af7a5 100644 (file)
@@ -209,7 +209,12 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest&& newRequest, co
     if (!redirectResponse.isNull()) {
         if (options().redirect != FetchOptions::Redirect::Follow) {
             if (options().redirect == FetchOptions::Redirect::Error) {
-                cancel(ResourceError { errorDomainWebKitInternal, 0, redirectResponse.url(), "Redirections are not allowed", ResourceError::Type::AccessControl });
+                ResourceError error { errorDomainWebKitInternal, 0, request().url(), makeString("Not allowed to follow a redirection while loading ", request().url().string()), ResourceError::Type::AccessControl };
+
+                if (m_frame && m_frame->document())
+                    m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, error.localizedDescription());
+
+                cancel(error);
                 return completionHandler(WTFMove(newRequest));
             }
 
index d5837e3..28edc12 100644 (file)
@@ -1,3 +1,24 @@
+2018-05-23  Youenn Fablet  <youenn@apple.com>
+
+        NetworkLoadChecker should check cached redirections
+        https://bugs.webkit.org/show_bug.cgi?id=185849
+
+        Reviewed by Chris Dumez.
+
+        * NetworkProcess/NetworkLoadChecker.cpp:
+        (WebKit::NetworkLoadChecker::checkRedirection):
+        Set the resource error url as done by WebCore SubresourceLoader.
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::retrieveCacheEntry):
+        Pass the resource request to dispatchWillSendRedirectedRequest now needs it.
+        (WebKit::NetworkResourceLoader::willSendRedirectedRequest):
+        Make sure that m_networkLoad is not null before cancelling it since we might be checking a cached redirection.
+        (WebKit::NetworkResourceLoader::continueWillSendRedirectedRequest):
+        Ensure the redirect response is coming from the Network before adding it to the cache.
+        (WebKit::NetworkResourceLoader::dispatchWillSendRequestForCacheEntry):
+        Call willSendRedirectedRequest to make sure the cached redirect is validated.
+        * NetworkProcess/NetworkResourceLoader.h:
+
 2018-05-23  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] WebDriver: implement AutomationSessionClient::didDisconnectFromRemote
index 8e9c6f7..fdcf7fe 100644 (file)
@@ -108,12 +108,12 @@ void NetworkLoadChecker::checkRedirection(ResourceResponse& redirectResponse, Re
     auto error = validateResponse(redirectResponse);
     if (!error.isNull()) {
         auto errorMessage = makeString("Cross-origin redirection to ", request.url().string(), " denied by Cross-Origin Resource Sharing policy: ", error.localizedDescription());
-        handler(makeUnexpected(ResourceError { String { }, 0, request.url(), WTFMove(errorMessage), ResourceError::Type::AccessControl }));
+        handler(makeUnexpected(ResourceError { String { }, 0, redirectResponse.url(), WTFMove(errorMessage), ResourceError::Type::AccessControl }));
         return;
     }
 
     if (m_options.redirect != FetchOptions::Redirect::Follow) {
-        handler(accessControlErrorForValidationHandler(ASCIILiteral("Redirections are not allowed")));
+        handler(accessControlErrorForValidationHandler(makeString("Not allowed to follow a redirection while loading ", redirectResponse.url().string())));
         return;
     }
 
index b8025f3..f44b7f5 100644 (file)
@@ -230,7 +230,7 @@ void NetworkResourceLoader::retrieveCacheEntry(const ResourceRequest& request)
         }
         if (entry->redirectRequest()) {
             RELEASE_LOG_IF_ALLOWED("retrieveCacheEntry: Handling redirect (pageID = %" PRIu64 ", frameID = %" PRIu64 ", resourceID = %" PRIu64 ", isMainResource = %d, isSynchronous = %d)", m_parameters.webPageID, m_parameters.webFrameID, m_parameters.identifier, isMainResource(), isSynchronous());
-            loader->dispatchWillSendRequestForCacheEntry(WTFMove(entry));
+            loader->dispatchWillSendRequestForCacheEntry(WTFMove(request), WTFMove(entry));
             return;
         }
         if (loader->m_parameters.needsCertificateInfo && !entry->response().certificateInfo()) {
@@ -615,6 +615,9 @@ void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request,
 {
     ++m_redirectCount;
 
+    if (redirectResponse.source() == ResourceResponse::Source::Network && canUseCachedRedirect(request))
+        m_cache->storeRedirect(request, redirectResponse, redirectRequest);
+
     if (m_networkLoadChecker) {
         m_networkLoadChecker->checkRedirection(redirectResponse, WTFMove(redirectRequest), [protectedThis = makeRef(*this), this, storedCredentialsPolicy = m_networkLoadChecker->storedCredentialsPolicy(), request = WTFMove(request), redirectResponse](auto&& result) mutable {
             if (!result.has_value()) {
@@ -622,9 +625,7 @@ void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request,
                     return;
 
                 if (m_parameters.options.redirect == FetchOptions::Redirect::Manual) {
-                    redirectResponse.setType(ResourceResponse::Type::Opaqueredirect);
-                    this->didReceiveResponse(WTFMove(redirectResponse));
-                    this->didFinishLoading({ });
+                    this->didFinishWithRedirectResponse(WTFMove(redirectResponse));
                     return;
                 }
 
@@ -634,7 +635,8 @@ void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request,
 
             if (storedCredentialsPolicy != m_networkLoadChecker->storedCredentialsPolicy()) {
                 // We need to restart the load to update the session according the new credential policy.
-                m_networkLoad->cancel();
+                if (m_networkLoad)
+                    m_networkLoad->cancel();
                 auto request = WTFMove(result.value());
                 m_networkLoadChecker->prepareRedirectedRequest(request);
 
@@ -656,13 +658,10 @@ void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request,
     continueWillSendRedirectedRequest(WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse));
 }
 
-void NetworkResourceLoader::continueWillSendRedirectedRequest(WebCore::ResourceRequest&& request, WebCore::ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse)
+void NetworkResourceLoader::continueWillSendRedirectedRequest(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse)
 {
     ASSERT(!isSynchronous());
 
-    if (canUseCachedRedirect(request))
-        m_cache->storeRedirect(request, redirectResponse, redirectRequest);
-
     if (m_parameters.shouldEnableFromOriginResponseHeader && shouldCancelCrossOriginLoad(redirectResponse, m_parameters.frameAncestorOrigins) && m_networkLoad) {
         didFailLoading(fromOriginResourceError(redirectResponse.url()));
         return;
@@ -671,6 +670,20 @@ void NetworkResourceLoader::continueWillSendRedirectedRequest(WebCore::ResourceR
     send(Messages::WebResourceLoader::WillSendRequest(redirectRequest, sanitizeResponseIfPossible(WTFMove(redirectResponse), ResourceResponse::SanitizationType::Redirection)));
 }
 
+void NetworkResourceLoader::didFinishWithRedirectResponse(ResourceResponse&& redirectResponse)
+{
+    redirectResponse.setType(ResourceResponse::Type::Opaqueredirect);
+    didReceiveResponse(WTFMove(redirectResponse));
+
+    WebCore::NetworkLoadMetrics networkLoadMetrics;
+    networkLoadMetrics.markComplete();
+    networkLoadMetrics.responseBodyBytesReceived = 0;
+    networkLoadMetrics.responseBodyDecodedSize = 0;
+    send(Messages::WebResourceLoader::DidFinishResourceLoad { networkLoadMetrics });
+
+    cleanup(LoadResult::Success);
+}
+
 ResourceResponse NetworkResourceLoader::sanitizeResponseIfPossible(ResourceResponse&& response, ResourceResponse::SanitizationType type)
 {
     if (m_parameters.shouldRestrictHTTPResponseAccess)
@@ -903,22 +916,15 @@ void NetworkResourceLoader::validateCacheEntry(std::unique_ptr<NetworkCache::Ent
     startNetworkLoad(WTFMove(revalidationRequest), FirstLoad::Yes);
 }
 
-void NetworkResourceLoader::dispatchWillSendRequestForCacheEntry(std::unique_ptr<NetworkCache::Entry> entry)
+void NetworkResourceLoader::dispatchWillSendRequestForCacheEntry(ResourceRequest&& request, std::unique_ptr<NetworkCache::Entry>&& entry)
 {
     ASSERT(entry->redirectRequest());
     ASSERT(!m_isWaitingContinueWillSendRequestForCachedRedirect);
 
     LOG(NetworkCache, "(NetworkProcess) Executing cached redirect");
 
-    auto& response = entry->response();
-    if (m_parameters.shouldEnableFromOriginResponseHeader && shouldCancelCrossOriginLoad(response, m_parameters.frameAncestorOrigins) && m_networkLoad) {
-        didFailLoading(fromOriginResourceError(response.url()));
-        return;
-    }
-
-    ++m_redirectCount;
-    send(Messages::WebResourceLoader::WillSendRequest { *entry->redirectRequest(), sanitizeResponseIfPossible(ResourceResponse { response }, ResourceResponse::SanitizationType::Redirection) });
     m_isWaitingContinueWillSendRequestForCachedRedirect = true;
+    willSendRedirectedRequest(WTFMove(request), ResourceRequest { *entry->redirectRequest() }, ResourceResponse { entry->response() });
 }
 
 IPC::Connection* NetworkResourceLoader::messageSenderConnection()
index 6197876..ec69788 100644 (file)
@@ -137,7 +137,7 @@ private:
     void didRetrieveCacheEntry(std::unique_ptr<NetworkCache::Entry>);
     void sendResultForCacheEntry(std::unique_ptr<NetworkCache::Entry>);
     void validateCacheEntry(std::unique_ptr<NetworkCache::Entry>);
-    void dispatchWillSendRequestForCacheEntry(std::unique_ptr<NetworkCache::Entry>);
+    void dispatchWillSendRequestForCacheEntry(WebCore::ResourceRequest&&, std::unique_ptr<NetworkCache::Entry>&&);
     void continueProcessingCachedEntryAfterDidReceiveResponse(std::unique_ptr<NetworkCache::Entry>);
 
     bool shouldInterruptLoadForXFrameOptions(const String&, const WebCore::URL&);
@@ -169,7 +169,7 @@ private:
 #endif
 
     void continueWillSendRedirectedRequest(WebCore::ResourceRequest&& request, WebCore::ResourceRequest&& redirectRequest, WebCore::ResourceResponse&&);
-
+    void didFinishWithRedirectResponse(WebCore::ResourceResponse&&);
     WebCore::ResourceResponse sanitizeResponseIfPossible(WebCore::ResourceResponse&&, WebCore::ResourceResponse::SanitizationType);
 
     // ContentSecurityPolicyClient