Closed Bug 1471805 Opened 6 years ago Closed 7 months ago

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

Categories

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

61 Branch
defect

Tracking

()

RESOLVED WORKSFORME
Webcompat Priority P1

People

(Reporter: pokechu022, Assigned: edenchuang)

Details

Attachments

(1 obsolete file)

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.
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.
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).
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.
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.
Priority: -- → P3
Severity: normal → S3

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.

Webcompat Priority: --- → ?
Webcompat Priority: ? → P1

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.

Flags: needinfo?(jjalkanen)

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

Severity: S3 → S2
Flags: needinfo?(jmarshall)
Flags: needinfo?(jjalkanen)
Flags: needinfo?(echuang)
Priority: P3 → P2
Flags: needinfo?(jmarshall)
Assignee: nobody → echuang
Flags: needinfo?(echuang)

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

Attachment #9353900 - Attachment is obsolete: true

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

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

Status: UNCONFIRMED → RESOLVED
Closed: 7 months ago
Resolution: --- → WORKSFORME

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.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: