We are currently failing a bunch of tests in fetch-response-tainting.https.html. This is because we set tainting on the nsIChannel immediately and then only maybe increase it when the service worker intercepts it. Instead the fetch spec is written to return the response produced by the service worker directly. So its exact tainting should be used even if its lower than the original nsIChannel. So a cross-origin no-cors request that is intercepted with a basic response would result in a basic response. Today we return an opaque in this circumstance. See this spec issue for more discussion: https://github.com/whatwg/fetch/issues/535

There is still an issue with non-e10s to figure out. We are getting DOM_BAD_URI in one of the WPT tests.

The non-e10s issue is bug 1370617.

Comment on attachment 8873969 [details] [diff] [review] P1 Expose a SynthesizeServiceWorkerTainting() on nsILoadInfo. r=ckerschb Christoph, this adds an nsILoadInfo API to set the tainting level. Normally we only allow the tainting to get more restrictive, but here we actually need to set to an explicit level. It can go from more-tainted to less-tainted. The situation where this is allowed is: 1. Page controlled by a service worker does a cross-origin no-cors request 2. Service worker intercepts and passes a basic Response to FetchEvent.respondWith() 3. Resulting page should see the basic Response instead of an opaque Response It works this way because in the spec the Response passed to respondWith() should be passed directly back to the outer window. Our implementation, however, has to synthesize the Response back on to the original nsIChannel. So we end up overwriting the original tainting value.

Comment on attachment 8873970 [details] [diff] [review] P2 Make the FetchEvent.respondWith() synthesize the exact tainting on the outer nsIChannel. r=asuth This patch uses the new nsILoadInfo API to override the tainting value when we synthesize the response on the nsIChannel.

Update the WPT test expectations.

Comment on attachment 8874593 [details] [diff] [review] P4 Update test_fetch_event.html expectations for synthesized CORS response returned for outer no-cors request. r=asuth Update our mochitests to expect the new behavior.

Comment on attachment 8874593 [details] [diff] [review] P4 Update test_fetch_event.html expectations for synthesized CORS response returned for outer no-cors request. r=asuth Review of attachment 8874593 [details] [diff] [review]: ----------------------------------------------------------------- restating: the "cors" response is now properly exposed as "cors" because we are able to lower the tainting from "no-cors"/"opaque".

Comment on attachment 8875011 [details] [diff] [review] P3 Update expectations on fetch-response-taint.https.html. r=asuth Review of attachment 8875011 [details] [diff] [review]: ----------------------------------------------------------------- Confirming that the removed failures were all cases of inability to lower tainting, specifically for (cross-origin) fetch requests against OTHER_ORIGIN (by a BASE_ORIGIN page): * SW responds with "same-origin" mode, "omit" creds to page fetch with * "no-cors", all 3 credential types: could not be lowered from "opaque" to "basic" * "cors", all 3 credential types: could not be lowered from "cors" to "basic" * SW responds with "same-origin" mode, "same-origin" creds to page fetch with * "no-cors", all 3 credential types: could not be lowered from "opaque" to "basic" * "cors", all 3 credential types: could not be lowered from "cors" to "basic" * SW responds with (cors-approved) "cors" mode, "omit" creds to page fetch with * "no-cors", all 3 credential types: could not be lowered from "opaque" to "cors" * SW responds with (cros-approved) "cors" mode, "include" creds to page fetch with * "no-cors", all 3 credential types: could not be lowered from "opaque" to "cors"

Comment on attachment 8873969 [details] [diff] [review] P1 Expose a SynthesizeServiceWorkerTainting() on nsILoadInfo. r=ckerschb Review of attachment 8873969 [details] [diff] [review]: ----------------------------------------------------------------- Hey Ben, I am a little worried that someone will abuse that API. I would feel more confident to just add a method to the LoadInfo and then do the following on the callsite: static_cast<mozilla::LoadInfo*>(loadInfo.get())->SynthesizeServiceWorkerTainting(tainting); For example, we don't allow changing the security flags after creation of the loadInfo, but for one case we had to allow that. So we added a method CloneWithNewSecFlags [1]. I would prefer if we use similar semantics for your tainting mechanism. Please add a 'hands off!'-comment in LoadInfo.h that no one should use it unless they know exactly what they are doing! (Please note that I will be on PTO starting tonight. I am adding f+ to this patch, you can consider this as an r+ if you incorporate my suggestion!) [1] https://dxr.mozilla.org/mozilla-central/search?q=CloneWithNewSecFlags&redirect=false

Updated per the suggestion in comment 15, so claiming r+.

I'm going to land this before bug 1370617.

Rebase for new bug landing order.

Pushed by bkelly@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/674e032ea90c P2 Make the FetchEvent.respondWith() synthesize the exact tainting on the outer nsIChannel. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/1ecdf657881e P3 Update expectations on fetch-response-taint.https.html. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/4fe11fc40572 P4 Update test_fetch_event.html expectations for synthesized CORS response returned for outer no-cors request. r=asuth

Backed out because I messed up the bug number on P1. https://hg.mozilla.org/integration/mozilla-inbound/rev/24c5cff1eadb25aba955c18a509651921dc4831e

Pushed by bkelly@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d61441b6594e P1 Expose LoadInfo::SynthesizeServiceWorkerTainting(). r=ckerschb https://hg.mozilla.org/integration/mozilla-inbound/rev/7d7d7b7c2d29 P2 Make the FetchEvent.respondWith() synthesize the exact tainting on the outer nsIChannel. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/7a4d1d742563 P3 Update expectations on fetch-response-taint.https.html. r=asuth https://hg.mozilla.org/integration/mozilla-inbound/rev/1a7b65a51a0b P4 Update test_fetch_event.html expectations for synthesized CORS response returned for outer no-cors request. r=asuth

https://hg.mozilla.org/mozilla-central/rev/d61441b6594e https://hg.mozilla.org/mozilla-central/rev/7d7d7b7c2d29 https://hg.mozilla.org/mozilla-central/rev/7a4d1d742563 https://hg.mozilla.org/mozilla-central/rev/1a7b65a51a0b