Closed Bug 1260388 Opened 8 years ago Closed 8 years ago

Web worker fails to load in sandboxed iframe with Firefox 45

Categories

(Core :: DOM: Workers, defect)

Product:
Core
Component:
DOM: Workers
Version:
45 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED INVALID
Tracking Flags:
Tracking Status
firefox45 --- affected

People

(Reporter: longsleep, Unassigned)

References

Details

(Keywords: dev-doc-complete, regression, site-compat) 

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.49 Safari/537.36

Steps to reproduce:

We try to use PDF.js inside a sandboxed iframe. We see this behavior when using pdf.js to display PDF's inside a sandboxed iframe. 



Actual results:

PDF.js fails to load the web worker with with Error: Failed to load script (nsresult = 0x805303f4)


Expected results:

The web worker should load inside a sandboxed iframe (document.domain == "").

It works fine with Firefox 44 and below and also with Chrome 50.
Correction, in Chrome 50 it actually also does not work but the following exception is catched by PDF.js

DOMException: Failed to construct 'Worker': Script at 'https://myserver/js/libs/pdf/pdf.worker.js' cannot be accessed from origin 'null' while in FF45 it fails with an uncatched exception Error: Failed to load script (nsresult = 0x805303f4) in <unknown>
Component: Untriaged → DOM: Workers
Product: Firefox → Core
Do you have a public URL available for testing?
status-firefox45: --- → affected
Flags: needinfo?(longsleep)
Keywords: regression, regressionwindow-wanted, site-compat
Worker scripts are required to be same-origin.  A sandboxed iframe has a unique origin that won't match anything.  So I think its expected that new Worker() does not function in a sandboxed iframe.

You could try <iframe sandbox="allow-same-origin">, but please be aware you are losing a lot of the protection from the sandbox that way.
Yes I tend to agree. Please confirm that this essentially means Web workers can not be used in a sanboxed iframe.

Also new Worker does not fail in Firefox. It fails some time later with error in unknown which is why PDF.js fails to catch this.
Flags: needinfo?(longsleep)
(In reply to Simon Eisenmann from comment #5)
> Also new Worker does not fail in Firefox. It fails some time later with
> error in unknown which is why PDF.js fails to catch this.

Well, the Worker constructor cannot fail itself since script loading is async.  It should fire the Worker.onerror event, though.  Does it not fire that event?
(In reply to Ben Kelly [:bkelly] from comment #6)
> (In reply to Simon Eisenmann from comment #5)
> > Also new Worker does not fail in Firefox. It fails some time later with
> > error in unknown which is why PDF.js fails to catch this.
> 
> Well, the Worker constructor cannot fail itself since script loading is
> async.  It should fire the Worker.onerror event, though.  Does it not fire
> that event?


The error event is triggered in Firefox 45 yes. But this does not prevent the general Error in unknown. Also having "new Worker" not failing is different behavior from Chrome and is not what is expected by PDF.js.

I create a simple test page at: https://www.stdin.xyz/downloads/people/longsleep/tmp/testcases/ff1260388/test1.html

In FF45:
worker onerror error { target: Worker, isTrusted: true, message: "Error: Failed to load script (nsresult = 0x805303f4)", filename: "", lineno: 0, colno: 0, currentTarget: Worker, eventPhase: 2, bubbles: false, cancelable: true, defaultPrevented: false }
Error: Failed to load script (nsresult = 0x805303f4)

In Chrome 49:
Uncaught SecurityError: Failed to construct 'Worker': Script at 'https://www.stdin.xyz/downloads/people/longsleep/tmp/testcases/ff1260388/test1-worker.js' cannot be accessed from origin 'null'.

The behavior from Chrome is what i would expect in FF45 as well.
See Also: → 1260961
Firefox: 48.0a1, Build ID: 20160331030231
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0

Hi,

I have tested this issue on the latest Firefox (45.0.1 - Build ID: 20160315153207) release, latest Nightly (48.0a1) and I have managed to reproduce it. I have also tested this on Firefox v 44.0 and this message showed up in the console: 
Sandbox loaded ...     test1-sandbox.html:5:1
SecurityError: The operation is insecure.           test1-sandbox.html:6:0

Reproducible also on windows 10 x64 and Ubuntu 14.04 x64. So I have performed a regression and this are the results:
Last good revision: 7d5a2743cc47f60e08dcd115e10f0130862eb5df
First bad revision: 6c6b3811aeb7472cb0de572ce1a456d07756be9a
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7d5a27
43cc47f60e08dcd115e10f0130862eb5df&tochange=6c6b3811aeb7472cb0de572ce1a456d07756
be9a

Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1218433

Thanks,
Cosmin.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 1218433
Flags: needinfo?(amarchesini)
I'm confused.  Why are we treating this like a regression?  It seems we were out-of-spec before allowing a worker in a sandboxed iframe.  AFAICT, we are now more spec compliant and compatible with other browsers.
(In reply to Ben Kelly [:bkelly] from comment #9)
> I'm confused.  Why are we treating this like a regression?  It seems we were
> out-of-spec before allowing a worker in a sandboxed iframe.  AFAICT, we are
> now more spec compliant and compatible with other browsers.

The regression is that the error is not properly raised when calling "new Worker", instead an uncatchable exception is raised some time later in <unknown>.
(In reply to Simon Eisenmann from comment #10)
> The regression is that the error is not properly raised when calling "new
> Worker", instead an uncatchable exception is raised some time later in
> <unknown>.

But per the spec it shouldn't throw in the Worker constructor.  It checks this as part of the network fetching which is performed asynchronously.  It has to check during the network fetching because the initial URL could be same-origin, but redirect to a cross-origin URL.
It should fire "a simple event named error" on the object returned from new Worker().  So you should be able to do worker.onerror = function(evt) { } to handle the error.  Does that not work?
Also, are you saying if you don't handle the onerror event on the worker object its bubbling up to the iframe?  As far as I can tell it should not bubble.  So that would be a bug.
(In reply to Ben Kelly [:bkelly] from comment #13)
> Also, are you saying if you don't handle the onerror event on the worker
> object its bubbling up to the iframe?  As far as I can tell it should not
> bubble.  So that would be a bug.

Yes thats that i am saying. It onerror with { target: Worker, isTrusted: true, message: "Error: Failed to load script (nsresult = 0x805303f4)", filename: "", lineno: 0, colno: 0, currentTarget: Worker, eventPhase: 2, bubbles: false, cancelable: true, defaultPrevented: false }, but it also raises the Error: Failed to load script (nsresult = 0x805303f4) in <unknown>. I did not find a way to catch that one.

If the spec says that new Worker should not fail and error handling should be done with onerror then fine, but the uncatchable Error looks like a bug.

See the console log when loading https://www.stdin.xyz/downloads/people/longsleep/tmp/testcases/ff1260388/test1.html
It needs to be documented anyway because Firefox 45 breaks something here. This case might be INVALID as :bkelly explains, while Bug 1260961 seems a "real" regression.
Keywords: dev-doc-needed
Code is like this:

var worker = new Worker('./test1-worker.js');
worker.addEventListener('error', function(err) {
  console.log('worker onerror', err);
}, false);

Run that in a sandboxed iframe, onerror is triggered and afterwards another Error: Failed to load script (nsresult = 0x805303f4) <unknown>

In older Firefox version and in Chrome, new Woker raises a Security exception directly (which is catched by PDF.js).
Yeah, I think the issue is that we do the behavior described at https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2 for load errors too.  This came up in bug 1254125.

As a work around, you can call preventDefault on the event in the worker's onerror handler and that will stop it from bubbling.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #17)
> Yeah, I think the issue is that we do the behavior described at
> https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2
> for load errors too.  This came up in bug 1254125.
> 
> As a work around, you can call preventDefault on the event in the worker's
> onerror handler and that will stop it from bubbling.

Yes that works, added a test2 which adds preventDefault - that way only the error handler is called. See https://www.stdin.xyz/downloads/people/longsleep/tmp/testcases/ff1260388/test2.html

So i think the only real issue is to add this to the docs as it changes the FF behavior and PDF.js needs an update to support this.
Based on my analysis, Bug 1260961 and this bug are basically duplicate.

* |new Worker| no longer throws when the worker failed to load
* PDF.js doesn't have the onerror handler for the worker
* The fallback code in PDF.js will never get called
There is already a pull request to add the onerror handler:
https://github.com/mozilla/pdf.js/pull/7107
Closing this bug itself as INVALID. Let's track the PDF.js issue in Bug 1260961.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(amarchesini)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.