Diff
Modified: trunk/LayoutTests/ChangeLog (242111 => 242112)
--- trunk/LayoutTests/ChangeLog 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/LayoutTests/ChangeLog 2019-02-26 23:53:22 UTC (rev 242112)
@@ -1,3 +1,16 @@
+2019-02-26 Youenn Fablet <[email protected]>
+
+ Move service worker response validation from the service worker client to the service worker itself
+ https://bugs.webkit.org/show_bug.cgi?id=194716
+
+ Reviewed by Geoffrey Garen.
+
+ Rebased tests as we now report to the console log any service worker response validation erorr.
+
+ * http/tests/inspector/network/resource-response-service-worker-expected.txt:
+ * http/tests/workers/service/basic-fetch.https-expected.txt:
+ * http/tests/workers/service/service-worker-crossorigin-fetch-expected.txt:
+
2019-02-26 Takashi Komori <[email protected]>
[Curl] Load HTTP body of 401 response when AuthenticationChange is cancelled.
Modified: trunk/LayoutTests/http/tests/inspector/network/resource-response-service-worker-expected.txt (242111 => 242112)
--- trunk/LayoutTests/http/tests/inspector/network/resource-response-service-worker-expected.txt 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/LayoutTests/http/tests/inspector/network/resource-response-service-worker-expected.txt 2019-02-26 23:53:22 UTC (rev 242112)
@@ -1,3 +1,4 @@
+CONSOLE MESSAGE: Response served by service worker is an error
Test for `Resource.ResponseSource.ServiceWorker`.
Modified: trunk/LayoutTests/http/tests/workers/service/basic-fetch.https-expected.txt (242111 => 242112)
--- trunk/LayoutTests/http/tests/workers/service/basic-fetch.https-expected.txt 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/LayoutTests/http/tests/workers/service/basic-fetch.https-expected.txt 2019-02-26 23:53:22 UTC (rev 242112)
@@ -1,5 +1,7 @@
+CONSOLE MESSAGE: Response served by service worker is an error
CONSOLE MESSAGE: Fetch event was canceled
CONSOLE MESSAGE: Fetch API cannot load https://127.0.0.1:8443/workers/service/resources/test5.
+CONSOLE MESSAGE: Response served by service worker is an error
test1 url: https://127.0.0.1:8443/workers/service/resources/test1
test1 status code: 200
Modified: trunk/LayoutTests/http/tests/workers/service/service-worker-crossorigin-fetch-expected.txt (242111 => 242112)
--- trunk/LayoutTests/http/tests/workers/service/service-worker-crossorigin-fetch-expected.txt 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/LayoutTests/http/tests/workers/service/service-worker-crossorigin-fetch-expected.txt 2019-02-26 23:53:22 UTC (rev 242112)
@@ -1,5 +1,6 @@
CONSOLE MESSAGE: Origin http://127.0.0.1:8000 is not allowed by Access-Control-Allow-Origin.
CONSOLE MESSAGE: Fetch API cannot load http://localhost:8080/resources/square100.png.fromserviceworker due to access control checks.
+CONSOLE MESSAGE: Response served by service worker is an error
PASS Testing unintercepted fetch with preflight, fetch should fail
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (242111 => 242112)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2019-02-26 23:53:22 UTC (rev 242112)
@@ -1,3 +1,14 @@
+2019-02-26 Youenn Fablet <[email protected]>
+
+ Move service worker response validation from the service worker client to the service worker itself
+ https://bugs.webkit.org/show_bug.cgi?id=194716
+
+ Reviewed by Geoffrey Garen.
+
+ Rebased tests as we now report to the console log any service worker response validation erorr.
+
+ * web-platform-tests/service-workers/service-worker/fetch-canvas-tainting-image.https-expected.txt:
+
2019-02-26 Frederic Wang <[email protected]>
Synchronize MathML WPT tests
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/fetch-canvas-tainting-image.https-expected.txt (242111 => 242112)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/fetch-canvas-tainting-image.https-expected.txt 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/fetch-canvas-tainting-image.https-expected.txt 2019-02-26 23:53:22 UTC (rev 242112)
@@ -21,13 +21,21 @@
CONSOLE MESSAGE: Cannot load image https://127.0.0.1:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&Auth&ACAOrigin=https://localhost:9443&ignore due to access control checks.
CONSOLE MESSAGE: line 26: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
CONSOLE MESSAGE: Response served by service worker is opaque
+CONSOLE MESSAGE: Cannot load https://127.0.0.1:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE.
+CONSOLE MESSAGE: Response served by service worker is opaque
CONSOLE MESSAGE: Cannot load image https://localhost:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=no-cors&url="" due to access control checks.
CONSOLE MESSAGE: Response served by service worker is opaque
+CONSOLE MESSAGE: Cannot load https://127.0.0.1:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE.
+CONSOLE MESSAGE: Response served by service worker is opaque
CONSOLE MESSAGE: Cannot load image https://localhost:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=no-cors&url="" due to access control checks.
CONSOLE MESSAGE: line 26: Unable to get image data from canvas because the canvas has been tainted by cross-origin data.
CONSOLE MESSAGE: Response served by service worker is opaque
+CONSOLE MESSAGE: Cannot load https://127.0.0.1:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE.
+CONSOLE MESSAGE: Response served by service worker is opaque
CONSOLE MESSAGE: Cannot load image https://127.0.0.1:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=no-cors&url="" due to access control checks.
CONSOLE MESSAGE: Response served by service worker is opaque
+CONSOLE MESSAGE: Cannot load https://127.0.0.1:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE.
+CONSOLE MESSAGE: Response served by service worker is opaque
CONSOLE MESSAGE: Cannot load image https://127.0.0.1:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=no-cors&url="" due to access control checks.
CONSOLE MESSAGE: FetchEvent.respondWith received an error: TypeError: Credentials flag is true, but Access-Control-Allow-Credentials is not "true".
CONSOLE MESSAGE: Cannot load https://localhost:9443/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=cors&url=""
Modified: trunk/Source/WebCore/ChangeLog (242111 => 242112)
--- trunk/Source/WebCore/ChangeLog 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/Source/WebCore/ChangeLog 2019-02-26 23:53:22 UTC (rev 242112)
@@ -1,3 +1,20 @@
+2019-02-26 Youenn Fablet <[email protected]>
+
+ Move service worker response validation from the service worker client to the service worker itself
+ https://bugs.webkit.org/show_bug.cgi?id=194716
+
+ Reviewed by Geoffrey Garen.
+
+ Added response validation at service worker side.
+
+ No change of behavior except for now logging validation error messages in the console.
+ Covered by rebased tests.
+
+ * workers/service/context/ServiceWorkerFetch.cpp:
+ (WebCore::ServiceWorkerFetch::validateResponse):
+ (WebCore::ServiceWorkerFetch::processResponse):
+ (WebCore::ServiceWorkerFetch::dispatchFetchEvent):
+
2019-02-26 Sihui Liu <[email protected]>
[Mac WK2] storage/indexeddb/IDBObject-leak.html is flaky
Modified: trunk/Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp (242111 => 242112)
--- trunk/Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp 2019-02-26 23:53:22 UTC (rev 242112)
@@ -33,6 +33,7 @@
#include "FetchEvent.h"
#include "FetchRequest.h"
#include "FetchResponse.h"
+#include "MIMETypeRegistry.h"
#include "ReadableStreamChunk.h"
#include "ResourceRequest.h"
#include "ServiceWorker.h"
@@ -43,8 +44,27 @@
namespace ServiceWorkerFetch {
-static void processResponse(Ref<Client>&& client, Expected<Ref<FetchResponse>, ResourceError>&& result)
+// https://fetch.spec.whatwg.org/#http-fetch step 3.3
+static inline Optional<ResourceError> validateResponse(const ResourceResponse& response, FetchOptions::Mode mode, FetchOptions::Redirect redirect)
{
+ if (response.type() == ResourceResponse::Type::Error)
+ return ResourceError { errorDomainWebKitInternal, 0, response.url(), "Response served by service worker is an error"_s, ResourceError::Type::General };
+
+ if (mode != FetchOptions::Mode::NoCors && response.tainting() == ResourceResponse::Tainting::Opaque)
+ return ResourceError { errorDomainWebKitInternal, 0, response.url(), "Response served by service worker is opaque"_s, ResourceError::Type::AccessControl };
+
+ // Navigate mode induces manual redirect.
+ if (redirect != FetchOptions::Redirect::Manual && mode != FetchOptions::Mode::Navigate && response.tainting() == ResourceResponse::Tainting::Opaqueredirect)
+ return ResourceError { errorDomainWebKitInternal, 0, response.url(), "Response served by service worker is opaque redirect"_s, ResourceError::Type::AccessControl };
+
+ if ((redirect != FetchOptions::Redirect::Follow || mode == FetchOptions::Mode::Navigate) && response.isRedirected())
+ return ResourceError { errorDomainWebKitInternal, 0, response.url(), "Response served by service worker has redirections"_s, ResourceError::Type::AccessControl };
+
+ return { };
+}
+
+static void processResponse(Ref<Client>&& client, Expected<Ref<FetchResponse>, ResourceError>&& result, FetchOptions::Mode mode, FetchOptions::Redirect redirect, const URL& requestURL)
+{
if (!result.has_value()) {
client->didFail(result.error());
return;
@@ -58,11 +78,31 @@
}
auto resourceResponse = response->resourceResponse();
+ if (auto error = validateResponse(resourceResponse, mode, redirect)) {
+ client->didFail(error.value());
+ return;
+ }
+
if (resourceResponse.isRedirection() && resourceResponse.httpHeaderFields().contains(HTTPHeaderName::Location)) {
client->didReceiveRedirection(resourceResponse);
return;
}
+ resourceResponse.setSource(ResourceResponse::Source::ServiceWorker);
+
+ // In case of main resource and mime type is the default one, we set it to text/html to pass more service worker WPT tests.
+ // FIXME: We should refine our MIME type sniffing strategy for synthetic responses.
+ if (mode == FetchOptions::Mode::Navigate) {
+ if (resourceResponse.mimeType() == defaultMIMEType()) {
+ resourceResponse.setMimeType("text/html"_s);
+ resourceResponse.setTextEncodingName("UTF-8"_s);
+ }
+ }
+
+ // As per https://fetch.spec.whatwg.org/#main-fetch step 9, copy request's url list in response's url list if empty.
+ if (resourceResponse.url().isNull())
+ resourceResponse.setURL(requestURL);
+
client->didReceiveResponse(resourceResponse);
if (response->isBodyReceivedByChunk()) {
@@ -95,6 +135,9 @@
{
auto requestHeaders = FetchHeaders::create(FetchHeaders::Guard::Immutable, HTTPHeaderMap { request.httpHeaderFields() });
+ FetchOptions::Mode mode = options.mode;
+ FetchOptions::Redirect redirect = options.redirect;
+
bool isNavigation = options.mode == FetchOptions::Mode::Navigate;
bool isNonSubresourceRequest = WebCore::isNonSubresourceRequest(options.destination);
@@ -129,8 +172,8 @@
init.cancelable = true;
auto event = FetchEvent::create(eventNames().fetchEvent, WTFMove(init), Event::IsTrusted::Yes);
- event->onResponse([client = client.copyRef()] (auto&& result) mutable {
- processResponse(WTFMove(client), WTFMove(result));
+ event->onResponse([client = client.copyRef(), mode, redirect, requestURL] (auto&& result) mutable {
+ processResponse(WTFMove(client), WTFMove(result), mode, redirect, requestURL);
});
globalScope.dispatchEvent(event);
Modified: trunk/Source/WebKit/ChangeLog (242111 => 242112)
--- trunk/Source/WebKit/ChangeLog 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/Source/WebKit/ChangeLog 2019-02-26 23:53:22 UTC (rev 242112)
@@ -1,3 +1,16 @@
+2019-02-26 Youenn Fablet <[email protected]>
+
+ Move service worker response validation from the service worker client to the service worker itself
+ https://bugs.webkit.org/show_bug.cgi?id=194716
+
+ Reviewed by Geoffrey Garen.
+
+ Removed response validation as it is now done in service worker side.
+
+ * WebProcess/Storage/ServiceWorkerClientFetch.cpp:
+ (WebKit::ServiceWorkerClientFetch::didReceiveRedirectResponse):
+ (WebKit::ServiceWorkerClientFetch::didReceiveResponse):
+
2019-02-26 Zalan Bujtas <[email protected]>
[ContentChangeObserver] clearContentChangeObservers should be internal to ContentChangeObserver class
Modified: trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp (242111 => 242112)
--- trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp 2019-02-26 23:45:02 UTC (rev 242111)
+++ trunk/Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp 2019-02-26 23:53:22 UTC (rev 242112)
@@ -34,7 +34,6 @@
#include <WebCore/CrossOriginAccessControl.h>
#include <WebCore/Document.h>
#include <WebCore/Frame.h>
-#include <WebCore/MIMETypeRegistry.h>
#include <WebCore/NotImplemented.h>
#include <WebCore/ResourceError.h>
#include <WebCore/SharedBuffer.h>
@@ -78,27 +77,6 @@
m_connection->startFetch(m_identifier, m_serviceWorkerRegistrationIdentifier, request, options, referrer);
}
-// https://fetch.spec.whatwg.org/#http-fetch step 3.3
-Optional<ResourceError> ServiceWorkerClientFetch::validateResponse(const ResourceResponse& response)
-{
- // FIXME: make a better error reporting.
- if (response.type() == ResourceResponse::Type::Error)
- return ResourceError { ResourceError::Type::General };
-
- auto& options = m_loader->options();
- if (options.mode != FetchOptions::Mode::NoCors && response.tainting() == ResourceResponse::Tainting::Opaque)
- return ResourceError { errorDomainWebKitInternal, 0, response.url(), "Response served by service worker is opaque"_s, ResourceError::Type::AccessControl };
-
- // Navigate mode induces manual redirect.
- if (options.redirect != FetchOptions::Redirect::Manual && options.mode != FetchOptions::Mode::Navigate && response.tainting() == ResourceResponse::Tainting::Opaqueredirect)
- return ResourceError { errorDomainWebKitInternal, 0, response.url(), "Response served by service worker is opaque redirect"_s, ResourceError::Type::AccessControl };
-
- if ((options.redirect != FetchOptions::Redirect::Follow || options.mode == FetchOptions::Mode::Navigate) && response.isRedirected())
- return ResourceError { errorDomainWebKitInternal, 0, response.url(), "Response served by service worker has redirections"_s, ResourceError::Type::AccessControl };
-
- return WTF::nullopt;
-}
-
void ServiceWorkerClientFetch::didReceiveRedirectResponse(ResourceResponse&& response)
{
callOnMainThread([this, protectedThis = makeRef(*this), response = WTFMove(response)]() mutable {
@@ -105,14 +83,6 @@
if (!m_loader)
return;
- if (auto error = validateResponse(response)) {
- m_loader->didFail(error.value());
- ASSERT(!m_loader);
- if (auto callback = WTFMove(m_callback))
- callback(Result::Succeeded);
-
- return;
- }
response.setSource(ResourceResponse::Source::ServiceWorker);
m_loader->willSendRequest(m_loader->request().redirectedRequest(response, m_shouldClearReferrerOnHTTPSToHTTPRedirect), response, [this, protectedThis = protectedThis.copyRef()](ResourceRequest&& request) {
@@ -133,33 +103,11 @@
if (!m_loader)
return;
- if (auto error = validateResponse(response)) {
- m_loader->didFail(error.value());
- ASSERT(!m_loader);
- if (auto callback = WTFMove(m_callback))
- callback(Result::Succeeded);
-
- return;
- }
- response.setSource(ResourceResponse::Source::ServiceWorker);
- ASSERT(!response.isRedirection() || !response.httpHeaderFields().contains(HTTPHeaderName::Location));
-
if (auto callback = WTFMove(m_callback))
callback(Result::Succeeded);
- // In case of main resource and mime type is the default one, we set it to text/html to pass more service worker WPT tests.
- // FIXME: We should refine our MIME type sniffing strategy for synthetic responses.
- if (m_loader->originalRequest().requester() == ResourceRequest::Requester::Main) {
- if (response.mimeType() == defaultMIMEType()) {
- response.setMimeType("text/html"_s);
- response.setTextEncodingName("UTF-8"_s);
- }
- }
+ ASSERT(!response.isRedirection() || !response.httpHeaderFields().contains(HTTPHeaderName::Location));
- // As per https://fetch.spec.whatwg.org/#main-fetch step 9, copy request's url list in response's url list if empty.
- if (response.url().isNull())
- response.setURL(m_loader->request().url());
-
if (!needsContinueDidReceiveResponseMessage) {
m_loader->didReceiveResponse(response, [] { });
return;
@@ -213,7 +161,8 @@
auto* document = m_loader->frame() ? m_loader->frame()->document() : nullptr;
if (document) {
- document->addConsoleMessage(MessageSource::JS, MessageLevel::Error, error.localizedDescription());
+ if (m_loader->options().destination != FetchOptions::Destination::EmptyString || error.isGeneral())
+ document->addConsoleMessage(MessageSource::JS, MessageLevel::Error, error.localizedDescription());
if (m_loader->options().destination != FetchOptions::Destination::EmptyString)
document->addConsoleMessage(MessageSource::JS, MessageLevel::Error, makeString("Cannot load ", error.failingURL().string(), "."));
}
