Enforce strict MIME type checks for worker-imported scripts. #4001

mikewest · 2018-09-05T06:30:22Z

This addresses a portion of the proposal in #3255. Tentative tests have
landed in 1.

This addresses a portion of the proposal in #3255. Tentative tests have landed in [1]. [1]: https://github.com/web-platform-tests/wpt/blob/master/workers/importscripts_mime.tentative.any.js

mikewest · 2018-09-05T06:31:33Z

Chrome is aiming to ship this behavior in Chrome 71 (https://chromium-review.googlesource.com/c/chromium/src/+/1206270). Intent to Remove thread with discussion and data at https://groups.google.com/a/chromium.org/d/msg/blink-dev/35t5cJQ3J_Q/FH45dl0vAwAJ.

Intent to Remove: https://groups.google.com/a/chromium.org/d/msg/blink-dev/35t5cJQ3J_Q/FH45dl0vAwAJ PR against HTML: whatwg/html#4001 Bug: 794548 Change-Id: I6a310aeeba12bc427062169c6397621725eb9bbd Reviewed-on: https://chromium-review.googlesource.com/1206270 Commit-Queue: Mike West <mkwst@chromium.org> Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org> Cr-Commit-Position: refs/heads/master@{#588854}

domenic

Spec text LGTM.

Given that this has multi-implementer interest per #3255, when do you think is a good time to land this and convert the tests to non-tentative? Perhaps wait until it hits Chrome stable and hasn't been reverted? Or do it now?

domenic · 2018-09-05T15:37:18Z

source

+     <li><p><var>response</var>'s <span data-x="concept-response-status">status</span> is not an
+     <span>ok status</span></p></li>
+
+     <li>


Nit: <p> should be on same line as <li>, with indentation the same as the previous two bullets.

annevk · 2018-09-06T07:59:14Z

To clarify OP, this would only affect importScripts()?

mikewest · 2018-09-06T08:05:59Z

To clarify OP, this would only affect importScripts()?

Correct. That's the subset we have solid numbers for, and that looks like we can tighten without visible impact on users. new Worker() and <script> still look a bit out of reach (though I'm hopeful about the former).

mikewest · 2018-09-06T08:06:46Z

Given that this has multi-implementer interest per #3255, when do you think is a good time to land this and convert the tests to non-tentative? Perhaps wait until it hits Chrome stable and hasn't been reverted? Or do it now?

Waiting until it hits Chrome stable and sticks seems reasonable. I just wanted to make sure we had a PR up clarifying the behavior Chrome aims to ship.

domenic · 2018-09-06T14:47:11Z

OK, cool. I will set a calendar reminder for December 4 and we can check back in then.

domenic · 2018-12-16T19:35:30Z

My calendar reminder fired! @mikewest, any problems to report? If not, let's merge this, rename any relevant web platform tests from .tentative to not, and file bugs on all the other browsers.

mikewest · 2018-12-17T08:59:02Z

Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1514680
WebKit: https://bugs.webkit.org/show_bug.cgi?id=192749

I'll rename the tests in a separate patch.

Supporting whatwg/html#4001.

…ts()', a=testonly Automatic update from web-platform-tests Ship strict MIME checks for 'importScripts()' (#14557) Supporting whatwg/html#4001. -- wpt-commits: cac03f7400d61442996918e051671b2267ef2409 wpt-pr: 14557 --HG-- rename : testing/web-platform/tests/workers/importscripts_mime.tentative.any.js => testing/web-platform/tests/workers/importscripts_mime.any.js

…ts()', a=testonly Automatic update from web-platform-tests Ship strict MIME checks for 'importScripts()' (#14557) Supporting whatwg/html#4001. -- wpt-commits: cac03f7400d61442996918e051671b2267ef2409 wpt-pr: 14557

…ts()', a=testonly Automatic update from web-platform-tests Ship strict MIME checks for 'importScripts()' (#14557) Supporting whatwg/html#4001. -- wpt-commits: cac03f7400d61442996918e051671b2267ef2409 wpt-pr: 14557 UltraBlame original commit: d9ab60a2ad83c0c45666f6f988f92efe6297d0c8

Kaiido · 2023-02-09T07:20:35Z

Were local URLs supposed to be ignored here?
It seems both Chrome and Firefox do, while Safari and if I read correctly, the specs, don't: data: URL, and blob: URL.

(Wondering what kind of issue should be opened, and where).

annevk · 2023-02-09T08:07:31Z

@Kaiido opening an issue against whatwg/html would be a good start. And if you want you could add test coverage to WPT for the specified behavior. And maybe @mikewest and @evilpie can chime in why their implementations don't match the specification.

mikewest · 2023-02-09T08:24:28Z

https://chromium-review.googlesource.com/c/chromium/src/+/1917528 says Chrome's exclusion of blob:, filesystem:, and data: was intentional to match Firefox. I don't imagine this is too common, but we'd need to gather some metrics to determine whether we can change the behavior safely.

Kaiido · 2023-02-09T08:26:06Z

I opened #8869 and I'll look into writing new tests based on https://github.com/web-platform-tests/wpt/blob/master/workers/importscripts_mime.any.js

…pts, r=smaug This patch introduces a new pref: dom.workers.importScripts.enforceStrictMimeType which is enabled by default on Nightly see: whatwg/html#8869, whatwg/html#4001 Differential Revision: https://phabricator.services.mozilla.com/D173149

Enforce strict MIME type checks for worker-imported scripts.

a06ee9c

This addresses a portion of the proposal in #3255. Tentative tests have landed in [1]. [1]: https://github.com/web-platform-tests/wpt/blob/master/workers/importscripts_mime.tentative.any.js

domenic approved these changes Sep 5, 2018

View reviewed changes

domenic added topic: workers topic: script normative change labels Sep 5, 2018

mikewest added a commit to web-platform-tests/wpt that referenced this pull request Dec 17, 2018

Ship strict MIME checks for 'importScripts()'

b9ad458

Supporting whatwg/html#4001.

mikewest mentioned this pull request Dec 17, 2018

Ship strict MIME checks for 'importScripts()' web-platform-tests/wpt#14557

Merged

mikewest added a commit to web-platform-tests/wpt that referenced this pull request Dec 17, 2018

Ship strict MIME checks for 'importScripts()' (#14557)

cac03f7

Supporting whatwg/html#4001.

domenic merged commit d1c3679 into master Dec 18, 2018

domenic deleted the importScripts-mime branch December 18, 2018 22:19

Kaiido mentioned this pull request Feb 9, 2023

Should strict MIME type for importScripts() also be enforced on local URLs? #8869

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enforce strict MIME type checks for worker-imported scripts. #4001

Enforce strict MIME type checks for worker-imported scripts. #4001

mikewest commented Sep 5, 2018 •

edited by pr-preview bot

mikewest commented Sep 5, 2018

domenic left a comment

domenic Sep 5, 2018

annevk commented Sep 6, 2018

mikewest commented Sep 6, 2018

mikewest commented Sep 6, 2018

domenic commented Sep 6, 2018

domenic commented Dec 16, 2018

mikewest commented Dec 17, 2018

Kaiido commented Feb 9, 2023

annevk commented Feb 9, 2023

mikewest commented Feb 9, 2023

Kaiido commented Feb 9, 2023

Enforce strict MIME type checks for worker-imported scripts. #4001

Enforce strict MIME type checks for worker-imported scripts. #4001

Conversation

mikewest commented Sep 5, 2018 • edited by pr-preview bot

mikewest commented Sep 5, 2018

domenic left a comment

Choose a reason for hiding this comment

domenic Sep 5, 2018

Choose a reason for hiding this comment

annevk commented Sep 6, 2018

mikewest commented Sep 6, 2018

mikewest commented Sep 6, 2018

domenic commented Sep 6, 2018

domenic commented Dec 16, 2018

mikewest commented Dec 17, 2018

Kaiido commented Feb 9, 2023

annevk commented Feb 9, 2023

mikewest commented Feb 9, 2023

Kaiido commented Feb 9, 2023

mikewest commented Sep 5, 2018 •

edited by pr-preview bot