Skip to content

GitLab

T
tangled
Commit c1b6d6a2 authored by Yutaka Hirano's avatar Yutaka Hirano Committed by Commit Bot
Browse files
Options

Remove per-client CORS checks 

Those are needed because a request could match with another request made
by a foreign origin, but [1] fixed the issue. This is basically reverting
[2]. Two layout tests are added to detect regressions.

This CL changes the behavior a bit. Without this CL,

 - A response with a proper access-control-allow-origin header for a
   cross-origin request with "no-cors" mode is treated as
   non-opaque, and
 - A response without a proper access-control-allow-origin header for a
   same-origin request with "no-cors" mode redirected from a cross-origin
   resource is treated as opaque

while with this CL,

 - A response with a proper access-control-allow-origin header for a
   cross-origin request with "no-cors" mode is treated as opaque, and
 - A response without a proper access-control-allow-origin header for a
   same-origin request with "no-cors" mode redirected from a cross-origin
   resource is treated as non-opaque.

Also, before this CL, CORSStatus is calculated with an opaque origin for
top-level worklet scripts. This CL changes that: now CORSStatus is
calculated with the origin of the parent context, and so it stops muting
error messages in worklets.

1: https://chromium.googlesource.com/chromium/src.git/+/c1df004861ab704945d31a0d207bb7f4c205e60c
2: https://chromium.googlesource.com/chromium/src.git/+/fad67a5b73639d7211b24fd9bdb242e82039b765

Bug: 809350, 799477
Change-Id: If28f59f6e6ac03a4d5992cca85801231748019bd
Reviewed-on: https://chromium-review.googlesource.com/1158165
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarHiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarFredrik Söderquist <fs@opera.com>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Reviewed-by: default avatarTakashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583202}
parent f4694086
Hide whitespace changes
Inline Side-by-side
Showing with 72 additions and 34 deletions
third_party/WebKit/LayoutTests/http/tests/security/cors-check-for-cached-image.html 0 → 100644
View file @ c1b6d6a2
<!doctype html>
<html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<img src="http://localhost:8080/security/resources/cors-image.php?max-age=60" crossorigin></img>
<script>
async_test((test) => {
document.querySelector('img').addEventListener('load', () => {
const iframe = document.createElement('iframe');
iframe.sandbox = 'allow-scripts';
iframe.src = 'resources/cors-check-for-cached-image-iframe.html';
document.body.appendChild(iframe);
});
window.addEventListener('message', test.step_func_done((e) => {
assert_equals(e.data, 'PASS');
}));
}, 'We should not reuse CORS check result when requested from another origin');
</script>
third_party/WebKit/LayoutTests/http/tests/security/cors-check-for-cached-script.html 0 → 100644
View file @ c1b6d6a2
<!doctype html>
<html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="http://localhost:8080/security/resources/cors-script.php?max-age=60" crossorigin></script>
<script>
async_test((test) => {
window.addEventListener('message', test.step_func_done((e) => {
assert_equals(e.data, 'PASS');
}));
}, 'We should not reuse CORS check result when requested from another origin');
</script>
<iframe sandbox="allow-scripts allow-modals" src="resources/cors-check-for-cached-script-iframe.html"></iframe>
third_party/WebKit/LayoutTests/http/tests/security/resources/cors-check-for-cached-image-iframe.html 0 → 100644
View file @ c1b6d6a2
<!doctype html>
<img src="http://localhost:8080/security/resources/cors-image.php?max-age=60" crossorigin
onload="parent.postMessage('FAIL', '*')"
onerror="parent.postMessage('PASS', '*')">
third_party/WebKit/LayoutTests/http/tests/security/resources/cors-check-for-cached-script-iframe.html 0 → 100644
View file @ c1b6d6a2
<!doctype html>
<html>
<script src="http://localhost:8080/security/resources/cors-script.php?max-age=60" crossorigin
onload="parent.postMessage('FAIL', '*')"
onerror="parent.postMessage('PASS', '*')"></script>
third_party/WebKit/LayoutTests/http/tests/security/resources/cors-image.php 0 → 100644
View file @ c1b6d6a2
<?php
$max_age = $_GET['max-age'];
if (!(empty($max_age))) {
header('Cache-Control: max-age=' . $max_age);
}
header("Access-Control-Allow-Origin: http://127.0.0.1:8000");
$name = 'abe.png';
$fp = fopen($name, 'rb');
header("Content-Type: image/png");
header("Content-Length: " . filesize($name));
fpassthru($fp);
exit;
?>
third_party/WebKit/LayoutTests/http/tests/security/resources/cors-script.php
View file @ c1b6d6a2
...@@ -8,6 +8,11 @@ if ($cors_arg != 'false') { ...@@ -8,6 +8,11 @@ if ($cors_arg != 'false') {
} }
} }
$max_age = $_GET['max-age'];
if (!(empty($max_age))) {
header('Cache-Control: max-age=' . $max_age);
}
if (strtolower($_GET['credentials']) == 'true') { if (strtolower($_GET['credentials']) == 'true') {
header('Access-Control-Allow-Credentials: true'); header('Access-Control-Allow-Credentials: true');
} }
......
third_party/blink/renderer/core/loader/modulescript/document_module_script_fetcher.cc
View file @ c1b6d6a2
...@@ -44,8 +44,7 @@ void DocumentModuleScriptFetcher::NotifyFinished(Resource* resource) { ...@@ -44,8 +44,7 @@ void DocumentModuleScriptFetcher::NotifyFinished(Resource* resource) {
ModuleScriptCreationParams params( ModuleScriptCreationParams params(
script_resource->GetResponse().Url(), script_resource->SourceText(), script_resource->GetResponse().Url(), script_resource->SourceText(),
script_resource->GetResourceRequest().GetFetchCredentialsMode(), script_resource->GetResourceRequest().GetFetchCredentialsMode(),
script_resource->CalculateAccessControlStatus( script_resource->CalculateAccessControlStatus());
fetcher_->Context().GetSecurityOrigin()));
client_->NotifyFetchFinished(params, error_messages); client_->NotifyFetchFinished(params, error_messages);
} }
......
third_party/blink/renderer/core/loader/modulescript/worker_module_script_fetcher.cc
View file @ c1b6d6a2
...@@ -99,8 +99,7 @@ void WorkerModuleScriptFetcher::NotifyFinished(Resource* resource) { ...@@ -99,8 +99,7 @@ void WorkerModuleScriptFetcher::NotifyFinished(Resource* resource) {
ModuleScriptCreationParams params( ModuleScriptCreationParams params(
script_resource->GetResponse().Url(), script_resource->SourceText(), script_resource->GetResponse().Url(), script_resource->SourceText(),
script_resource->GetResourceRequest().GetFetchCredentialsMode(), script_resource->GetResourceRequest().GetFetchCredentialsMode(),
script_resource->CalculateAccessControlStatus( script_resource->CalculateAccessControlStatus());
global_scope_->EnsureFetcher()->Context().GetSecurityOrigin()));
// Step 13.7. "Asynchronously complete the perform the fetch steps with // Step 13.7. "Asynchronously complete the perform the fetch steps with
// response." [spec text] // response." [spec text]
......
third_party/blink/renderer/core/loader/modulescript/worklet_module_script_fetcher.cc
View file @ c1b6d6a2
...@@ -51,8 +51,7 @@ void WorkletModuleScriptFetcher::NotifyFinished(Resource* resource) { ...@@ -51,8 +51,7 @@ void WorkletModuleScriptFetcher::NotifyFinished(Resource* resource) {
params.emplace( params.emplace(
script_resource->GetResponse().Url(), script_resource->SourceText(), script_resource->GetResponse().Url(), script_resource->SourceText(),
script_resource->GetResourceRequest().GetFetchCredentialsMode(), script_resource->GetResourceRequest().GetFetchCredentialsMode(),
script_resource->CalculateAccessControlStatus( script_resource->CalculateAccessControlStatus());
fetcher_->Context().GetSecurityOrigin()));
} }
// This will eventually notify |client| passed to // This will eventually notify |client| passed to
......
third_party/blink/renderer/core/loader/resource/image_resource.cc
View file @ c1b6d6a2
...@@ -669,8 +669,7 @@ bool ImageResource::IsAccessAllowed( ...@@ -669,8 +669,7 @@ bool ImageResource::IsAccessAllowed(
ImageResourceInfo::kHasSingleSecurityOrigin) ImageResourceInfo::kHasSingleSecurityOrigin)
return false; return false;
DCHECK(security_origin); if (IsSameOriginOrCORSSuccessful())
if (PassesAccessControlCheck(*security_origin))
return true; return true;
return security_origin->CanReadContent(GetResponse().Url()); return security_origin->CanReadContent(GetResponse().Url());
......
third_party/blink/renderer/core/loader/resource/script_resource.cc
View file @ c1b6d6a2
...@@ -237,17 +237,12 @@ void ScriptResource::DestroyDecodedDataForFailedRevalidation() { ...@@ -237,17 +237,12 @@ void ScriptResource::DestroyDecodedDataForFailedRevalidation() {
SetDecodedSize(0); SetDecodedSize(0);
} }
AccessControlStatus ScriptResource::CalculateAccessControlStatus( AccessControlStatus ScriptResource::CalculateAccessControlStatus() const {
const SecurityOrigin* security_origin) const { if (GetCORSStatus() == CORSStatus::kServiceWorkerOpaque)
if (GetResponse().WasFetchedViaServiceWorker()) { return kOpaqueResource;
if (GetCORSStatus() == CORSStatus::kServiceWorkerOpaque)
return kOpaqueResource;
return kSharableCrossOrigin;
}
if (security_origin && PassesAccessControlCheck(*security_origin)) if (IsSameOriginOrCORSSuccessful())
return kSharableCrossOrigin; return kSharableCrossOrigin;
return kNotSharableCrossOrigin; return kNotSharableCrossOrigin;
} }
......
third_party/blink/renderer/core/loader/resource/script_resource.h
View file @ c1b6d6a2
...@@ -71,7 +71,7 @@ class CORE_EXPORT ScriptResource final : public TextResource { ...@@ -71,7 +71,7 @@ class CORE_EXPORT ScriptResource final : public TextResource {
const MovableString& SourceText(); const MovableString& SourceText();
AccessControlStatus CalculateAccessControlStatus(const SecurityOrigin*) const; AccessControlStatus CalculateAccessControlStatus() const;
SingleCachedMetadataHandler* CacheHandler(); SingleCachedMetadataHandler* CacheHandler();
......
third_party/blink/renderer/core/script/classic_pending_script.cc
View file @ c1b6d6a2
...@@ -399,10 +399,8 @@ ClassicScript* ClassicPendingScript::GetSource(const KURL& document_url, ...@@ -399,10 +399,8 @@ ClassicScript* ClassicPendingScript::GetSource(const KURL& document_url,
// "the URL from which the script was obtained" [spec text] // "the URL from which the script was obtained" [spec text]
// https://html.spec.whatwg.org/multipage/webappapis.html#concept-script-base-url // https://html.spec.whatwg.org/multipage/webappapis.html#concept-script-base-url
const KURL& base_url = source_code.Url(); const KURL& base_url = source_code.Url();
return ClassicScript::Create( return ClassicScript::Create(source_code, base_url, options_,
source_code, base_url, options_, resource->CalculateAccessControlStatus());
resource->CalculateAccessControlStatus(
GetElement()->GetDocument().GetSecurityOrigin()));
} }
void ClassicPendingScript::SetStreamer(ScriptStreamer* streamer) { void ClassicPendingScript::SetStreamer(ScriptStreamer* streamer) {
......
third_party/blink/renderer/platform/loader/fetch/resource.cc
View file @ c1b6d6a2
...@@ -403,16 +403,6 @@ AtomicString Resource::HttpContentType() const { ...@@ -403,16 +403,6 @@ AtomicString Resource::HttpContentType() const {
return GetResponse().HttpContentType(); return GetResponse().HttpContentType();
} }
bool Resource::PassesAccessControlCheck(
const SecurityOrigin& security_origin) const {
base::Optional<network::CORSErrorStatus> cors_status = CORS::CheckAccess(
GetResponse().Url(), GetResponse().HttpStatusCode(),
GetResponse().HttpHeaderFields(),
LastResourceRequest().GetFetchCredentialsMode(), security_origin);
return !cors_status;
}
bool Resource::MustRefetchDueToIntegrityMetadata( bool Resource::MustRefetchDueToIntegrityMetadata(
const FetchParameters& params) const { const FetchParameters& params) const {
if (params.IntegrityMetadata().IsEmpty()) if (params.IntegrityMetadata().IsEmpty())
......
third_party/blink/renderer/platform/loader/fetch/resource.h
View file @ c1b6d6a2
...@@ -215,8 +215,6 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>, ...@@ -215,8 +215,6 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
virtual void Finish(TimeTicks finish_time, base::SingleThreadTaskRunner*); virtual void Finish(TimeTicks finish_time, base::SingleThreadTaskRunner*);
void FinishForTest() { Finish(TimeTicks(), nullptr); } void FinishForTest() { Finish(TimeTicks(), nullptr); }
bool PassesAccessControlCheck(const SecurityOrigin&) const;
virtual scoped_refptr<const SharedBuffer> ResourceBuffer() const { virtual scoped_refptr<const SharedBuffer> ResourceBuffer() const {
return data_; return data_;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment