Issue 1133238 in chromium: initiating script is not plumbed around setTimeout(), resulting in wrong base URL, credentials mode, nonce etc. in dynamic import

6 views
Skip to first unread message

hiroshige via monorail

unread,
Aug 2, 2021, 12:07:39 AM8/2/21
to modul...@chromium.org
Updates:
Cc: kou...@chromium.org modul...@chromium.org

Comment #6 on issue 1133238 by hiro...@chromium.org: initiating script is not plumbed around setTimeout(), resulting in wrong base URL, credentials mode, nonce etc. in dynamic import
https://bugs.chromium.org/p/chromium/issues/detail?id=1133238#c6

Issue 773622 has been merged into this issue.

--
You received this message because:
1. You were specifically CC'd on the issue

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Reply to this email to add a comment or make updates.

Git Watcher via monorail

unread,
Aug 2, 2021, 2:37:09 AM8/2/21
to modul...@chromium.org

Comment #7 on issue 1133238 by Git Watcher: initiating script is not plumbed around setTimeout(), resulting in wrong base URL, credentials mode, nonce etc. in dynamic import
https://bugs.chromium.org/p/chromium/issues/detail?id=1133238#c7

The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/1fa592567517b9259f48af5e91b95b43b9adb5da

commit 1fa592567517b9259f48af5e91b95b43b9adb5da
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Mon Aug 02 06:36:05 2021

[WPT] Convert TestExpectations to -expected.txt in dynamic-import

To clarify which subtests are failing, to prepare tests and code
changes in following CLs.

Bug: 1133238, 1235202
Change-Id: I617581e49eff1aa13e15badecc6f9b51097cd23f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3066146
Auto-Submit: Hiroshige Hayashizaki <hiro...@chromium.org>
Commit-Queue: Kouhei Ueno <kou...@chromium.org>
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#907462}

[modify] https://crrev.com/1fa592567517b9259f48af5e91b95b43b9adb5da/third_party/blink/web_tests/TestExpectations
[add] https://crrev.com/1fa592567517b9259f48af5e91b95b43b9adb5da/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-classic-expected.txt
[add] https://crrev.com/1fa592567517b9259f48af5e91b95b43b9adb5da/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-module-expected.txt
[add] https://crrev.com/1fa592567517b9259f48af5e91b95b43b9adb5da/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-classic-expected.txt
[add] https://crrev.com/1fa592567517b9259f48af5e91b95b43b9adb5da/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-module-expected.txt

Git Watcher via monorail

unread,
Aug 2, 2021, 4:36:12 PM8/2/21
to modul...@chromium.org

Comment #9 on issue 1133238 by Git Watcher: initiating script is not plumbed around setTimeout(), resulting in wrong base URL, credentials mode, nonce etc. in dynamic import
https://bugs.chromium.org/p/chromium/issues/detail?id=1133238#c9


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/f914fb5081dad67c1b94c4cc5e1e015877413a10

commit f914fb5081dad67c1b94c4cc5e1e015877413a10
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Mon Aug 02 20:35:46 2021

[WPT] Distinguish Document base URL, Document URL, and inline script base URL in dynamic-import

This exposes Chromium bugs:

- The base URL of dynamic imports from event handlers is
Document URL while it should be Document base URL
(crbug.com/1235202)
- The base URL of dynamic imports from setTimeout is
Document base URL while it should be initiating script's base URL
(crbug.com/1133238)

Bug: 1133238, 1235202
Change-Id: I8d9af692af29fbbe02bdacca2a4e320c2d565f80
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3066340
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Reviewed-by: Domenic Denicola <dom...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#907686}

[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/scripts/inline-event-handlers-UA-code.js
[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/scripts/reflected-inline-event-handlers.js
[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-classic-expected.txt
[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-classic.html
[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-module-expected.txt
[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-module.html
[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-classic-expected.txt
[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-classic.html
[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-module-expected.txt
[modify] https://crrev.com/f914fb5081dad67c1b94c4cc5e1e015877413a10/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-module.html

Git Watcher via monorail

unread,
Aug 10, 2021, 8:57:12 PM8/10/21
to modul...@chromium.org

Comment #10 on issue 1133238 by Git Watcher: initiating script is not plumbed around setTimeout(), resulting in wrong base URL, credentials mode, nonce etc. in dynamic import
https://bugs.chromium.org/p/chromium/issues/detail?id=1133238#c10


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/8a7275c6166e81b5285855afcbb85b38f219bca0

commit 8a7275c6166e81b5285855afcbb85b38f219bca0
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Wed Aug 11 00:56:27 2021

Fix base URL of dynamic imports for no referencing scripts

Before this CL, the following three types of dynamic import cases
were all encoded to 0-length V8HostDefinedOptions and thus
decoded to default-constructed `ReferrerScriptInfo()`:

1. From no referencing scripts (e.g. event handlers)
2. From referencing scripts with default base URL and
default ScriptFetchOptions (e.g. normal classic scripts)
3. From referencing scripts with null base URL and
default ScriptFetchOptions (e.g. setTimeout())

And thus caused Issue 1235202.

This CL distinguishes these three, by encoding:

1. => 0-length V8HostDefinedOptions
2. => 1-length V8HostDefinedOptions
3. => full-length V8HostDefinedOptions

This CL introduces
ReferrerScriptInfo::CreateNoReferencingScript() (for 1) and
ReferrerScriptInfo::CreateWithReferencingScript() (for 2 and 3)
to clarifies this semantics.

This CL fixes the behavior for Case 1:
Previously the base URL fell back
v8::ScriptOrModule::GetResourceName(), but after this CL
the base URL falls back to the ExecutionContext's base URL,
which is spec conformant.

This CL preserves the behavior for 3:
The base URL is the ExecutionContext's base URL.
This CL adds fallback to the ExecutionContext's base URL
even when there is a referencing script
in DynamicModuleResolver::ResolveDynamically(),
which is not in the spec.
In the future, we might refactor this condition after
Issue 1133238 is fixed and setTimeout() is moved to Case 2.

Bug: 1235202, 1114993, 1133238
Change-Id: I943298b6d9f32037f295e01be3f29985a1444936
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2737783
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#910604}

[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/bindings/core/v8/module_record.cc
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/bindings/core/v8/referrer_script_info.cc
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/bindings/core/v8/referrer_script_info.h
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/bindings/core/v8/referrer_script_info_test.cc
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/bindings/core/v8/script_streamer_test.cc
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/core/script/dynamic_module_resolver.cc
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/renderer/core/script/dynamic_module_resolver_test.cc
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-classic-expected.txt
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-module-expected.txt
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-classic-expected.txt
[modify] https://crrev.com/8a7275c6166e81b5285855afcbb85b38f219bca0/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-module-expected.txt

Git Watcher via monorail

unread,
Sep 3, 2021, 2:38:05 AM9/3/21
to modul...@chromium.org

Comment #11 on issue 1133238 by Git Watcher: initiating script is not plumbed around setTimeout(), resulting in wrong base URL, credentials mode, nonce etc. in dynamic import
https://bugs.chromium.org/p/chromium/issues/detail?id=1133238#c11


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/132e52980bfcbe97b48b4c781418ee5b1a90f810

commit 132e52980bfcbe97b48b4c781418ee5b1a90f810
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Fri Sep 03 06:36:50 2021

Revert "Fix base URL of dynamic imports for no referencing scripts"

This reverts commit 8a7275c6166e81b5285855afcbb85b38f219bca0.

Reason for revert: The original CL caused a regression
crbug.com/1244145 by exposing underlying Issue v8:10284
(https://bugs.chromium.org/p/v8/issues/detail?id=10284).
Given that the fixes for Issue v8:10284 won't be merged to M-94,
the original CL is reverted and the revert will be merged to M-94
instead to at least prevent regressions.

Original change's description:
Bug: 1235202, 1114993, 1133238, 1244145
Change-Id: Id952429e08d35af026022ecd8d60a913f62e1588
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3141093
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Commit-Queue: Kouhei Ueno <kou...@chromium.org>
Auto-Submit: Hiroshige Hayashizaki <hiro...@chromium.org>
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#917989}

[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/bindings/core/v8/module_record.cc
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/bindings/core/v8/referrer_script_info.cc
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/bindings/core/v8/referrer_script_info.h
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/bindings/core/v8/referrer_script_info_test.cc
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/bindings/core/v8/script_streamer_test.cc
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/core/script/dynamic_module_resolver.cc
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/renderer/core/script/dynamic_module_resolver_test.cc
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-classic-expected.txt
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-module-expected.txt
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-classic-expected.txt
[modify] https://crrev.com/132e52980bfcbe97b48b4c781418ee5b1a90f810/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-module-expected.txt

Git Watcher via monorail

unread,
Sep 7, 2021, 10:39:30 PM9/7/21
to modul...@chromium.org
Updates:
Labels: merge-merged-4606 merge-merged-94

Comment #12 on issue 1133238 by Git Watcher: initiating script is not plumbed around setTimeout(), resulting in wrong base URL, credentials mode, nonce etc. in dynamic import
https://bugs.chromium.org/p/chromium/issues/detail?id=1133238#c12


The following revision refers to this bug:
https://chromium.googlesource.com/chromium/src/+/dceb6dd2708e43b1d56db258f3b68c63e22f94e9

commit dceb6dd2708e43b1d56db258f3b68c63e22f94e9
Author: Hiroshige Hayashizaki <hiro...@chromium.org>
Date: Wed Sep 08 02:38:35 2021
(cherry picked from commit 132e52980bfcbe97b48b4c781418ee5b1a90f810)


Bug: 1235202, 1114993, 1133238, 1244145
Change-Id: Id952429e08d35af026022ecd8d60a913f62e1588
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3141093
Commit-Queue: Hiroshige Hayashizaki <hiro...@chromium.org>
Commit-Queue: Kouhei Ueno <kou...@chromium.org>
Auto-Submit: Hiroshige Hayashizaki <hiro...@chromium.org>
Reviewed-by: Kouhei Ueno <kou...@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#917989}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3145438
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4606@{#850}
Cr-Branched-From: 35b0d5a9dc8362adfd44e2614f0d5b7402ef63d0-refs/heads/master@{#911515}

[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/bindings/core/v8/module_record.cc
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/bindings/core/v8/referrer_script_info.cc
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/bindings/core/v8/referrer_script_info.h
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/bindings/core/v8/referrer_script_info_test.cc
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/bindings/core/v8/script_streamer_test.cc
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/core/script/dynamic_module_resolver.cc
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/renderer/core/script/dynamic_module_resolver_test.cc
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-classic-expected.txt
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-external-module-expected.txt
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-classic-expected.txt
[modify] https://crrev.com/dceb6dd2708e43b1d56db258f3b68c63e22f94e9/third_party/blink/web_tests/external/wpt/html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-base-url-inline-module-expected.txt
Reply all
Reply to author
Forward
0 new messages