Skip to content

Commit 8b1be3f

Browse files
magomezmagomez
authored
Merge pull request #1391 from WebPlatformForEmbedded/pgorszkowski/2.38/XMLHttpRequest-without-credentials-fails-with-CORS-error-on-redirections
XMLHttpRequest without credentials fails with CORS error on redirections
2 parents 3b3a227 + 678b835 commit 8b1be3f

12 files changed

Lines changed: 46 additions & 25 deletions

‎LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects-expected.txt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,12 @@ PASS: NetworkError: A network error occurred.
3030
Testing http://localhost:8000/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi(async)
3131
Expecting success: false
3232
PASS: 0
33+
Testing resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi (sync)
34+
Expecting success: true
35+
PASS: PASS: Cross-domain access allowed.
36+
37+
Testing resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi(async)
38+
Expecting success: true
39+
PASS: PASS: Cross-domain access allowed.
40+
3341

‎LayoutTests/http/tests/xmlhttprequest/access-control-and-redirects.html‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@
4747
var tests = [
4848
["/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi", true, true],
4949
["http://localhost:8000/resources/redirect.py?url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow.cgi", false, false],
50-
["http://localhost:8000/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi", false, false]
50+
["http://localhost:8000/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi", false, false],
51+
["resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi", true, true]
5152
]
5253

5354
var currentTest = 0;

‎LayoutTests/platform/mac-wk1/http/tests/xmlhttprequest/access-control-and-redirects-expected.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,11 @@ PASS: NetworkError: A network error occurred.
2727
Testing http://localhost:8000/resources/redirect.py?url=http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow.cgi(async)
2828
Expecting success: false
2929
PASS: 0
30+
Testing resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi (sync)
31+
Expecting success: true
32+
FAIL: NetworkError: A network error occurred.
33+
Testing resources/redirect-cors.py?access-control-allow-origin=*&url=http://localhost:8000/xmlhttprequest/resources/redirect-cors.py?access-control-allow-origin=*%26url=http://127.0.0.1:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi(async)
34+
Expecting success: true
35+
PASS: PASS: Cross-domain access allowed.
36+
3037

‎Source/WebCore/loader/CrossOriginAccessControl.cpp‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -240,41 +240,43 @@ bool CrossOriginAccessControlCheckDisabler::crossOriginAccessControlCheckEnabled
240240
return m_accessControlCheckEnabled;
241241
}
242242

243-
Expected<void, String> passesAccessControlCheck(const ResourceResponse& response, StoredCredentialsPolicy storedCredentialsPolicy, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
243+
Expected<void, String> passesAccessControlCheck(const ResourceResponse& response, FetchOptions::Credentials fetchOptionsCredentials, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
244244
{
245-
// A wildcard Access-Control-Allow-Origin can not be used if credentials are to be sent,
246-
// even with Access-Control-Allow-Credentials set to true.
247245
const String& accessControlOriginString = response.httpHeaderField(HTTPHeaderName::AccessControlAllowOrigin);
248-
bool starAllowed = storedCredentialsPolicy == StoredCredentialsPolicy::DoNotUse;
249-
if (!starAllowed)
250-
starAllowed = checkDisabler && !checkDisabler->crossOriginAccessControlCheckEnabled();
251-
if (accessControlOriginString == "*"_s && starAllowed)
252-
return { };
253-
254-
String securityOriginString = securityOrigin.toString();
255-
if (accessControlOriginString != securityOriginString) {
256-
if (accessControlOriginString == "*"_s)
246+
const String securityOriginString = securityOrigin.toString();
247+
if (accessControlOriginString == "*"_s) {
248+
if (checkDisabler && !checkDisabler->crossOriginAccessControlCheckEnabled())
249+
return { };
250+
// A wildcard Access-Control-Allow-Origin can not be used if credentials are to be sent,
251+
// even with Access-Control-Allow-Credentials set to true.
252+
// https://fetch.spec.whatwg.org/#cors-protocol-and-credentials.
253+
if (fetchOptionsCredentials == FetchOptions::Credentials::Include)
257254
return makeUnexpected("Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true."_s);
255+
} else if (accessControlOriginString != securityOriginString) {
256+
// Multiple origins are not allowed in the allow origin header.
257+
// https://fetch.spec.whatwg.org/#http-access-control-allow-origin.
258258
if (accessControlOriginString.find(',') != notFound)
259259
return makeUnexpected("Access-Control-Allow-Origin cannot contain more than one origin."_s);
260+
260261
return makeUnexpected(makeString("Origin ", securityOriginString, " is not allowed by Access-Control-Allow-Origin.", " Status code: ", response.httpStatusCode()));
261262
}
262263

263-
if (storedCredentialsPolicy == StoredCredentialsPolicy::Use) {
264+
if (fetchOptionsCredentials == FetchOptions::Credentials::Include) {
264265
const String& accessControlCredentialsString = response.httpHeaderField(HTTPHeaderName::AccessControlAllowCredentials);
266+
// https://fetch.spec.whatwg.org/#http-access-control-allow-credentials.
265267
if (accessControlCredentialsString != "true"_s)
266268
return makeUnexpected("Credentials flag is true, but Access-Control-Allow-Credentials is not \"true\"."_s);
267269
}
268270

269271
return { };
270272
}
271273

272-
Expected<void, String> validatePreflightResponse(PAL::SessionID sessionID, const ResourceRequest& request, const ResourceResponse& response, StoredCredentialsPolicy storedCredentialsPolicy, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
274+
Expected<void, String> validatePreflightResponse(PAL::SessionID sessionID, const ResourceRequest& request, const ResourceResponse& response, FetchOptions::Credentials fetchOptionsCredentials, StoredCredentialsPolicy storedCredentialsPolicy, const SecurityOrigin& securityOrigin, const CrossOriginAccessControlCheckDisabler* checkDisabler)
273275
{
274276
if (!response.isSuccessful())
275277
return makeUnexpected(makeString("Preflight response is not successful. Status code: ", response.httpStatusCode()));
276278

277-
auto accessControlCheckResult = passesAccessControlCheck(response, storedCredentialsPolicy, securityOrigin, checkDisabler);
279+
auto accessControlCheckResult = passesAccessControlCheck(response, fetchOptionsCredentials, securityOrigin, checkDisabler);
278280
if (!accessControlCheckResult)
279281
return accessControlCheckResult;
280282

‎Source/WebCore/loader/CrossOriginAccessControl.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
#pragma once
2828

29+
#include "FetchOptions.h"
2930
#include "HTTPHeaderNames.h"
3031
#include "ReferrerPolicy.h"
3132
#include "StoredCredentialsPolicy.h"
@@ -84,8 +85,8 @@ class WEBCORE_EXPORT CrossOriginAccessControlCheckDisabler {
8485
bool m_accessControlCheckEnabled { true };
8586
};
8687

87-
WEBCORE_EXPORT Expected<void, String> passesAccessControlCheck(const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
88-
WEBCORE_EXPORT Expected<void, String> validatePreflightResponse(PAL::SessionID, const ResourceRequest&, const ResourceResponse&, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
88+
WEBCORE_EXPORT Expected<void, String> passesAccessControlCheck(const ResourceResponse&, FetchOptions::Credentials, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
89+
WEBCORE_EXPORT Expected<void, String> validatePreflightResponse(PAL::SessionID, const ResourceRequest&, const ResourceResponse&, FetchOptions::Credentials, StoredCredentialsPolicy, const SecurityOrigin&, const CrossOriginAccessControlCheckDisabler*);
8990

9091
enum class ForNavigation : bool { No, Yes };
9192
WEBCORE_EXPORT std::optional<ResourceError> validateCrossOriginResourcePolicy(CrossOriginEmbedderPolicyValue, const SecurityOrigin&, const URL&, const ResourceResponse&, ForNavigation);

‎Source/WebCore/loader/CrossOriginPreflightChecker.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ void CrossOriginPreflightChecker::validatePreflightResponse(DocumentThreadableLo
7272
return;
7373
}
7474

75-
auto result = WebCore::validatePreflightResponse(page->sessionID(), request, response, loader.options().storedCredentialsPolicy, loader.securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
75+
auto result = WebCore::validatePreflightResponse(page->sessionID(), request, response, loader.options().credentials, loader.options().storedCredentialsPolicy, loader.securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
7676
if (!result) {
7777
loader.document().addConsoleMessage(MessageSource::Security, MessageLevel::Error, result.error());
7878
loader.preflightFailure(identifier, ResourceError(errorDomainWebKitInternal, 0, request.url(), result.error(), ResourceError::Type::AccessControl));

‎Source/WebCore/loader/DocumentThreadableLoader.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,7 @@ void DocumentThreadableLoader::loadRequest(ResourceRequest&& request, SecurityCh
658658
else {
659659
ASSERT(m_options.mode == FetchOptions::Mode::Cors);
660660
response.setTainting(ResourceResponse::Tainting::Cors);
661-
auto accessControlCheckResult = passesAccessControlCheck(response, m_options.storedCredentialsPolicy, securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
661+
auto accessControlCheckResult = passesAccessControlCheck(response, m_options.credentials, securityOrigin(), &CrossOriginAccessControlCheckDisabler::singleton());
662662
if (!accessControlCheckResult) {
663663
logErrorAndFail(ResourceError(errorDomainWebKitInternal, 0, response.url(), accessControlCheckResult.error(), ResourceError::Type::AccessControl));
664664
return;

‎Source/WebCore/loader/SubresourceLoader.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,7 @@ Expected<void, String> SubresourceLoader::checkResponseCrossOriginAccessControl(
639639

640640
ASSERT(m_origin);
641641

642-
return passesAccessControlCheck(response, options().credentials == FetchOptions::Credentials::Include ? StoredCredentialsPolicy::Use : StoredCredentialsPolicy::DoNotUse, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
642+
return passesAccessControlCheck(response, options().credentials, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
643643
}
644644

645645
Expected<void, String> SubresourceLoader::checkRedirectionCrossOriginAccessControl(const ResourceRequest& previousRequest, const ResourceResponse& redirectResponse, ResourceRequest& newRequest)
@@ -663,7 +663,7 @@ Expected<void, String> SubresourceLoader::checkRedirectionCrossOriginAccessContr
663663

664664
ASSERT(m_origin);
665665
if (crossOriginFlag) {
666-
auto accessControlCheckResult = passesAccessControlCheck(redirectResponse, options().storedCredentialsPolicy, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
666+
auto accessControlCheckResult = passesAccessControlCheck(redirectResponse, options().credentials, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
667667
if (!accessControlCheckResult)
668668
return accessControlCheckResult;
669669
}

‎Source/WebCore/loader/cache/CachedResource.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ void CachedResource::loadFrom(const CachedResource& resource)
305305

306306
if (isCrossOrigin() && m_options.mode == FetchOptions::Mode::Cors) {
307307
ASSERT(m_origin);
308-
auto accessControlCheckResult = WebCore::passesAccessControlCheck(resource.response(), m_options.storedCredentialsPolicy, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
308+
auto accessControlCheckResult = WebCore::passesAccessControlCheck(resource.response(), m_options.credentials, *m_origin, &CrossOriginAccessControlCheckDisabler::singleton());
309309
if (!accessControlCheckResult) {
310310
setResourceError(ResourceError(String(), 0, url(), accessControlCheckResult.error(), ResourceError::Type::AccessControl));
311311
return;

‎Source/WebKit/NetworkProcess/NetworkCORSPreflightChecker.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ void NetworkCORSPreflightChecker::didCompleteWithError(const WebCore::ResourceEr
142142

143143
CORS_CHECKER_RELEASE_LOG("didComplete http_status_code=%d", m_response.httpStatusCode());
144144

145-
auto result = validatePreflightResponse(m_parameters.sessionID, m_parameters.originalRequest, m_response, m_parameters.storedCredentialsPolicy, m_parameters.sourceOrigin, m_networkResourceLoader.get());
145+
auto result = validatePreflightResponse(m_parameters.sessionID, m_parameters.originalRequest, m_response, m_parameters.fetchOptionsCredentials, m_parameters.storedCredentialsPolicy, m_parameters.sourceOrigin, m_networkResourceLoader.get());
146146
if (!result) {
147147
CORS_CHECKER_RELEASE_LOG("didComplete, AccessControl error: %s", result.error().utf8().data());
148148
m_completionCallback(ResourceError { errorDomainWebKitInternal, 0, m_parameters.originalRequest.url(), result.error(), ResourceError::Type::AccessControl });

0 commit comments

Comments
 (0)