Make fetch() use "same-origin" credentials by default
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jul 2018 02:50:58 +0000 (02:50 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jul 2018 02:50:58 +0000 (02:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176023

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

Tests updated according upstream WPT repo.

* web-platform-tests/fetch/api/request/request-init-003.sub.html:
* web-platform-tests/fetch/api/request/request-structure.html:

Source/WebCore:

Covered by updated tests.

* Modules/fetch/FetchRequest.cpp:
(WebCore::FetchRequest::initializeWith):
Setting credentials mode to same-origin for FetchRequest by default.
* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::redirectReceived):
Handle correctly referrer in case we restart a load.
* page/PerformanceResourceTiming.cpp:
(WebCore::entryStartTime):
(WebCore::entryEndTime):
In case it is not allowed to disclose resource timing info, update as
https://www.w3.org/TR/resource-timing-1/#performanceresourcetiming

Source/WebKit:

Before the patch, when changing the credential mode in case of redirection,
we were not waiting for WebProcess response to restart the load.
This patch updates the implementation to ask the WebProcess whether to proceed as for other regular asynchronous loads.
This requires some refactoring in particular we now pass request, redirectRequest and redirectResponse to NetworkLoadChecker
that will send them back as part of the completion handler.

To do so, we change manual redirection handling and make it a successful case and not an error case as before.

* NetworkProcess/NetworkLoadChecker.cpp:
(WebKit::redirectionError):
(WebKit::NetworkLoadChecker::checkRedirection):
* NetworkProcess/NetworkLoadChecker.h:
* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::willSendRedirectedRequest):
(WebKit::NetworkResourceLoader::restartNetworkLoad):
(WebKit::NetworkResourceLoader::continueWillSendRequest):
* NetworkProcess/NetworkResourceLoader.h:
* NetworkProcess/PingLoad.cpp:
(WebKit::PingLoad::willPerformHTTPRedirection):

LayoutTests:

Resource timing does not work properly on WK1 when stopping fetch/XHR load to restart it without credentials.
Updated expected results accordingly.

* http/tests/inspector/network/resource-mime-type.html:
Update resource-mime-type.html to ensure we go to the network for every load.
* http/wpt/resource-timing/rt-cors.js:
(assertRedirectWithDisallowedTimingData):
Updated test according https://www.w3.org/TR/resource-timing-1/#performanceresourcetiming.
* platform/mac-wk1/http/wpt/resource-timing/rt-cors-expected.txt: Added.
* platform/mac-wk1/http/wpt/resource-timing/rt-cors.worker-expected.txt: Added.
* platform/win/http/wpt/resource-timing/rt-cors-expected.txt: Added.
* platform/win/http/wpt/resource-timing/rt-cors.worker-expected.txt: Added.

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

20 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/inspector/network/resource-mime-type.html
LayoutTests/http/wpt/resource-timing/rt-cors.js
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/fetch/api/request/request-init-003.sub.html
LayoutTests/imported/w3c/web-platform-tests/fetch/api/request/request-structure.html
LayoutTests/platform/mac-wk1/http/wpt/resource-timing/rt-cors-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-wk1/http/wpt/resource-timing/rt-cors.worker-expected.txt [new file with mode: 0644]
LayoutTests/platform/win/http/wpt/resource-timing/rt-cors-expected.txt [new file with mode: 0644]
LayoutTests/platform/win/http/wpt/resource-timing/rt-cors.worker-expected.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/fetch/FetchRequest.cpp
Source/WebCore/loader/DocumentThreadableLoader.cpp
Source/WebCore/page/PerformanceResourceTiming.cpp
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
Source/WebKit/NetworkProcess/NetworkLoadChecker.h
Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
Source/WebKit/NetworkProcess/NetworkResourceLoader.h
Source/WebKit/NetworkProcess/PingLoad.cpp

index e64e16a..e165808 100644 (file)
@@ -1,3 +1,23 @@
+2018-07-10  Youenn Fablet  <youenn@apple.com>
+
+        Make fetch() use "same-origin" credentials by default
+        https://bugs.webkit.org/show_bug.cgi?id=176023
+
+        Reviewed by Chris Dumez.
+
+        Resource timing does not work properly on WK1 when stopping fetch/XHR load to restart it without credentials.
+        Updated expected results accordingly.
+
+        * http/tests/inspector/network/resource-mime-type.html:
+        Update resource-mime-type.html to ensure we go to the network for every load.
+        * http/wpt/resource-timing/rt-cors.js:
+        (assertRedirectWithDisallowedTimingData):
+        Updated test according https://www.w3.org/TR/resource-timing-1/#performanceresourcetiming.
+        * platform/mac-wk1/http/wpt/resource-timing/rt-cors-expected.txt: Added.
+        * platform/mac-wk1/http/wpt/resource-timing/rt-cors.worker-expected.txt: Added.
+        * platform/win/http/wpt/resource-timing/rt-cors-expected.txt: Added.
+        * platform/win/http/wpt/resource-timing/rt-cors.worker-expected.txt: Added.
+
 2018-07-10  Saam Barati  <sbarati@apple.com>
 
         Layout Test editing/selection/navigation-clears-editor-state.html is flaky
index faa1206..246748d 100644 (file)
@@ -30,12 +30,12 @@ function loadImageWithURL(url) {
 
 function loadXHRWithURL(url) {
     let xhr = new XMLHttpRequest;
-    xhr.open("GET", url, true);
+    xhr.open("GET", url + "?xhr", true);
     xhr.send();
 }
 
 function loadFetchWithURL(url) {
-    fetch(url);
+    fetch(url + "?fetch");
 }
 
 function test()
index 50867e3..9173f43 100644 (file)
@@ -23,7 +23,7 @@ function assertRedirectWithDisallowedTimingData(entry) {
     assertAlways(entry);
     assert_equals(entry.redirectStart, 0, "entry should not have a redirectStart time");
     assert_equals(entry.redirectEnd, 0, "entry should not have a redirectEnd time");
-    assert_not_equals(entry.startTime, entry.fetchStart, "entry startTime should have matched redirectStart but it was disallowed so it should not match fetchStart");
+    assert_equals(entry.startTime, entry.fetchStart, "entry startTime should have matched redirectStart but it was disallowed so it should match fetchStart");
 }
 
 function assertNonRedirectTimingData(entry) {
index 4dde95f..e565347 100644 (file)
@@ -1,3 +1,15 @@
+2018-07-10  Youenn Fablet  <youenn@apple.com>
+
+        Make fetch() use "same-origin" credentials by default
+        https://bugs.webkit.org/show_bug.cgi?id=176023
+
+        Reviewed by Chris Dumez.
+
+        Tests updated according upstream WPT repo.
+
+        * web-platform-tests/fetch/api/request/request-init-003.sub.html:
+        * web-platform-tests/fetch/api/request/request-structure.html:
+
 2018-07-09  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Make WPT test at animation-model/keyframe-effects/effect-value-overlapping-keyframes.html pass reliably
index 507007f..79c91cd 100644 (file)
@@ -44,7 +44,7 @@
                              "referrer" : "about:client",
                              "referrerPolicy" : "",
                              "mode" : "cors",
-                             "credentials" : "omit",
+                             "credentials" : "same-origin",
                              "cache" : "default",
                              "redirect" : "follow",
                              "integrity" : "",
index 98c2451..ccdb707 100644 (file)
@@ -76,7 +76,7 @@
             break;
 
           case "credentials":
-            defaultValue = "omit";
+            defaultValue = "same-origin";
             newValue = "cors";
             break;
 
diff --git a/LayoutTests/platform/mac-wk1/http/wpt/resource-timing/rt-cors-expected.txt b/LayoutTests/platform/mac-wk1/http/wpt/resource-timing/rt-cors-expected.txt
new file mode 100644 (file)
index 0000000..a4f1f34
--- /dev/null
@@ -0,0 +1,27 @@
+Resource Timing: CORS requests
+
+
+PASS Same Origin request must have timing data 
+PASS Cross Origin resource without Timing-Allow-Origin must have filtered timing data 
+PASS Cross Origin resource with Timing-Allow-Origin null value must have filtered timing data 
+PASS Cross Origin resource with Timing-Allow-Origin wildcard must have timing data 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (only entry) 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, comma separated) 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, multiple headers) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, comma separated) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, multiple headers) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (case-sensitive) 
+PASS Redirect to Same Origin request must have timing data 
+PASS Redirect to Cross Origin resource without Timing-Allow-Origin must have filtered timing data 
+PASS Redirect to Cross Origin resource with Timing-Allow-Origin null value must have filtered timing data 
+FAIL Redirect to Cross Origin resource with Timing-Allow-Origin wildcard must have timing data assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (only entry) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, comma separated) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, multiple headers) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, comma separated) 
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, multiple headers) 
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (case-sensitive) 
+PASS Multiple level redirect to Same Origin resource must have timing data 
+PASS Multiple level redirect to Cross Origin resource without Timing-Allow-Origin must have must have filtered timing data 
+FAIL Multiple level redirect to Cross Origin resource with Timing-Allow-Origin must have must have timing data assert_not_equals: entry should have a redirectStart time got disallowed value 0
+
diff --git a/LayoutTests/platform/mac-wk1/http/wpt/resource-timing/rt-cors.worker-expected.txt b/LayoutTests/platform/mac-wk1/http/wpt/resource-timing/rt-cors.worker-expected.txt
new file mode 100644 (file)
index 0000000..b86f6b3
--- /dev/null
@@ -0,0 +1,26 @@
+
+PASS Must have PerformanceObserver and PerformanceResourceTiming in DedicatedWorkerGlobalScope 
+PASS Same Origin request must have timing data 
+PASS Cross Origin resource without Timing-Allow-Origin must have filtered timing data 
+PASS Cross Origin resource with Timing-Allow-Origin null value must have filtered timing data 
+PASS Cross Origin resource with Timing-Allow-Origin wildcard must have timing data 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (only entry) 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, comma separated) 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, multiple headers) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, comma separated) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, multiple headers) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (case-sensitive) 
+PASS Redirect to Same Origin request must have timing data 
+PASS Redirect to Cross Origin resource without Timing-Allow-Origin must have filtered timing data 
+PASS Redirect to Cross Origin resource with Timing-Allow-Origin null value must have filtered timing data 
+FAIL Redirect to Cross Origin resource with Timing-Allow-Origin wildcard must have timing data assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (only entry) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, comma separated) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, multiple headers) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, comma separated) 
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, multiple headers) 
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (case-sensitive) 
+PASS Multiple level redirect to Same Origin resource must have timing data 
+PASS Multiple level redirect to Cross Origin resource without Timing-Allow-Origin must have must have filtered timing data 
+FAIL Multiple level redirect to Cross Origin resource with Timing-Allow-Origin must have must have timing data assert_not_equals: entry should have a redirectStart time got disallowed value 0
+
diff --git a/LayoutTests/platform/win/http/wpt/resource-timing/rt-cors-expected.txt b/LayoutTests/platform/win/http/wpt/resource-timing/rt-cors-expected.txt
new file mode 100644 (file)
index 0000000..a4f1f34
--- /dev/null
@@ -0,0 +1,27 @@
+Resource Timing: CORS requests
+
+
+PASS Same Origin request must have timing data 
+PASS Cross Origin resource without Timing-Allow-Origin must have filtered timing data 
+PASS Cross Origin resource with Timing-Allow-Origin null value must have filtered timing data 
+PASS Cross Origin resource with Timing-Allow-Origin wildcard must have timing data 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (only entry) 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, comma separated) 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, multiple headers) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, comma separated) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, multiple headers) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (case-sensitive) 
+PASS Redirect to Same Origin request must have timing data 
+PASS Redirect to Cross Origin resource without Timing-Allow-Origin must have filtered timing data 
+PASS Redirect to Cross Origin resource with Timing-Allow-Origin null value must have filtered timing data 
+FAIL Redirect to Cross Origin resource with Timing-Allow-Origin wildcard must have timing data assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (only entry) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, comma separated) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, multiple headers) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, comma separated) 
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, multiple headers) 
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (case-sensitive) 
+PASS Multiple level redirect to Same Origin resource must have timing data 
+PASS Multiple level redirect to Cross Origin resource without Timing-Allow-Origin must have must have filtered timing data 
+FAIL Multiple level redirect to Cross Origin resource with Timing-Allow-Origin must have must have timing data assert_not_equals: entry should have a redirectStart time got disallowed value 0
+
diff --git a/LayoutTests/platform/win/http/wpt/resource-timing/rt-cors.worker-expected.txt b/LayoutTests/platform/win/http/wpt/resource-timing/rt-cors.worker-expected.txt
new file mode 100644 (file)
index 0000000..b86f6b3
--- /dev/null
@@ -0,0 +1,26 @@
+
+PASS Must have PerformanceObserver and PerformanceResourceTiming in DedicatedWorkerGlobalScope 
+PASS Same Origin request must have timing data 
+PASS Cross Origin resource without Timing-Allow-Origin must have filtered timing data 
+PASS Cross Origin resource with Timing-Allow-Origin null value must have filtered timing data 
+PASS Cross Origin resource with Timing-Allow-Origin wildcard must have timing data 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (only entry) 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, comma separated) 
+PASS Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, multiple headers) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, comma separated) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, multiple headers) 
+PASS Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (case-sensitive) 
+PASS Redirect to Same Origin request must have timing data 
+PASS Redirect to Cross Origin resource without Timing-Allow-Origin must have filtered timing data 
+PASS Redirect to Cross Origin resource with Timing-Allow-Origin null value must have filtered timing data 
+FAIL Redirect to Cross Origin resource with Timing-Allow-Origin wildcard must have timing data assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (only entry) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, comma separated) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+FAIL Redirect to Cross Origin resource with origin in Timing-Allow-Origin list must have timing data (middle entry, multiple headers) assert_not_equals: entry should have a redirectStart time got disallowed value 0
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, comma separated) 
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (other origins, multiple headers) 
+PASS Redirect to Cross Origin resource with origin not in Timing-Allow-Origin list must have filtered timing data (case-sensitive) 
+PASS Multiple level redirect to Same Origin resource must have timing data 
+PASS Multiple level redirect to Cross Origin resource without Timing-Allow-Origin must have must have filtered timing data 
+FAIL Multiple level redirect to Cross Origin resource with Timing-Allow-Origin must have must have timing data assert_not_equals: entry should have a redirectStart time got disallowed value 0
+
index a73bcad..5695106 100644 (file)
@@ -1,3 +1,24 @@
+2018-07-10  Youenn Fablet  <youenn@apple.com>
+
+        Make fetch() use "same-origin" credentials by default
+        https://bugs.webkit.org/show_bug.cgi?id=176023
+
+        Reviewed by Chris Dumez.
+
+        Covered by updated tests.
+
+        * Modules/fetch/FetchRequest.cpp:
+        (WebCore::FetchRequest::initializeWith):
+        Setting credentials mode to same-origin for FetchRequest by default.
+        * loader/DocumentThreadableLoader.cpp:
+        (WebCore::DocumentThreadableLoader::redirectReceived):
+        Handle correctly referrer in case we restart a load.
+        * page/PerformanceResourceTiming.cpp:
+        (WebCore::entryStartTime):
+        (WebCore::entryEndTime):
+        In case it is not allowed to disclose resource timing info, update as
+        https://www.w3.org/TR/resource-timing-1/#performanceresourcetiming
+
 2018-07-10  Chris Dumez  <cdumez@apple.com>
 
         "serviceworker.js" is fetched several times in a row
index cb7b5f9..7cbc632 100644 (file)
@@ -151,7 +151,7 @@ ExceptionOr<void> FetchRequest::initializeWith(const String& url, Init&& init)
         return Exception { TypeError, "URL is not valid or contains user credentials."_s };
 
     m_options.mode = Mode::Cors;
-    m_options.credentials = Credentials::Omit;
+    m_options.credentials = Credentials::SameOrigin;
     m_referrer = "client"_s;
     m_request.setURL(requestURL);
     m_request.setRequester(ResourceRequest::Requester::Fetch);
index 0a1b2b0..1c2d1dc 100644 (file)
@@ -320,6 +320,10 @@ void DocumentThreadableLoader::redirectReceived(CachedResource& resource, Resour
 
     clearResource();
 
+    m_referrer = request.httpReferrer();
+    if (m_referrer.isNull())
+        m_options.referrerPolicy = ReferrerPolicy::NoReferrer;
+
     // Let's fetch the request with the original headers (equivalent to request cloning specified by fetch algorithm).
     // Do not copy the Authorization header if removed by the network layer.
     if (!request.httpHeaderFields().contains(HTTPHeaderName::Authorization))
index f8f5839..5753358 100644 (file)
@@ -54,11 +54,17 @@ static double monotonicTimeToDOMHighResTimeStamp(MonotonicTime timeOrigin, Monot
 
 static double entryStartTime(MonotonicTime timeOrigin, const ResourceTiming& resourceTiming)
 {
+    if (!resourceTiming.allowTimingDetails())
+        return monotonicTimeToDOMHighResTimeStamp(timeOrigin, resourceTiming.loadTiming().fetchStart());
+
     return monotonicTimeToDOMHighResTimeStamp(timeOrigin, resourceTiming.loadTiming().startTime());
 }
 
 static double entryEndTime(MonotonicTime timeOrigin, const ResourceTiming& resourceTiming)
 {
+    if (!resourceTiming.allowTimingDetails())
+        return entryStartTime(timeOrigin, resourceTiming);
+
     if (resourceTiming.networkLoadMetrics().isComplete()) {
         Seconds endTime = (resourceTiming.loadTiming().fetchStart() + resourceTiming.networkLoadMetrics().responseEnd) - timeOrigin;
         return Performance::reduceTimeResolution(endTime).milliseconds();
index c90b79a..9231fa6 100644 (file)
@@ -1,3 +1,30 @@
+2018-07-10  Youenn Fablet  <youenn@apple.com>
+
+        Make fetch() use "same-origin" credentials by default
+        https://bugs.webkit.org/show_bug.cgi?id=176023
+
+        Reviewed by Chris Dumez.
+
+        Before the patch, when changing the credential mode in case of redirection,
+        we were not waiting for WebProcess response to restart the load.
+        This patch updates the implementation to ask the WebProcess whether to proceed as for other regular asynchronous loads.
+        This requires some refactoring in particular we now pass request, redirectRequest and redirectResponse to NetworkLoadChecker
+        that will send them back as part of the completion handler.
+
+        To do so, we change manual redirection handling and make it a successful case and not an error case as before.
+
+        * NetworkProcess/NetworkLoadChecker.cpp:
+        (WebKit::redirectionError):
+        (WebKit::NetworkLoadChecker::checkRedirection):
+        * NetworkProcess/NetworkLoadChecker.h:
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::willSendRedirectedRequest):
+        (WebKit::NetworkResourceLoader::restartNetworkLoad):
+        (WebKit::NetworkResourceLoader::continueWillSendRequest):
+        * NetworkProcess/NetworkResourceLoader.h:
+        * NetworkProcess/PingLoad.cpp:
+        (WebKit::PingLoad::willPerformHTTPRedirection):
+
 2018-07-10  Chris Dumez  <cdumez@apple.com>
 
         [IOS] We should prevent WebProcess suspension while the UIProcess is waiting for a reply from its injected bundle
index 7bb2b5d..a87bdeb 100644 (file)
@@ -102,19 +102,27 @@ void NetworkLoadChecker::prepareRedirectedRequest(ResourceRequest& request)
         request.setHTTPHeaderField(HTTPHeaderName::DNT, m_dntHeaderValue);
 }
 
-void NetworkLoadChecker::checkRedirection(ResourceResponse& redirectResponse, ResourceRequest&& request, ContentSecurityPolicyClient* client, ValidationHandler&& handler)
+static inline NetworkLoadChecker::RedirectionRequestOrError redirectionError(const ResourceResponse& redirectResponse, String&& errorMessage)
+{
+    return makeUnexpected(ResourceError { String { }, 0, redirectResponse.url(), WTFMove(errorMessage), ResourceError::Type::AccessControl });
+}
+
+void NetworkLoadChecker::checkRedirection(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse, ContentSecurityPolicyClient* client, RedirectionValidationHandler&& handler)
 {
     ASSERT(!isChecking());
 
     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, redirectResponse.url(), WTFMove(errorMessage), ResourceError::Type::AccessControl }));
+        handler(redirectionError(redirectResponse, makeString("Cross-origin redirection to ", redirectRequest.url().string(), " denied by Cross-Origin Resource Sharing policy: ", error.localizedDescription())));
         return;
     }
 
-    if (m_options.redirect != FetchOptions::Redirect::Follow) {
-        handler(accessControlErrorForValidationHandler(makeString("Not allowed to follow a redirection while loading ", redirectResponse.url().string())));
+    if (m_options.redirect == FetchOptions::Redirect::Error) {
+        handler(redirectionError(redirectResponse, makeString("Not allowed to follow a redirection while loading ", redirectResponse.url().string())));
+        return;
+    }
+    if (m_options.redirect == FetchOptions::Redirect::Manual) {
+        handler(RedirectionTriplet { WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse) });
         return;
     }
 
@@ -122,14 +130,20 @@ void NetworkLoadChecker::checkRedirection(ResourceResponse& redirectResponse, Re
     // See https://github.com/whatwg/fetch/issues/393
 
     if (++m_redirectCount > 20) {
-        handler(accessControlErrorForValidationHandler("Load cannot follow more than 20 redirections"_s));
+        handler(redirectionError(redirectResponse, "Load cannot follow more than 20 redirections"_s));
         return;
     }
 
     m_previousURL = WTFMove(m_url);
-    m_url = request.url();
+    m_url = redirectRequest.url();
 
-    checkRequest(WTFMove(request), client, WTFMove(handler));
+    checkRequest(WTFMove(redirectRequest), client, [handler = WTFMove(handler), request = WTFMove(request), redirectResponse = WTFMove(redirectResponse)](auto&& result) mutable {
+        if (!result.has_value()) {
+            handler(makeUnexpected(WTFMove(result.error())));
+            return;
+        }
+        handler(RedirectionTriplet { WTFMove(request), WTFMove(result.value()), WTFMove(redirectResponse) });
+    });
 }
 
 ResourceError NetworkLoadChecker::validateResponse(ResourceResponse& response)
index d1070f8..6801768 100644 (file)
@@ -53,7 +53,16 @@ public:
     using RequestOrError = Expected<WebCore::ResourceRequest, WebCore::ResourceError>;
     using ValidationHandler = CompletionHandler<void(RequestOrError&&)>;
     void check(WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, ValidationHandler&&);
-    void checkRedirection(WebCore::ResourceResponse&, WebCore::ResourceRequest&&, WebCore::ContentSecurityPolicyClient*, ValidationHandler&&);
+
+    struct RedirectionTriplet {
+        WebCore::ResourceRequest request;
+        WebCore::ResourceRequest redirectRequest;
+        WebCore::ResourceResponse redirectResponse;
+    };
+    using RedirectionRequestOrError = Expected<RedirectionTriplet, WebCore::ResourceError>;
+    using RedirectionValidationHandler = CompletionHandler<void(RedirectionRequestOrError&&)>;
+    void checkRedirection(WebCore::ResourceRequest&& request, WebCore::ResourceRequest&& redirectRequest, WebCore::ResourceResponse&& redirectResponse, WebCore::ContentSecurityPolicyClient*, RedirectionValidationHandler&&);
+
     void prepareRedirectedRequest(WebCore::ResourceRequest&);
 
     WebCore::ResourceError validateResponse(WebCore::ResourceResponse&);
index 058211a..52ebf9a 100644 (file)
@@ -581,39 +581,35 @@ void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request,
 
     if (m_networkLoadChecker) {
         m_networkLoadChecker->storeRedirectionIfNeeded(request, redirectResponse);
-        m_networkLoadChecker->checkRedirection(redirectResponse, WTFMove(redirectRequest), this, [protectedThis = makeRef(*this), this, storedCredentialsPolicy = m_networkLoadChecker->storedCredentialsPolicy(), request = WTFMove(request), redirectResponse](auto&& result) mutable {
+        m_networkLoadChecker->checkRedirection(WTFMove(request), WTFMove(redirectRequest), WTFMove(redirectResponse), this, [protectedThis = makeRef(*this), this, storedCredentialsPolicy = m_networkLoadChecker->storedCredentialsPolicy()](auto&& result) mutable {
             if (!result.has_value()) {
                 if (result.error().isCancellation())
                     return;
 
-                if (m_parameters.options.redirect == FetchOptions::Redirect::Manual) {
-                    this->didFinishWithRedirectResponse(WTFMove(redirectResponse));
-                    return;
-                }
-
                 this->didFailLoading(result.error());
                 return;
             }
 
-            if (storedCredentialsPolicy != m_networkLoadChecker->storedCredentialsPolicy()) {
-                // We need to restart the load to update the session according the new credential policy.
-                if (m_networkLoad)
-                    m_networkLoad->cancel();
-                auto request = WTFMove(result.value());
-                m_networkLoadChecker->prepareRedirectedRequest(request);
-
-                this->startNetworkLoad(WTFMove(request), FirstLoad::No);
+            if (m_parameters.options.redirect == FetchOptions::Redirect::Manual) {
+                this->didFinishWithRedirectResponse(WTFMove(result->redirectResponse));
                 return;
             }
 
             if (this->isSynchronous()) {
+                if (storedCredentialsPolicy != m_networkLoadChecker->storedCredentialsPolicy()) {
+                    // We need to restart the load to update the session according the new credential policy.
+                    this->restartNetworkLoad(WTFMove(result->redirectRequest));
+                    return;
+                }
+
                 // We do not support prompting for credentials for synchronous loads. If we ever change this policy then
                 // we need to take care to prompt if and only if request and redirectRequest are not mixed content.
-                this->continueWillSendRequest(WTFMove(result.value()), false);
+                this->continueWillSendRequest(WTFMove(result->redirectRequest), false);
                 return;
             }
 
-            this->continueWillSendRedirectedRequest(WTFMove(request), WTFMove(result.value()), WTFMove(redirectResponse));
+            m_shouldRestartLoad = storedCredentialsPolicy != m_networkLoadChecker->storedCredentialsPolicy();
+            this->continueWillSendRedirectedRequest(WTFMove(result->request), WTFMove(result->redirectRequest), WTFMove(result->redirectResponse));
         });
         return;
     }
@@ -649,8 +645,24 @@ ResourceResponse NetworkResourceLoader::sanitizeResponseIfPossible(ResourceRespo
     return WTFMove(response);
 }
 
+void NetworkResourceLoader::restartNetworkLoad(WebCore::ResourceRequest&& newRequest)
+{
+    if (m_networkLoad)
+        m_networkLoad->cancel();
+    if (m_networkLoadChecker)
+        m_networkLoadChecker->prepareRedirectedRequest(newRequest);
+
+    startNetworkLoad(WTFMove(newRequest), FirstLoad::No);
+}
+
 void NetworkResourceLoader::continueWillSendRequest(ResourceRequest&& newRequest, bool isAllowedToAskUserForCredentials)
 {
+    if (m_shouldRestartLoad) {
+        m_shouldRestartLoad = false;
+        restartNetworkLoad(WTFMove(newRequest));
+        return;
+    }
+
     if (m_networkLoadChecker) {
         // FIXME: We should be doing this check when receiving the redirection and not allow about protocol as per fetch spec.
         if (!newRequest.url().protocolIsInHTTPFamily() && !newRequest.url().isBlankURL() && m_redirectCount) {
index 931f65e..672c132 100644 (file)
@@ -145,6 +145,7 @@ private:
 
     enum class FirstLoad { No, Yes };
     void startNetworkLoad(WebCore::ResourceRequest&&, FirstLoad);
+    void restartNetworkLoad(WebCore::ResourceRequest&&);
     void continueDidReceiveResponse();
 
     enum class LoadResult {
@@ -210,6 +211,7 @@ private:
     bool m_isWaitingContinueWillSendRequestForCachedRedirect { false };
     std::unique_ptr<NetworkCache::Entry> m_cacheEntryWaitingForContinueDidReceiveResponse;
     std::unique_ptr<NetworkLoadChecker> m_networkLoadChecker;
+    bool m_shouldRestartLoad { false };
 
     std::optional<NetworkActivityTracker> m_networkActivityTracker;
 };
index 8710cb3..626d1f4 100644 (file)
@@ -93,17 +93,17 @@ void PingLoad::loadRequest(ResourceRequest&& request)
 
 void PingLoad::willPerformHTTPRedirection(ResourceResponse&& redirectResponse, ResourceRequest&& request, RedirectCompletionHandler&& completionHandler)
 {
-    m_networkLoadChecker->checkRedirection(redirectResponse, WTFMove(request), nullptr, [this, completionHandler = WTFMove(completionHandler)](auto&& result) {
+    m_networkLoadChecker->checkRedirection(ResourceRequest { }, WTFMove(request), WTFMove(redirectResponse), nullptr, [this, completionHandler = WTFMove(completionHandler)](auto&& result) {
         if (!result.has_value()) {
             completionHandler({ });
             this->didFinish(result.error());
             return;
         }
-        auto request = WTFMove(result.value());
+        auto request = WTFMove(result->redirectRequest);
         m_networkLoadChecker->prepareRedirectedRequest(request);
 
-        if (!result.value().url().protocolIsInHTTPFamily()) {
-            this->didFinish(ResourceError { String { }, 0, result.value().url(), "Redirection to URL with a scheme that is not HTTP(S)"_s, ResourceError::Type::AccessControl });
+        if (!request.url().protocolIsInHTTPFamily()) {
+            this->didFinish(ResourceError { String { }, 0, request.url(), "Redirection to URL with a scheme that is not HTTP(S)"_s, ResourceError::Type::AccessControl });
             return;
         }