Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 717525 Module scripts should not be requested with credentials
Starred by 5 users Project Member Reported by jakearchibald@chromium.org, May 2 Back to list
Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 594639
issue 710837



Sign in to add a comment
https://module-script-tests-zoelmqooyv.now.sh/cookie-page

By default, module scripts should not be requested with credentials. The output of the above page should be:

No random number cookie found.
Random number cookie is: 0.30567383219441924
Random number cookie is: 0.30567383219441924

(Obviously your random number will be different).
 
Cc: adamk@chromium.org kouhei@chromium.org addyo@chromium.org
Blocking: 594639
Labels: -Pri-3 Pri-2
Status: Available
Cc: module-dev@chromium.org
Owner: hirosh...@chromium.org
Tentatively assigning this to hiroshige@, as I'm technically currently on vacation.
Status: Started
Cc: tyoshino@chromium.org
Components: Blink>Loader
Step 16 of prepare script sets module script credentials to "omit"
(correct in ScriptLoader) but in ModuleScriptLoader::Fetch() we set
CrossOriginAttributeValue cross_origin to kCrossOriginAttributeAnonymous,
for which "their credentials mode set to "same-origin"."
https://html.spec.whatwg.org/#attr-crossorigin-anonymous

I think we should call SetCrossOriginAccessControl() or something to enable
CORS while omitting credentials, but currently SetCrossOriginAccessControl() does not support credentials mode "omit":
According to spec
https://fetch.spec.whatwg.org/#http-network-or-cache-fetch
Step 4:
> Let credentials flag be set if one of
> request’s credentials mode is "include"
> request’s credentials mode is "same-origin" and request’s response tainting is "basic"
Implementation:
>  if (is_same_origin_request || use_credentials) {
>    options_.allow_credentials = kAllowStoredCredentials;
>    resource_request_.SetAllowStoredCredentials(true);

tyoshino@, what do you think?

(Next update will be next week as Loader/Network API teams on Tokyo are on public holidays this week)
Due to this bug, currently <script type=module> issues two network requests for every same-origin module script. First HTMLPreloadScanner requests it without credentials, and then ScriptLoader requests it again with credentials.

If this bug isn't easy to fix, we might want to disable preload for module scripts for now.

disabling preload for now sgtm
Components: -Blink>HTML>Modules
Blocking: 710837
Project Member Comment 11 by bugdroid1@chromium.org, May 11
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32a2d84e17b0b63b284f259260accc8dd765d26b

commit 32a2d84e17b0b63b284f259260accc8dd765d26b
Author: ksakamoto <ksakamoto@chromium.org>
Date: Thu May 11 06:54:02 2017

Disable preload for module scripts

This patch disables preload for <script type="module">, to prevent
duplicated requests due to credentials mode mismatch. In the long term,
we should teach Preload Scanner about the module script credentials mode
and re-enable preload.

BUG=717525

Review-Url: https://codereview.chromium.org/2871153002
Cr-Commit-Position: refs/heads/master@{#470853}

[modify] https://crrev.com/32a2d84e17b0b63b284f259260accc8dd765d26b/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp

#5: Yeah, SetCrossOriginAccessControl() doesn't support "omit" now. But I guess it can easily extended to support it. I'll be ooo next week but can help after that.
Comment 13 by tyoshino@chromium.org, May 24 (3 days ago)
BTW, is there any document/mail by which I can understand why it's specified to use "omit". I think I checked it before but cannot remember. Just in case some of you know.
Comment 14 by adamk@chromium.org, May 24 (3 days ago)
In https://github.com/whatwg/html/issues/1888#issuecomment-253011325, Domenic suggests that the "omit" default matches Fetch (not sure if that was the reasoning, though). There's also some more recent discussion of this issue on the spec at https://github.com/whatwg/html/issues/2557
Comment 15 by domenic@chromium.org, May 25 (2 days ago)
Yes, that was basically the reasoning. I think we should follow fetch; https://github.com/whatwg/html/issues/2557 discusses changing both together, which I would be OK with.
Sign in to add a comment