1471805 - Webworker violating cross-origin policy silently ignored, instead of throwing a SecurityError

Status: RESOLVED WORKSFORME

(Core :: DOM: Workers, defect, P2)

Description

[pokechu022]

To reproduce:

1. Go to https://example.org, and then open the browser console.
2. Run `new Worker("https://example.org"); console.log("Foo");`.
3. Run `new Worker("https://example.com"); console.log("Bar");`.  (A different site)

Expected behavior:

The "https://example.org" request would succeed (and later fail due to a parse error), and "Foo" would be logged.  Then, the "https://example.com" request would be denied with a SecurityError, and execution would halt without logging "Bar".

I initially assumed that the correct behavior would be to just log a warning (as is done for XMLHttpRequests per bug 713980), but MDN actually specifies that a SecurityError should occur in this case as seen on https://developer.mozilla.org/en-US/docs/Web/API/Worker/Worker.

Actual behavior:

The "https://example.org" request behaves as expected.  However, the "https://example.com" request is denied, but execution continues ("Bar" is printed) and _no_ error or warning is logged at all.  This causes major confusion, since the worker simply never starts and it looks like nothing at all happened.

The other browsers I tested (Chrome, IE11, and Edge) all correctly threw a SecurityError.

Versions:

This issue affects the latest nightly (20180627222831), along with release 20180621125625.

The bug was not present when the information was added to the documentation (https://developer.mozilla.org/en-US/docs/Web/API/Worker/Worker$compare?to=465539&from=464957 -- 2013-09-08).

Using mozregression, I've identified specific dates where this behavior changed.  Unfortunately I couldn't get an exact commit, as looking for builds within a single day failed with "WARNING: Skipping build #####: Unable to find build info using the taskcluster route 'gecko.v2.mozilla-central.revision.#####.firefox.win64-opt'" for all versions.

On 2015-11-17, it correctly threw a SecurityError.  On 2015-11-18 this stopped being the case, instead giving the message "Error: Failed to load script (nsresult = 0x805303f4)".  At this point, "Bar" started being printed (i.e. no exception was thrown) -- this printing happened before the other message was logged (asnyc?).  Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=898c2c656e4b156c323416ef0c859915f3fd2308&tochange=eb3016abd37db2e6a6d923265047e84b12c0af61.
I note that on there, there are several commits relating to bug 1218433, which in turn references bug 1211967.  Those commits are _probably_ the underlying cause of this issue.

On 2016-02-28 it still used the "failed to load script" message.  On 2016-02-29, it started displaying "SecurityError: Failed to load worker script at "https://example.com"", but this still happened after "Bar" had been printed (I'm not sure if an actual SecurityError was raised).  Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5e0140b6d11821e0c2a2de25bc5431783f03380a&tochange=9da51cb4974e03cdd8fa45a34086fe1033abfeaf.

On 2017-02-01, it still displayed that "failed to load worker script" message.  On 2017-02-02, it stopped displaying _any_ message.  Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=fe22af79bacf6db526737536eab551caf68ba440&tochange=f985243bb630b2c78cd57731c8d8ab191aa09527.  I see references to bug 1288768, among others.  This is the current state.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

The spec does not throw a SecurityError for a cross-origin URL there.  See:

https://html.spec.whatwg.org/#dom-worker

Step 1 allows the user agent to throw if workers are disabled or something, but does not require throwing.  Step 4 throws for an invalid URL that does not parse.

The same-origin check is not performed until we get to the fetching stuff under step 9.1 which happens in parallel.  We can't throw once we go parallel.

So I think this is an MDN documentation bug.

Comment 2

[pokechu022]

I'm not entirely sure of the correct behavior then.  This is way out of what I normally do; sorry.  In any case, I still think the lack of any warning or other message when this happens is a bug that should be fixed; the current behavior is _very_ confusing.

I will still note that all other browsers I tested (Chrome, IE11, and Edge) all threw a SecurityError, and firefox also used to throw a SecurityError on 2015-11-17 and before.

The docs are definitely incorrect in another way; it says SecurityError will be used for syntax errors but that directly contradicts with the spec which says SyntaxError should be used (and the Exceptions section further down on that page also says that).

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

You can register an error event handler:

  var w = new Worker(foo);
  w.onerror = function(e) {
    // fired on cross-origin load, syntax error, etc.
  }

Also consider that this can happen from a redirect to a cross-origin URL.  That cannot be handled via synchronously throwing.

Comment 4

[pokechu022]

Hm, good to know.  Yeah, that properly gives an error (but I didn't initially know to do that, or rather the library I was using didn't do that so it was still silently failing with no indication as to why).

Good point regarding cross-origin redirects.  Other browsers don't throw a SecurityException in that case; they only do the event.  Makes sense.

Comment 5

[Thomas Wisniewski [:twisniewski]]

A Google employee just confirmed that this is actually causing a webcompat issue on some versions of Google Maps, if sites are misconfigured. Chrome will still end up loading the map because it detects a SecurityError being thrown on worker creation, while Firefox does not, and ends up hitting less tested codepaths in Maps as a result:

The site in question seems to be trying to use the webGL version of maps, which uses Web Workers. On my machine, Chrome fails this worker creation with a SecurityError exception blocking the script from running due to content security policy of the site, so Maps JS API fallback to running a png map. On Firefox, this seems to not throw, but just silently not start the workers, so Maps API tries running WebGL with workers and stalls waiting for the worker to start.

I suppose a strict reading of the spec does not require the browser to throw a SecurityError in such a case, but the spec says the user agent "may throw a 'SecurityError'DOMException if the request violates a policy decision" https://html.spec.whatwg.org/multipage/workers.html#dom-worker

I suspect for general debuggability by all application developers, it's probably valuable to throw a SecurityError in such a case of a worker script violating the content security policy of a page.

This problem can currently be seen at https://www.exploretock.com/city/seattle/search?city=Seattle&date=2022-03-29&latlng=47.6062095%2C-122.3320708&size=2&time=15%3A30&type=DINE_IN_EXPERIENCES , as reported at https://github.com/webcompat/web-bugs/issues/100874#issuecomment-1611839988 .

I will let the Google folks know to try using onerror as a work-around; hopefully that will be enough for them to fix the issue without too much trouble. But it strikes me as a good idea to match Chrome if this is demonstrably causing such webcompat breakage.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is set to S3. However, this bug has a P1 WebCompat priority.
:jjalkanen, could you consider increasing the severity of this web compatibility bug?

For more information, please visit BugBot documentation.

Comment 7

[Jari Jalkanen [:jari]]

I increase the severity for better visibility because major functionality/product is severely impaired.

Comment 8

[Eden Chuang[:edenchuang]]

Attachment: Bug 1471805 - Synchronous throwing an security error for cross-origin Worker scripts. r=asuth.

Comment 9

[Eden Chuang[:edenchuang]]

Fire a spec issue for discussion, https://github.com/whatwg/html/issues/9794

Comment 10

[Eden Chuang[:edenchuang]]

https://github.com/web-platform-tests/wpt/issues/41745

According to the discussion, I think this bug can be closed.

Comment 11

[Thomas Wisniewski [:twisniewski]]

Alright. I've just pinged my email thread with Google, as ExploreTock is still broken on Firefox (and will be on WebKit and Blink when they adopt the spec text). Hopefully the Maps team will at least handle sync and async cases until browsers align on the new spec text.

Comment 12

[Ksenia Berezina [:ksenia] on leave until 09/15]

Reopening as the map on exploretock is still broken (bug1927467).

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Eden, why did you close this? Google maps on exploretock is still broken

Comment 14

[Emilio Cobos Álvarez (:emilio)]

SecurityError: Access to the script at 'blob:https://www.exploretock.com/95014f01-fd91-41b2-a324-46149a039101' is denied by the document's Content Security Policy.
WR (webgl.js:44)
G6a (webgl.js:141)
qX (webgl.js:142)
Mg (webgl.js:1013)
(anonymous) (map.js:68)
invoke (explore.js:2)
run (explore.js:2)
(anonymous) (explore.js:2)
invokeTask (explore.js:2)
runTask (explore.js:2)
(anonymous) (explore.js:2)
v (explore.js:2)
Promise.then
g (explore.js:2)
scheduleTask (explore.js:2)
scheduleTask (explore.js:2)
(anonymous) (explore.js:2)
scheduleMicroTask (explore.js:2)
P (explore.js:2)
C (explore.js:2)
(anonymous) (explore.js:2)
(anonymous) (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1039)
(anonymous) (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1037)
Dk (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1038)
Ek (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1037)
(anonymous) (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1037)
(anonymous) (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1039)
(anonymous) (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1037)
Dk (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1038)
hba (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1037)
ul (js?key=AIzaSyDrvg7Eh2vWBDr_gnSZz7PqQF5z27uDSH0&v=3.47&libraries=places&callback=initMap:1183)
(anonymous) (webgl.js:1)

Comment 15

[Emilio Cobos Álvarez (:emilio)]

I think we should probably just fix it, doesn't seem like the kind of hill worth dying on.

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #13)

Eden, why did you close this? Google maps on exploretock is still broken

Apologies, we lost track of the exploretock bug in the thicket of bugs around this issue.

https://bugzilla.mozilla.org/show_bug.cgi?id=1919592#c7 has spec issue links but the tl;dr is that browsers are in agreement that everyone should do what Firefox is doing and Blink is moving to conform and Webkit by way of :annevk was strongly opposed to changing the spec back to what non-Firefox browsers were doing, although it's not immediately clear when they will conform.

Also note that we got this into interop-2025: https://github.com/web-platform-tests/interop/issues/855 noting that the initial set of tests was wrong and will be corrected to only be the CSP test (see https://github.com/web-platform-tests/interop/issues/932) but which is the specific case Jeff has identified in https://bugzilla.mozilla.org/show_bug.cgi?id=1927467#c3 and the WPT tests I believe have now been all updated. Browsers will not be able to get to 100% in interop-2025 without fixing their implementations.

It's probably most efficient if you engage on the spec issues directly if you want to make the case for permanent carve-outs in the fetch spec, although I should point out that the workaround Eden was pursuing for this ran into some real technical problems when it comes to nested workers which will potentially result in Firefox deadlocking where other browsers don't deadlock because we can't run our security checks OMT.