We should use "error" redirect mode for fetching service worker scripts

https://bugs.webkit.org/show_bug.cgi?id=180950

Reviewed by Youenn Fablet.

LayoutTests/imported/w3c:

Rebaseline test now that behavior has changed. Note that we are still failing becuase we reject
the registration promise with a TypeError instead of a SecurityError. I cannot find any reason
to throw a SecurityError here based on the specification though, so the test may not match the
specification.

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

Source/WebCore:

We should use "error" redirect mode for fetching service worker scripts, as per:
- https://w3c.github.io/ServiceWorker/#update (Step 7.5)

No new tests, rebaselined existing test.

* loader/SubresourceLoader.cpp:
(WebCore::SubresourceLoader::willSendRequestInternal):
* workers/Worker.cpp:
(WebCore::Worker::create):
* workers/WorkerScriptLoader.cpp:
(WebCore::WorkerScriptLoader::loadAsynchronously):
* workers/WorkerScriptLoader.h:
* workers/service/ServiceWorkerJob.cpp:
(WebCore::ServiceWorkerJob::fetchScriptWithContext):

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

diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index e3ff94b..f6bafa8 100644 (file)
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -207,6 +207,8 @@ imported/w3c/web-platform-tests/workers/Worker_cross_origin_security_err.htm [ S
 imported/w3c/web-platform-tests/workers/constructors/SharedWorker [ Skip ]
 imported/w3c/web-platform-tests/workers/SharedWorker_blobUrl.html [ Skip ]
 
+imported/w3c/web-platform-tests/fetch/api/redirect/redirect-mode.html [ DumpJSConsoleLogInStdErr ]
+
 # textarea.animate is not supported
 imported/w3c/web-platform-tests/css/css-ui-3/caret-color-018.html [ Skip ]
 imported/w3c/web-platform-tests/css/css-ui-3/caret-color-019.html [ Skip ]

diff --git a/LayoutTests/http/tests/fetch/redirectmode-and-preload-expected.txt b/LayoutTests/http/tests/fetch/redirectmode-and-preload-expected.txt
index 48976a0..e958842 100644 (file)
--- a/LayoutTests/http/tests/fetch/redirectmode-and-preload-expected.txt
+++ b/LayoutTests/http/tests/fetch/redirectmode-and-preload-expected.txt
@@ -1,3 +1,5 @@
+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. Redirections are not allowed
+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. Redirections are not allowed
 
 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) 

diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
index cc2bbde..2750c91 100644 (file)
--- a/LayoutTests/imported/w3c/ChangeLog
+++ b/LayoutTests/imported/w3c/ChangeLog
@@ -1,5 +1,19 @@
 2017-12-18  Chris Dumez  <cdumez@apple.com>
 
+        We should use "error" redirect mode for fetching service worker scripts
+        https://bugs.webkit.org/show_bug.cgi?id=180950
+
+        Reviewed by Youenn Fablet.
+
+        Rebaseline test now that behavior has changed. Note that we are still failing becuase we reject
+        the registration promise with a TypeError instead of a SecurityError. I cannot find any reason
+        to throw a SecurityError here based on the specification though, so the test may not match the
+        specification.
+
+        * web-platform-tests/service-workers/service-worker/registration-security-error.https-expected.txt:
+
+2017-12-18  Chris Dumez  <cdumez@apple.com>
+
         ExtendableMessageEvent constructor fails to initialize the 'source' attribute
         https://bugs.webkit.org/show_bug.cgi?id=180954
 

diff --git a/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/registration-security-error.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/registration-security-error.https-expected.txt
index 4839d4f..7b7a228 100644 (file)
--- a/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/registration-security-error.https-expected.txt
+++ b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/registration-security-error.https-expected.txt
@@ -3,7 +3,7 @@ FAIL Registering same scope as the script directory without the last slash asser
 FAIL Registration scope outside the script directory assert_unreached: Should have rejected: Registration scope outside the script directory should fail with SecurityError. Reached unreachable code
 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=%2Fresources%2Fregistration-worker.js fetch resulted in error: Failed to load script" 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=%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 Scope including parent-reference and not under the script directory assert_unreached: Should have rejected: Scope not under the script directory should be rejected. Reached unreachable code
 FAIL Script URL including consecutive slashes assert_unreached: Should have rejected: Consecutive slashes in the script url should not be unified. Reached unreachable code
 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/imported/w3c/web-platform-tests/service-workers/service-worker/resources/registration-tests-security-error.js b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/resources/registration-tests-security-error.js
index c84bb66..c98c4b9 100644 (file)
--- a/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/resources/registration-tests-security-error.js
+++ b/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/resources/registration-tests-security-error.js
@@ -40,7 +40,7 @@ function registration_tests_security_error(register_method, check_error_types) {
 
   promise_test(function(t) {
       var script = 'resources/redirect.py?Redirect=' +
-                    encodeURIComponent('/resources/registration-worker.js');
+                    encodeURIComponent('/service-workers/service-worker/resources/registration-worker.js');
       var scope = 'resources/scope/redirect/';
       return promise_rejects(t,
           check_error_types ? 'SecurityError' : null,

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 50f5d91..ec31b07 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,27 @@
 2017-12-18  Chris Dumez  <cdumez@apple.com>
 
+        We should use "error" redirect mode for fetching service worker scripts
+        https://bugs.webkit.org/show_bug.cgi?id=180950
+
+        Reviewed by Youenn Fablet.
+
+        We should use "error" redirect mode for fetching service worker scripts, as per:
+        - https://w3c.github.io/ServiceWorker/#update (Step 7.5)
+
+        No new tests, rebaselined existing test.
+
+        * loader/SubresourceLoader.cpp:
+        (WebCore::SubresourceLoader::willSendRequestInternal):
+        * workers/Worker.cpp:
+        (WebCore::Worker::create):
+        * workers/WorkerScriptLoader.cpp:
+        (WebCore::WorkerScriptLoader::loadAsynchronously):
+        * workers/WorkerScriptLoader.h:
+        * workers/service/ServiceWorkerJob.cpp:
+        (WebCore::ServiceWorkerJob::fetchScriptWithContext):
+
+2017-12-18  Chris Dumez  <cdumez@apple.com>
+
         ExtendableMessageEvent constructor fails to initialize the 'source' attribute
         https://bugs.webkit.org/show_bug.cgi?id=180954
 

diff --git a/Source/WebCore/loader/SubresourceLoader.cpp b/Source/WebCore/loader/SubresourceLoader.cpp
index 34a8e16..9958dfd 100644 (file)
--- a/Source/WebCore/loader/SubresourceLoader.cpp
+++ b/Source/WebCore/loader/SubresourceLoader.cpp
@@ -206,7 +206,7 @@ void SubresourceLoader::willSendRequestInternal(ResourceRequest&& newRequest, co
     if (!redirectResponse.isNull()) {
         if (options().redirect != FetchOptions::Redirect::Follow) {
             if (options().redirect == FetchOptions::Redirect::Error) {
-                cancel();
+                cancel(ResourceError { errorDomainWebKitInternal, 0, redirectResponse.url(), "Redirections are not allowed", ResourceError::Type::AccessControl });
                 return completionHandler(WTFMove(newRequest));
             }
 

diff --git a/Source/WebCore/workers/Worker.cpp b/Source/WebCore/workers/Worker.cpp
index eca52f3..386f0e5 100644 (file)
--- a/Source/WebCore/workers/Worker.cpp
+++ b/Source/WebCore/workers/Worker.cpp
@@ -101,7 +101,7 @@ ExceptionOr<Ref<Worker>> Worker::create(ScriptExecutionContext& context, JSC::Ru
 
     ResourceRequest request { scriptURL.releaseReturnValue() };
     request.setInitiatorIdentifier(worker->m_identifier);
-    worker->m_scriptLoader->loadAsynchronously(context, WTFMove(request), FetchOptions::Mode::SameOrigin, FetchOptions::Cache::Default, contentSecurityPolicyEnforcement, worker);
+    worker->m_scriptLoader->loadAsynchronously(context, WTFMove(request), FetchOptions::Mode::SameOrigin, FetchOptions::Cache::Default, FetchOptions::Redirect::Follow, contentSecurityPolicyEnforcement, worker);
     return WTFMove(worker);
 }
 

diff --git a/Source/WebCore/workers/WorkerScriptLoader.cpp b/Source/WebCore/workers/WorkerScriptLoader.cpp
index 0c14615..5b7bc3b 100644 (file)
--- a/Source/WebCore/workers/WorkerScriptLoader.cpp
+++ b/Source/WebCore/workers/WorkerScriptLoader.cpp
@@ -73,7 +73,7 @@ void WorkerScriptLoader::loadSynchronously(ScriptExecutionContext* scriptExecuti
     WorkerThreadableLoader::loadResourceSynchronously(workerGlobalScope, WTFMove(*request), *this, options);
 }
 
-void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext& scriptExecutionContext, ResourceRequest&& scriptRequest, FetchOptions::Mode mode, FetchOptions::Cache cachePolicy, ContentSecurityPolicyEnforcement contentSecurityPolicyEnforcement, WorkerScriptLoaderClient& client)
+void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext& scriptExecutionContext, ResourceRequest&& scriptRequest, FetchOptions::Mode mode, FetchOptions::Cache cachePolicy, FetchOptions::Redirect redirectPolicy, ContentSecurityPolicyEnforcement contentSecurityPolicyEnforcement, WorkerScriptLoaderClient& client)
 {
     m_client = &client;
     m_url = scriptRequest.url();
@@ -92,6 +92,7 @@ void WorkerScriptLoader::loadAsynchronously(ScriptExecutionContext& scriptExecut
     options.credentials = FetchOptions::Credentials::SameOrigin;
     options.mode = mode;
     options.cache = cachePolicy;
+    options.redirect = redirectPolicy;
     options.sendLoadCallbacks = SendCallbacks;
     options.contentSecurityPolicyEnforcement = contentSecurityPolicyEnforcement;
 #if ENABLE(SERVICE_WORKER)

diff --git a/Source/WebCore/workers/WorkerScriptLoader.h b/Source/WebCore/workers/WorkerScriptLoader.h
index 53aafc8..d688b38 100644 (file)
--- a/Source/WebCore/workers/WorkerScriptLoader.h
+++ b/Source/WebCore/workers/WorkerScriptLoader.h
@@ -53,7 +53,7 @@ public:
     }
 
     void loadSynchronously(ScriptExecutionContext*, const URL&, FetchOptions::Mode, FetchOptions::Cache, ContentSecurityPolicyEnforcement, const String& initiatorIdentifier);
-    void loadAsynchronously(ScriptExecutionContext&, ResourceRequest&&, FetchOptions::Mode, FetchOptions::Cache, ContentSecurityPolicyEnforcement, WorkerScriptLoaderClient&);
+    void loadAsynchronously(ScriptExecutionContext&, ResourceRequest&&, FetchOptions::Mode, FetchOptions::Cache, FetchOptions::Redirect, ContentSecurityPolicyEnforcement, WorkerScriptLoaderClient&);
 
     void notifyError();
 

diff --git a/Source/WebCore/workers/service/ServiceWorkerJob.cpp b/Source/WebCore/workers/service/ServiceWorkerJob.cpp
index 1b930c3..698261c 100644 (file)
--- a/Source/WebCore/workers/service/ServiceWorkerJob.cpp
+++ b/Source/WebCore/workers/service/ServiceWorkerJob.cpp
@@ -97,7 +97,7 @@ void ServiceWorkerJob::fetchScriptWithContext(ScriptExecutionContext& context, F
     request.setInitiatorIdentifier("serviceWorkerScriptLoad:");
     request.addHTTPHeaderField(ASCIILiteral("Service-Worker"), ASCIILiteral("script"));
 
-    m_scriptLoader->loadAsynchronously(context, WTFMove(request), FetchOptions::Mode::SameOrigin, cachePolicy, ContentSecurityPolicyEnforcement::DoNotEnforce, *this);
+    m_scriptLoader->loadAsynchronously(context, WTFMove(request), FetchOptions::Mode::SameOrigin, cachePolicy, FetchOptions::Redirect::Error, ContentSecurityPolicyEnforcement::DoNotEnforce, *this);
 }
 
 void ServiceWorkerJob::didReceiveResponse(unsigned long, const ResourceResponse& response)