Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

import() should use referring script's base URL as the referrer #3744

Closed
domenic opened this issue Jun 6, 2018 · 8 comments · Fixed by #9407
Closed

import() should use referring script's base URL as the referrer #3744

domenic opened this issue Jun 6, 2018 · 8 comments · Fixed by #9407

Comments

@domenic
Copy link
Member

domenic commented Jun 6, 2018

Right now we treat import() as a "new" module script graph, i.e. it calls fetch a module script graph directly. This then uses "client" as the referrer, i.e. it uses the base URL of the appropriate settings object.

But this seems semantically wrong. import() knows what script it's being called from; it uses that information to compute URLs and and fetch options. It should also use that to compute the referrer information.

Found by @jonco3. /cc @whatwg/modules. I will try to work on a spec fix and tests for this soon...

@nhiroki
Copy link
Contributor

nhiroki commented Jun 12, 2018

/cc @hiroshige-g @nyaxt and me. We're now adding WPTs for referrer on module workers.

@domfarolino
Copy link
Member

Any update on the status here (tests or spec)?

@domenic
Copy link
Member Author

domenic commented May 11, 2020

It looks like the spec still does not do this, but it would be much easier to change now. I think it's just a matter of changing https://html.spec.whatwg.org/multipage/webappapis.html#fetch-an-import()-module-script-graph step 3 to pass base URL instead of "client".

@nhiroki
Copy link
Contributor

nhiroki commented May 14, 2020

Regarding the tests, we have auto-generated tests for top-level worker module script loading, but not yet for import() IIUC:
https://github.com/web-platform-tests/wpt/blob/451adc0f8a2a76b97c74ff3471b0e068164e9eb8/referrer-policy/spec.src.json#L656

@nhiroki
Copy link
Contributor

nhiroki commented May 14, 2020

@nicolo-ribaudo
Copy link
Contributor

I was testing this issue, and it looks like Firefox implements the proposed behavior while Webkit and Chromium implement the specified behavior.

I was looking at wpt tests, but I couldn't find any. Would it be ok to just duplicate all the referrer-* tests in https://github.com/web-platform-tests/wpt/tree/master/html/semantics/scripting-1/the-script-element/module, to test the referrer when:

  • using static import in an imported external module
  • using dynamic import in an imported external module

Should the tests include all the referrer policy combinations, or given that this change would affect which referrer is passed and not when it would be enough to test the default one?

@annevk
Copy link
Member

annevk commented Jun 9, 2023

Duplicating (or abstracting) sounds fine. And no need to test all combinations. Thanks!

@nicolo-ribaudo
Copy link
Contributor

Thank you! PR opened.

domenic pushed a commit that referenced this issue Jul 4, 2023
Remove the explicit non-propagation of the referrer for dynamic import() calls, and fix #3744. By doing so, we align fetching static and dynamic imports.
rubberyuzu pushed a commit to rubberyuzu/html that referenced this issue Jul 20, 2023
Remove the explicit non-propagation of the referrer for dynamic import() calls, and fix whatwg#3744. By doing so, we align fetching static and dynamic imports.
rubberyuzu pushed a commit to rubberyuzu/html that referenced this issue Jul 21, 2023
Remove the explicit non-propagation of the referrer for dynamic import() calls, and fix whatwg#3744. By doing so, we align fetching static and dynamic imports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants