1523706 - Consider strictly enforcing MIME checks for Worker scripts

Status: RESOLVED FIXED

(Core :: DOM: Workers, enhancement, P3)

Description

[Tom S [:evilpie]]

It would be good to figure out if we could also restrict the MIME types of worker scripts, instead of just those loaded via importScripts() (Bug 1514680).

I think this issue also still applies: https://github.com/whatwg/html/issues/3255

Comment 1

[Tom S [:evilpie]]

Unless something is wrong with our telemetry I think we should at least try doing this.

Comment 2

[Tom S [:evilpie]]

Attachment: Bug 1523706 - Consider strictly enforcing MIME checks for Worker scripts. r?ckerschb

No test changes yet.

Comment 3

[Tom S [:evilpie]]

This causes quite a lot of test failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=96da6a532b3d2b2fe1e7caea6b79a1ff9f06b29b. I do think some of those might be unrelated.

Comment 4

[Tom S [:evilpie]]

Interesting find, seems like for some reason tests in WPT use polygot HTML/JS files ... !!

For example:
workers/interfaces/WorkerGlobalScope/self.html
workers/semantics/interface-objects/003.html
workers/semantics/interface-objects/004.html
workers/interfaces/WorkerUtils/WindowTimers/003.html
workers/interfaces/WorkerUtils/WindowTimers/005.html

https://searchfox.org/mozilla-central/search?q=new+(Shared)%3FWorker..%23&case=false&regexp=true&path=

Comment 5

[Tom S [:evilpie]]

Should we change those tests or just disable them for us?

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Tom Schuster [:evilpie] from comment #5)

Should we change those tests or just disable them for us?

From my quick check it seems those usages are not intentional by the authors of the tests and I think we should update them.

Comment 7

[Tom S [:evilpie]]

I am waiting for review to update some of the WPT tests, but at least a few need to be disabled, because they just don't work when blocking other MIME types.

Comment 8

[Tom S [:evilpie]]

Attachment: Bug 1523706 - Use JavaScript mime type for two worker tests. r?ckerschb

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6782caf07c7d
Use JavaScript mime type for two worker tests. r=ckerschb

Comment 10

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/6782caf07c7d

Comment 11

[Tom S [:evilpie]]

Attachment: Bug 1523706 - Disable WPT Worker tests that require a non JavaScript mime. r?ckerschb

Depends on D32806

Comment 12

[Tom S [:evilpie]]

Attachment: Bug 1523706 - Extend devtools test. r?ckerschb

Depends on D37911

Comment 13

[Tom S [:evilpie]]

I am going to send an Intent to Ship for this, considering that Chrome seems disinclined to implement this.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8fcae0a0d731
Consider strictly enforcing MIME checks for Worker scripts. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/122642699fc5
Disable WPT Worker tests that require a non JavaScript mime. r=ckerschb
https://hg.mozilla.org/integration/autoland/rev/09edf04895b6
Extend devtools test. r=ckerschb

Comment 15

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/8fcae0a0d731
https://hg.mozilla.org/mozilla-central/rev/122642699fc5
https://hg.mozilla.org/mozilla-central/rev/09edf04895b6

Comment 16

[Kohei Yoshino]

Posted site compatibility note: https://www.fxsitecompat.dev/en-CA/docs/2019/worker-scripts-with-wrong-mime-type-will-be-blocked-from-loading-with-worker-or-sharedworker/

Comment 17

[Boris Zbarsky [:bzbarsky]]

Is there a bug tracking re-enabling the tests that got disabled here?

Comment 18

[Tom S [:evilpie]]

Yes, see bug 1569123.

Comment 19

[Boris Zbarsky [:bzbarsky]]

That's about re-enabling the type checks (which got disabled in bug 1569122). I am talking about things like https://searchfox.org/mozilla-central/rev/153feabebc2d13bb4c29ef8adf104ec1ebd246ae/testing/web-platform/meta/workers/Worker_script_mimetype.htm.ini#2 which disabled various tests.

Comment 20

[Tom S [:evilpie]]

Attachment: Bug 1523706 - Backout changes to renable Worker MIME tests. r?bzbarsky

Comment 21

[Tom S [:evilpie]]

hg backout 122642699fc5 worked. See review request.

Comment 22

[Tom S [:evilpie]]

ni myself to fix those tests.

Comment 23

[Tom S [:evilpie]]

What should I do about workers/Worker_script_mimetype.htm? It seems like that test uses plaintext on purpose.

Comment 24

[Boris Zbarsky [:bzbarsky]]

That one we should mark as failing on the branches where we have the strict MIME type enforcement (and keep testing that it passes on the branches where we do not have that turned on).

Comment 25

[James Graham [:jgraham]]

Do we actually have branches with the test changes and not the code/pref changes? Is that situation expected to persist (i.e. is there some part of this that isn't expected to ride the trains?). Typically for things that are on on nightly but not on beta/release we either set the pref unconditionally so the feature is on on all branches, or annotate things like

if release_or_beta: PASS
FAIL

It sounds like the first option is undesirable here, so the second might be possible. If there are differences between release and beta that need to be annotated I'm not sure what you can do; I can't see a related mozinfo field, but I may be overlooking something obvious.

Comment 26

[Chris Mills [:cmills]]

I've documented this; see https://github.com/mdn/sprints/issues/2121#issuecomment-541665289 for the full details.

Comment 27

[Tom S [:evilpie]]

Opened bug 1595297 to re-enable those tests.