Skip to content
Permalink
Browse files
Dedicated worker and shared worker global scope should use the respon…
…se URL

https://bugs.webkit.org/show_bug.cgi?id=247615
rdar://problem/102088086

Reviewed by Chris Dumez.

As per https://html.spec.whatwg.org/multipage/workers.html#run-a-worker step 14.3.1, we should use the response URL.
Update the code accordingly, which allows removing the redirectReceived callback.
Update DocumentThreadableLoader to provide a correct response URL, including fragment identifier.
As DocumentThreadableLoader may reuse existing loads with same URL but different fragment identifier, we need to correctly set the response URL fragment identifier ourselves.

Covered by rebased test.

* LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/worker-interception-redirect.https-expected.txt:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/service-workers/service-worker/worker-interception-redirect.https-expected.txt:
* Source/WebCore/loader/DocumentThreadableLoader.cpp:
* Source/WebCore/loader/DocumentThreadableLoader.h:
* Source/WebCore/loader/ThreadableLoaderClient.h:
(WebCore::ThreadableLoaderClient::redirectReceived): Deleted.
* Source/WebCore/loader/ThreadableLoaderClientWrapper.h:
(WebCore::ThreadableLoaderClientWrapper::redirectReceived): Deleted.
* Source/WebCore/loader/WorkerThreadableLoader.cpp:
(WebCore::WorkerThreadableLoader::MainThreadBridge::redirectReceived): Deleted.
* Source/WebCore/loader/WorkerThreadableLoader.h:
* Source/WebCore/workers/Worker.cpp:
(WebCore::Worker::notifyFinished):
* Source/WebCore/workers/WorkerFetchResult.h:
(WebCore::WorkerFetchResult::isolatedCopy const):
(WebCore::WorkerFetchResult::encode const):
(WebCore::WorkerFetchResult::decode):
* Source/WebCore/workers/WorkerScriptLoader.cpp:
(WebCore::WorkerScriptLoader::loadSynchronously):
(WebCore::WorkerScriptLoader::loadAsynchronously):
(WebCore::WorkerScriptLoader::fetchResult const):
(WebCore::WorkerScriptLoader::redirectReceived): Deleted.
* Source/WebCore/workers/WorkerScriptLoader.h:
(WebCore::WorkerScriptLoader::url const):
(WebCore::WorkerScriptLoader::lastRequestURL const): Deleted.
* Source/WebCore/workers/shared/context/SharedWorkerThreadProxy.cpp:
(WebCore::generateWorkerParameters):
* Source/WebKit/WebProcess/Storage/WebSharedWorkerContextManagerConnection.cpp:
(WebKit::WebSharedWorkerContextManagerConnection::launchSharedWorker):

Canonical link: https://commits.webkit.org/256532@main
  • Loading branch information
youennf committed Nov 10, 2022
1 parent cbf7509 commit 99d539834b6ab6378da65545d5be275f7b08d44d
Show file tree
Hide file tree
Showing 14 changed files with 31 additions and 117 deletions.
@@ -41,18 +41,18 @@ PASS Case #2: network scope1->out-scope (classic SharedWorker, location.href)
PASS Case #2: network scope1->out-scope (module SharedWorker, importScripts())
PASS Case #2: network scope1->out-scope (module SharedWorker, fetch())
PASS Case #2: network scope1->out-scope (module SharedWorker, location.href)
FAIL Case #3: sw scope1->scope2 (classic DedicatedWorker, importScripts()) assert_equals: expected "sw2 saw importScripts from the worker: /service-workers/service-worker/resources/subdir/import-scripts-echo.py" but got "sw2 saw importScripts from the worker: /service-workers/service-worker/resources/scope2/import-scripts-echo.py"
FAIL Case #3: sw scope1->scope2 (classic DedicatedWorker, fetch()) assert_equals: expected "fetch(): sw2 saw the fetch from the worker: /service-workers/service-worker/resources/subdir/simple.txt" but got "fetch(): sw2 saw the fetch from the worker: /service-workers/service-worker/resources/scope2/simple.txt"
FAIL Case #3: sw scope1->scope2 (classic DedicatedWorker, location.href) assert_equals: location.href expected "https://localhost:9443/service-workers/service-worker/resources/subdir/worker_interception_redirect_webworker.py?greeting=sw2%20saw%20the%20request%20for%20the%20worker%20script" but got "https://localhost:9443/service-workers/service-worker/resources/scope2/worker_interception_redirect_webworker.py"
PASS Case #3: sw scope1->scope2 (classic DedicatedWorker, importScripts())
PASS Case #3: sw scope1->scope2 (classic DedicatedWorker, fetch())
PASS Case #3: sw scope1->scope2 (classic DedicatedWorker, location.href)
PASS Case #3: sw scope1->scope2 (module DedicatedWorker, importScripts())
FAIL Case #3: sw scope1->scope2 (module DedicatedWorker, fetch()) assert_equals: expected "fetch(): sw2 saw the fetch from the worker: /service-workers/service-worker/resources/subdir/simple.txt" but got "fetch(): sw2 saw the fetch from the worker: /service-workers/service-worker/resources/scope2/simple.txt"
FAIL Case #3: sw scope1->scope2 (module DedicatedWorker, location.href) assert_equals: location.href expected "https://localhost:9443/service-workers/service-worker/resources/subdir/worker_interception_redirect_webworker.py?greeting=sw2%20saw%20the%20request%20for%20the%20worker%20script" but got "https://localhost:9443/service-workers/service-worker/resources/scope2/worker_interception_redirect_webworker.py"
FAIL Case #3: sw scope1->scope2 (classic SharedWorker, importScripts()) assert_equals: expected "sw2 saw importScripts from the worker: /service-workers/service-worker/resources/subdir/import-scripts-echo.py" but got "sw2 saw importScripts from the worker: /service-workers/service-worker/resources/scope2/import-scripts-echo.py"
FAIL Case #3: sw scope1->scope2 (classic SharedWorker, fetch()) assert_equals: expected "fetch(): sw2 saw the fetch from the worker: /service-workers/service-worker/resources/subdir/simple.txt" but got "fetch(): sw2 saw the fetch from the worker: /service-workers/service-worker/resources/scope2/simple.txt"
FAIL Case #3: sw scope1->scope2 (classic SharedWorker, location.href) assert_equals: location.href expected "https://localhost:9443/service-workers/service-worker/resources/subdir/worker_interception_redirect_webworker.py?greeting=sw2%20saw%20the%20request%20for%20the%20worker%20script" but got "https://localhost:9443/service-workers/service-worker/resources/scope2/worker_interception_redirect_webworker.py"
PASS Case #3: sw scope1->scope2 (module DedicatedWorker, fetch())
PASS Case #3: sw scope1->scope2 (module DedicatedWorker, location.href)
PASS Case #3: sw scope1->scope2 (classic SharedWorker, importScripts())
PASS Case #3: sw scope1->scope2 (classic SharedWorker, fetch())
PASS Case #3: sw scope1->scope2 (classic SharedWorker, location.href)
PASS Case #3: sw scope1->scope2 (module SharedWorker, importScripts())
FAIL Case #3: sw scope1->scope2 (module SharedWorker, fetch()) assert_equals: expected "fetch(): sw2 saw the fetch from the worker: /service-workers/service-worker/resources/subdir/simple.txt" but got "fetch(): sw2 saw the fetch from the worker: /service-workers/service-worker/resources/scope2/simple.txt"
FAIL Case #3: sw scope1->scope2 (module SharedWorker, location.href) assert_equals: location.href expected "https://localhost:9443/service-workers/service-worker/resources/subdir/worker_interception_redirect_webworker.py?greeting=sw2%20saw%20the%20request%20for%20the%20worker%20script" but got "https://localhost:9443/service-workers/service-worker/resources/scope2/worker_interception_redirect_webworker.py"
PASS Case #3: sw scope1->scope2 (module SharedWorker, fetch())
PASS Case #3: sw scope1->scope2 (module SharedWorker, location.href)
PASS Case #4: sw scope1->out-scope (classic DedicatedWorker, importScripts())
PASS Case #4: sw scope1->out-scope (classic DedicatedWorker, fetch())
PASS Case #4: sw scope1->out-scope (classic DedicatedWorker, location.href)

This file was deleted.

@@ -315,8 +315,7 @@ void DocumentThreadableLoader::redirectReceived(CachedResource& resource, Resour
Ref<DocumentThreadableLoader> protectedThis(*this);
--m_options.maxRedirectCount;

if (m_client)
m_client->redirectReceived(request.url());
m_responseURL = request.url();

// FIXME: We restrict this check to Fetch API for the moment, as this might disrupt WorkerScriptLoader.
// Reassess this check based on https://github.com/whatwg/fetch/issues/393 discussions.
@@ -392,12 +391,18 @@ void DocumentThreadableLoader::dataSent(CachedResource& resource, unsigned long
void DocumentThreadableLoader::responseReceived(CachedResource& resource, const ResourceResponse& response, CompletionHandler<void()>&& completionHandler)
{
ASSERT_UNUSED(resource, &resource == m_resource);
ResourceResponse responseWithCorrectFragmentIdentifier;
if (response.source() != ResourceResponse::Source::ServiceWorker && response.url().fragmentIdentifier() != m_responseURL.fragmentIdentifier()) {
responseWithCorrectFragmentIdentifier = response;
responseWithCorrectFragmentIdentifier.setURL(m_responseURL);
}

if (!m_responsesCanBeOpaque) {
ResourceResponse responseWithoutTainting = response;
ResourceResponse responseWithoutTainting = responseWithCorrectFragmentIdentifier.isNull() ? response : responseWithCorrectFragmentIdentifier;
responseWithoutTainting.setTainting(ResourceResponse::Tainting::Basic);
didReceiveResponse(m_resource->identifier(), responseWithoutTainting);
} else
didReceiveResponse(m_resource->identifier(), response);
didReceiveResponse(m_resource->identifier(), responseWithCorrectFragmentIdentifier.isNull() ? response : responseWithCorrectFragmentIdentifier);

if (completionHandler)
completionHandler();
@@ -563,6 +568,8 @@ void DocumentThreadableLoader::loadRequest(ResourceRequest&& request, SecurityCh
Ref<DocumentThreadableLoader> protectedThis(*this);

// Any credential should have been removed from the cross-site requests.
m_responseURL = request.url();

const URL& requestURL = request.url();
m_options.securityCheck = securityCheck;
ASSERT(m_sameOriginRequest || !requestURL.hasCredentials());
@@ -140,6 +140,7 @@ namespace WebCore {
std::optional<CrossOriginEmbedderPolicy> m_crossOriginEmbedderPolicy;
std::optional<CrossOriginPreflightChecker> m_preflightChecker;
std::optional<HTTPHeaderMap> m_originalHeaders;
URL m_responseURL;

ShouldLogError m_shouldLogError;
#if ENABLE(SERVICE_WORKER)
@@ -45,7 +45,6 @@ class ThreadableLoaderClient {
public:
virtual void didSendData(unsigned long long /*bytesSent*/, unsigned long long /*totalBytesToBeSent*/) { }

virtual void redirectReceived(const URL& /*redirectURL*/) { }
virtual void didReceiveResponse(ResourceLoaderIdentifier, const ResourceResponse&) { }
virtual void didReceiveData(const SharedBuffer&) { }
virtual void didFinishLoading(ResourceLoaderIdentifier, const NetworkLoadMetrics&) { }
@@ -54,12 +54,6 @@ class ThreadableLoaderClientWrapper : public ThreadSafeRefCounted<ThreadableLoad
return m_done;
}

void redirectReceived(const URL& redirectURL)
{
if (m_client)
m_client->redirectReceived(redirectURL);
}

void didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
{
if (m_client)
@@ -228,14 +228,6 @@ void WorkerThreadableLoader::MainThreadBridge::clearClientWrapper()
m_workerClientWrapper->clearClient();
}

void WorkerThreadableLoader::MainThreadBridge::redirectReceived(const URL& redirectURL)
{
ScriptExecutionContext::postTaskForModeToWorkerOrWorklet(m_contextIdentifier, [protectedWorkerClientWrapper = Ref { *m_workerClientWrapper }, redirectURL = redirectURL.isolatedCopy()] (ScriptExecutionContext& context) mutable {
ASSERT_UNUSED(context, context.isWorkerGlobalScope() || context.isWorkletGlobalScope());
protectedWorkerClientWrapper->redirectReceived(redirectURL);
}, m_taskMode);
}

void WorkerThreadableLoader::MainThreadBridge::didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
{
ScriptExecutionContext::postTaskForModeToWorkerOrWorklet(m_contextIdentifier, [protectedWorkerClientWrapper = Ref { *m_workerClientWrapper }, bytesSent, totalBytesToBeSent] (ScriptExecutionContext& context) mutable {
@@ -103,7 +103,6 @@ class WorkerThreadableLoader : public RefCounted<WorkerThreadableLoader>, public
void clearClientWrapper();

// All executed on the main thread.
void redirectReceived(const URL& redirectURL) override;
void didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent) override;
void didReceiveResponse(ResourceLoaderIdentifier, const ResourceResponse&) override;
void didReceiveData(const SharedBuffer&) override;
@@ -224,9 +224,9 @@ void Worker::notifyFinished()
m_scriptLoader->takeServiceWorkerData(),
#endif
m_clientIdentifier,
context->userAgent(m_scriptLoader->lastRequestURL())
context->userAgent(m_scriptLoader->responseURL())
};
m_contextProxy.startWorkerGlobalScope(m_scriptLoader->lastRequestURL(), *sessionID, m_options.name, WTFMove(initializationData), m_scriptLoader->script(), contentSecurityPolicyResponseHeaders, m_shouldBypassMainWorldContentSecurityPolicy, m_scriptLoader->crossOriginEmbedderPolicy(), m_workerCreationTime, referrerPolicy, m_options.type, m_options.credentials, m_runtimeFlags);
m_contextProxy.startWorkerGlobalScope(m_scriptLoader->responseURL(), *sessionID, m_options.name, WTFMove(initializationData), m_scriptLoader->script(), contentSecurityPolicyResponseHeaders, m_shouldBypassMainWorldContentSecurityPolicy, m_scriptLoader->crossOriginEmbedderPolicy(), m_workerCreationTime, referrerPolicy, m_options.type, m_options.credentials, m_runtimeFlags);
InspectorInstrumentation::scriptImported(*context, m_scriptLoader->identifier(), m_scriptLoader->script().toString());
}

0 comments on commit 99d5398

Please sign in to comment.