Commit 08cc019f authored by Yutaka Hirano's avatar Yutaka Hirano Committed by Commit Bot

[XHR] Use response tainting to calculate CORS-exposed header-name list

XHR uses the same-originness of the request origin and the destination
URL to calculate the CORS-exposed header-name list, which leads to
wrong results with redirects. Use response tainting as specced.

Bug: 959390
Change-Id: Iec448dfe7d2b47d00f0f471391eb7918a1fe7bc4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1598949Reviewed-by: default avatarTakashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657626}
parent 09b2b752
...@@ -1043,9 +1043,9 @@ void XMLHttpRequest::CreateRequest(scoped_refptr<EncodedFormData> http_body, ...@@ -1043,9 +1043,9 @@ void XMLHttpRequest::CreateRequest(scoped_refptr<EncodedFormData> http_body,
} }
} }
same_origin_request_ = GetSecurityOrigin()->CanRequest(url_); const bool same_origin_request = GetSecurityOrigin()->CanRequest(url_);
if (!same_origin_request_ && with_credentials_) { if (!same_origin_request && with_credentials_) {
UseCounter::Count(&execution_context, UseCounter::Count(&execution_context,
WebFeature::kXMLHttpRequestCrossOriginWithCredentials); WebFeature::kXMLHttpRequestCrossOriginWithCredentials);
} }
...@@ -1053,7 +1053,7 @@ void XMLHttpRequest::CreateRequest(scoped_refptr<EncodedFormData> http_body, ...@@ -1053,7 +1053,7 @@ void XMLHttpRequest::CreateRequest(scoped_refptr<EncodedFormData> http_body,
// We also remember whether upload events should be allowed for this request // We also remember whether upload events should be allowed for this request
// in case the upload listeners are added after the request is started. // in case the upload listeners are added after the request is started.
upload_events_allowed_ = upload_events_allowed_ =
same_origin_request_ || upload_events || same_origin_request || upload_events ||
!cors::IsCorsSafelistedMethod(method_) || !cors::IsCorsSafelistedMethod(method_) ||
!cors::ContainsOnlyCorsSafelistedHeaders(request_headers_); !cors::ContainsOnlyCorsSafelistedHeaders(request_headers_);
...@@ -1489,11 +1489,12 @@ String XMLHttpRequest::getAllResponseHeaders() const { ...@@ -1489,11 +1489,12 @@ String XMLHttpRequest::getAllResponseHeaders() const {
!GetSecurityOrigin()->CanLoadLocalResources()) !GetSecurityOrigin()->CanLoadLocalResources())
continue; continue;
if (!same_origin_request_ && if (response_.GetType() == network::mojom::FetchResponseType::kCors &&
!cors::IsCorsSafelistedResponseHeader(it->key) && !cors::IsCorsSafelistedResponseHeader(it->key) &&
access_control_expose_header_set.find(it->key.Ascii().data()) == access_control_expose_header_set.find(it->key.Ascii().data()) ==
access_control_expose_header_set.end()) access_control_expose_header_set.end()) {
continue; continue;
}
string_builder.Append(it->key.LowerASCII()); string_builder.Append(it->key.LowerASCII());
string_builder.Append(':'); string_builder.Append(':');
...@@ -1525,7 +1526,8 @@ const AtomicString& XMLHttpRequest::getResponseHeader( ...@@ -1525,7 +1526,8 @@ const AtomicString& XMLHttpRequest::getResponseHeader(
: network::mojom::FetchCredentialsMode::kSameOrigin, : network::mojom::FetchCredentialsMode::kSameOrigin,
response_); response_);
if (!same_origin_request_ && !cors::IsCorsSafelistedResponseHeader(name) && if (response_.GetType() == network::mojom::FetchResponseType::kCors &&
!cors::IsCorsSafelistedResponseHeader(name) &&
access_control_expose_header_set.find(name.Ascii().data()) == access_control_expose_header_set.find(name.Ascii().data()) ==
access_control_expose_header_set.end()) { access_control_expose_header_set.end()) {
LogConsoleError(GetExecutionContext(), LogConsoleError(GetExecutionContext(),
......
...@@ -368,7 +368,6 @@ class XMLHttpRequest final : public XMLHttpRequestEventTarget, ...@@ -368,7 +368,6 @@ class XMLHttpRequest final : public XMLHttpRequestEventTarget,
bool error_ = false; bool error_ = false;
bool upload_events_allowed_ = true; bool upload_events_allowed_ = true;
bool upload_complete_ = false; bool upload_complete_ = false;
bool same_origin_request_ = true;
// True iff the ongoing resource loading is using the downloadToBlob // True iff the ongoing resource loading is using the downloadToBlob
// option. // option.
bool downloading_to_blob_ = false; bool downloading_to_blob_ = false;
......
<!DOCTYPE html>
<html>
<head>
<title>XHR should respect access-control-expose-headers header on redirect</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/common/get-host-info.sub.js"></script>
</head>
<body>
<script type="text/javascript">
async_test((test) => {
const xhr = new XMLHttpRequest;
xhr.onerror = test.unreached_func("Unexpected error.");
xhr.onload = test.step_func_done(() => {
assert_equals(xhr.status, 200);
assert_equals(xhr.getResponseHeader('foo'), 'bar');
assert_equals(xhr.getResponseHeader('hoge'), null);
});
const destination = get_host_info().HTTP_REMOTE_ORIGIN +
'/common/blank.html?pipe=header(access-control-allow-origin,*)|' +
'header(access-control-expose-headers,foo)|' +
'header(foo,bar)|header(hoge,fuga)';
const url =
'resources/redirect.py?location=' + encodeURIComponent(destination);
xhr.open('GET', url);
xhr.send();
});
</script>
</body>
</html>
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