Skip to content

Commit

Permalink
Set base URL of cross-origin classic scripts to about:blank
Browse files Browse the repository at this point in the history
crbug.com/1074317 changed the base URL from response URL
to request URL to hide cross-origin response URLs.

This CL

- Changes the base URL back to response URL again, and
- Hides the response URL by setting the base URL
  to about:blank if cross-origin, instead.
- Uses `kDoNotSanitize` in
  LocalFrameMojoHandler::JavaScriptExecuteRequestForTests()
  because relative path dynamic imports are performed there in
  some ChromeOS browser tests.

Bug: 1082086, whatwg/html#5751
Change-Id: Ifec44c5242db829bfc1ae8d143897e08564c15a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2707694
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/main@{#934466}
  • Loading branch information
hiroshige-g authored and Chromium LUCI CQ committed Oct 25, 2021
1 parent f112344 commit 09e82ae
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -883,18 +883,19 @@ void LocalFrameMojoHandler::JavaScriptExecuteRequestForTests(

v8::HandleScope handle_scope(V8PerIsolateData::MainThreadIsolate());
v8::Local<v8::Value> result;

// `kDoNotSanitize` is used because this is only for tests and some tests
// need `kDoNotSanitize` for dynamic imports.
ClassicScript* script = ClassicScript::CreateUnspecifiedScript(
javascript, SanitizeScriptErrors::kDoNotSanitize);

if (world_id == DOMWrapperWorld::kMainWorldId) {
result = ClassicScript::CreateUnspecifiedScript(javascript)
->RunScriptAndReturnValue(DomWindow());
result = script->RunScriptAndReturnValue(DomWindow());
} else {
CHECK_GT(world_id, DOMWrapperWorld::kMainWorldId);
CHECK_LT(world_id, DOMWrapperWorld::kDOMWrapperWorldEmbedderWorldIdLimit);
// Note: An error event in an isolated world will never be dispatched to
// a foreign world.
result =
ClassicScript::CreateUnspecifiedScript(
javascript, SanitizeScriptErrors::kDoNotSanitize)
->RunScriptInIsolatedWorldAndReturnValue(DomWindow(), world_id);
script->RunScriptInIsolatedWorldAndReturnValue(DomWindow(), world_id);
}

if (wants_result) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ ClassicScript* ClassicPendingScript::GetSource(const KURL& document_url) const {
//
// <spec href="https://html.spec.whatwg.org/C/#concept-script-base-url">
// ... the URL from which the script was obtained, ...</spec>
const KURL& base_url = source_code.Url();
const KURL& base_url = resource->GetResponse().ResponseUrl();
return MakeGarbageCollected<ClassicScript>(
source_code, base_url, options_,
resource->GetResponse().IsCorsSameOrigin()
Expand Down
24 changes: 24 additions & 0 deletions third_party/blink/renderer/core/script/classic_script.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,30 @@

namespace blink {

namespace {

KURL SanitizeBaseUrl(const KURL& raw_base_url,
SanitizeScriptErrors sanitize_script_errors) {
// https://html.spec.whatwg.org/C/#creating-a-classic-script
// 2. If muted errors is true, then set baseURL to about:blank.
// [spec text]
if (sanitize_script_errors == SanitizeScriptErrors::kSanitize) {
return BlankURL();
}

return raw_base_url;
}

} // namespace

ClassicScript::ClassicScript(const ScriptSourceCode& script_source_code,
const KURL& base_url,
const ScriptFetchOptions& fetch_options,
SanitizeScriptErrors sanitize_script_errors)
: Script(fetch_options, SanitizeBaseUrl(base_url, sanitize_script_errors)),
script_source_code_(script_source_code),
sanitize_script_errors_(sanitize_script_errors) {}

ClassicScript* ClassicScript::CreateUnspecifiedScript(
const ScriptSourceCode& script_source_code,
SanitizeScriptErrors sanitize_script_errors) {
Expand Down
9 changes: 3 additions & 6 deletions third_party/blink/renderer/core/script/classic_script.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,10 @@ class CORE_EXPORT ClassicScript final : public Script {
// For scripts specified in the HTML spec.
// Please leave spec comments and spec links that explain given argument
// values at callers.
ClassicScript(const ScriptSourceCode& script_source_code,
ClassicScript(const ScriptSourceCode&,
const KURL& base_url,
const ScriptFetchOptions& fetch_options,
SanitizeScriptErrors sanitize_script_errors)
: Script(fetch_options, base_url),
script_source_code_(script_source_code),
sanitize_script_errors_(sanitize_script_errors) {}
const ScriptFetchOptions&,
SanitizeScriptErrors);

// For scripts not specified in the HTML spec.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,14 @@ void DynamicModuleResolver::ResolveDynamically(
if (!url.IsValid()) {
error_message = "Failed to resolve module specifier '" +
module_request.specifier + "'";
if (referrer_info.BaseURL().IsAboutBlankURL() &&
base_url.IsAboutBlankURL()) {
error_message =
error_message +
". The base URL is about:blank because import() is called from a "
"CORS-cross-origin script.";
}

} else {
error_message = "\"" + module_request.GetModuleTypeString() +
"\" is not a valid module type.";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ void WorkerGlobalScope::ImportScriptsInternal(const Vector<String>& urls) {
ClassicScript* script = MakeGarbageCollected<ClassicScript>(
ScriptSourceCode(source_code, ScriptSourceLocationType::kUnknown,
handler, script_url),
script_url /* base_url */, ScriptFetchOptions(),
response_url /* base_url */, ScriptFetchOptions(),
sanitize_script_errors);

// Step 5.2: "Run the classic script script, with the rethrow errors
Expand Down

This file was deleted.

This file was deleted.

0 comments on commit 09e82ae

Please sign in to comment.