Consider strictly enforcing MIME checks for Worker scripts
Categories
(Core :: DOM: Workers, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox70 | --- | fixed |
People
(Reporter: evilpie, Assigned: evilpie)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(4 files, 1 obsolete file)
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
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
Unless something is wrong with our telemetry I think we should at least try doing this.
Assignee | ||
Comment 2•4 years ago
|
||
No test changes yet.
Assignee | ||
Comment 3•4 years ago
|
||
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.
Assignee | ||
Comment 4•4 years ago
•
|
||
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
Assignee | ||
Comment 5•4 years ago
|
||
Should we change those tests or just disable them for us?
Comment 6•4 years ago
|
||
(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.
Assignee | ||
Comment 7•4 years ago
|
||
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.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 8•4 years ago
|
||
Pushed by evilpies@gmail.com: https://hg.mozilla.org/integration/autoland/rev/6782caf07c7d Use JavaScript mime type for two worker tests. r=ckerschb
Comment 10•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
Depends on D32806
Assignee | ||
Comment 12•4 years ago
|
||
Depends on D37911
Assignee | ||
Comment 13•4 years ago
|
||
I am going to send an Intent to Ship for this, considering that Chrome seems disinclined to implement this.
Updated•4 years ago
|
Comment 14•4 years ago
|
||
Pushed by evilpies@gmail.com: 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•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 16•4 years ago
•
|
||
![]() |
||
Comment 17•4 years ago
|
||
Is there a bug tracking re-enabling the tests that got disabled here?
![]() |
||
Comment 19•4 years ago
|
||
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.
![]() |
||
Updated•4 years ago
|
Assignee | ||
Comment 20•4 years ago
|
||
Assignee | ||
Comment 21•4 years ago
|
||
hg backout 122642699fc5
worked. See review request.
Assignee | ||
Comment 23•4 years ago
|
||
What should I do about workers/Worker_script_mimetype.htm
? It seems like that test uses plaintext on purpose.
![]() |
||
Comment 24•4 years ago
|
||
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•4 years ago
|
||
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•4 years ago
|
||
I've documented this; see https://github.com/mdn/sprints/issues/2121#issuecomment-541665289 for the full details.
Assignee | ||
Comment 27•4 years ago
|
||
Opened bug 1595297 to re-enable those tests.
Updated•4 years ago
|
Description
•