Early return when handling fetch event in case service worker origin does not match origin of a subresource load
<rdar://problem/41329832>
Created attachment 343841 [details] Patch
Comment on attachment 343841 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343841&action=review > Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:190 > + RELEASE_LOG_ERROR(ServiceWorker, "%{public}s: service worker is %{public}s://...:%d, request URL is %{public}s://...:%d", message, serviceWorkerURL.protocol().utf8().data(), serviceWorkerPort, requestOriginURL.protocol().utf8().data(), requestOriginURLPort); I do not believe logging URLs is OK.
Created attachment 343850 [details] Patch
Comment on attachment 343850 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343850&action=review > Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:195 > + RELEASE_ASSERT_WITH_MESSAGE(request.url().host() == serviceWorkerURL.host(), "Hosts do not match"); I think we may want this one first as it is the most important, no? > Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:205 > + ASSERT(url.host() == serviceWorkerURL.host()); ditto.
Created attachment 343860 [details] Patch for landing
> > Source/WebKit/WebProcess/Storage/WebSWContextManagerConnection.cpp:195 > > + RELEASE_ASSERT_WITH_MESSAGE(request.url().host() == serviceWorkerURL.host(), "Hosts do not match"); > > I think we may want this one first as it is the most important, no? Done
Comment on attachment 343860 [details] Patch for landing Clearing flags on attachment: 343860 Committed r233335: <https://trac.webkit.org/changeset/233335>
All reviewed patches have been landed. Closing bug.
It looks like after r233335 this test became a flakey timeout: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fservice-workers%2Fservice-worker%2Fclaim-with-redirect.https.html I built r233335 locally and ran: run-webkit-tests imported/w3c/web-platform-tests/service-workers/service-worker/claim-with-redirect.https.html --iterations 500 -f and out of 500 runs got 18 timeouts. I reproduced this multiple times. I built r233334 locally and ran the same command and received 0 timeout through multiple runs.
(In reply to Truitt Savell from comment #10) > It looks like after r233335 this test became a flakey timeout: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fservice- > workers%2Fservice-worker%2Fclaim-with-redirect.https.html That is great news!! We are crashing in debug builds: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x0000000207754020 WTFCrash + 16 (Assertions.cpp:267) 1 com.apple.WebKit 0x0000000106b766d3 WebKit::isValidFetch(WebCore::ResourceRequest const&, WebCore::FetchOptions const&, WebCore::URL const&, WTF::String const&) + 1219 (WebSWContextManagerConnection.cpp:204) 2 com.apple.WebKit 0x0000000106b75fad WebKit::WebSWContextManagerConnection::startFetch(WTF::ObjectIdentifier<WebCore::SWServerConnectionIdentifierType>, WTF::ObjectIdentifier<WebCore::ServiceWorkerIdentifierType>, WTF::ObjectIdentifier<WebCore::FetchIdentifierType>, WebCore::ResourceRequest&&, WebCore::FetchOptions&&, IPC::FormDataReference&&, WTF::String&&) + 285 (WebSWContextManagerConnection.cpp:226)
(In reply to youenn fablet from comment #11) > (In reply to Truitt Savell from comment #10) > > It looks like after r233335 this test became a flakey timeout: > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > > html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fservice- > > workers%2Fservice-worker%2Fclaim-with-redirect.https.html > > That is great news!! > > We are crashing in debug builds: > Thread 0 Crashed:: Dispatch queue: com.apple.main-thread > 0 com.apple.JavaScriptCore 0x0000000207754020 WTFCrash + 16 > (Assertions.cpp:267) > 1 com.apple.WebKit 0x0000000106b766d3 > WebKit::isValidFetch(WebCore::ResourceRequest const&, WebCore::FetchOptions > const&, WebCore::URL const&, WTF::String const&) + 1219 > (WebSWContextManagerConnection.cpp:204) > 2 com.apple.WebKit 0x0000000106b75fad > WebKit::WebSWContextManagerConnection::startFetch(WTF:: > ObjectIdentifier<WebCore::SWServerConnectionIdentifierType>, > WTF::ObjectIdentifier<WebCore::ServiceWorkerIdentifierType>, > WTF::ObjectIdentifier<WebCore::FetchIdentifierType>, > WebCore::ResourceRequest&&, WebCore::FetchOptions&&, > IPC::FormDataReference&&, WTF::String&&) + 285 > (WebSWContextManagerConnection.cpp:226) So the host is not even matching :/ Seems bad :)
OK, this is just a mistake made in this patch, fix is at https://bugs.webkit.org/show_bug.cgi?id=187282