Closed Bug 1330682 Opened 8 years ago Closed 8 years ago

Implement <script type="module"> CORS behavior.

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: evilpie, Assigned: jonco)

References

Details

Attachments

(1 file)

Unlike classic scripts, module scripts require the use of the CORS protocol for cross-origin fetching. There was some discussion about not doing this in https://github.com/whatwg/html/issues/1888, but that is closed and done. We should not introduce a new security hole here.
Hmm. So I guess there are two actual requirements here: 1) Per https://html.spec.whatwg.org/multipage/scripting.html#script-processing-model step 15 the credentials mode must be "omit" if there is no "crossorigin" attribute on the <script>. 2) Then we land in https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-module-script-tree which calls into https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure and thence in <https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-single-module-script>. This does a request with mode set to "cors". Given requirement 2, I' mot sure why we use "omit" as the credentials mode in the "no crossorigin attribute" case. Seems to me like "same-origin" might have made more sense...
Flags: needinfo?(annevk)
It is intentional to align with fetch(), which also defaults to "omit" / "cors", but it's indeed a little funky due to HTML's naming of the attribute. I suggest we continue with the specification for now and if we get sufficient feedback that it doesn't make sense we can still change, since not sending credentials is the more conservative behavior.
Flags: needinfo?(annevk)
Here's a patch for this. The relevant parts of the spec are steps 14, 15 and 20 from: https://html.spec.whatwg.org/multipage/scripting.html#script-processing-model I'd like to add some tests for this - do you know how I might exercise this with a mochitest? I couldn't find any existing tests for script CORS behaviour with the crossorigin attribute.
Assignee: nobody → jcoppeard
Attachment #8826550 - Flags: review?(bkelly)
You would exercise this with a web platform test, ideally. That said, dom/base/test/test_bug696301-1.html and dom/base/test/test_bug696301-2.html are some existing tests for the behavior. But they're using a very lightweight setup, which can't detect whether cookies are being sent (which you'd need to test the "omit" behavior, right?). We also have testing/web-platform/tests/html/semantics/scripting-1/the-script-element/script-crossorigin-network.html and testing/web-platform/tests/html/semantics/scripting-1/the-script-element/script-crossorigin.html and the first of those seems to have a setup for having the server report back whether credentials were sent. So cloning that one for modules might be a good approach. I can try to port the mochitests mentioned above to web platform tests so we have testing of the actual "loosen the cross-origin behavior" bit in wpt as well; at that point you could likewise clone them for modules. Or you can clone the mochitests for modules and then I'll convert all the mochitests at once; your call.
I'm going to enable module scripts for all content, preffed off by default, which will allow us to test this using web platform tests (WPT don't support testing chrome-only features).
Depends on: 1330657
WebKit has a lot of tests for this, as does Edge. I am trying to collate all the tests together into a document today. Is this thread the best place to ping when I do?
Comment on attachment 8826550 [details] [diff] [review] bug1330682-module-cors-behaviour Review of attachment 8826550 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. For tests you can use a .py in wpt tests like this: https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/tests/service-workers/service-worker/resources/fetch-access-control.py You can look where we use that for other CORS tests in service-workers and fetch.
Attachment #8826550 - Flags: review?(bkelly) → review+
Can we land this?
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/bac40d5a6356 Use CORS when loading modules as per current spec r=bkelly
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: