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

Status: RESOLVED FIXED

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

Description

[Tom S [:evilpie]]

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.

Comment 1

[Boris Zbarsky [:bzbarsky]]

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...

Comment 2

[Anne (: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.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: bug1330682-module-cors-behaviour

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.

Comment 4

[Boris Zbarsky [:bzbarsky]]

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.

Comment 5

[Jon Coppeard (:jonco)]

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).

Comment 6

[Domenic Denicola]

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 7

[Ben Kelly [:bkelly, not reviewing]]

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.

Comment 8

[Tom S [:evilpie]]

Can we land this?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bac40d5a6356
Use CORS when loading modules as per current spec r=bkelly

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bac40d5a6356