XMLHttpRequest fails with CORS error on redirections

[filipe-norte-red]

When performing a XMLHttpRequest without credentials and using a Access-Control-Allow-Origin: * header on a site that performs redirections, the request fails with CORS errors.

[Error] Cross-origin redirection to http://192.168.1.75:8000/origin1_target.php denied by Cross-Origin Resource Sharing policy: Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true.
[Error] Failed to load resource: Cross-origin redirection to http://192.168.1.75:8000/origin1_target.php denied by Cross-Origin Resource Sharing policy: Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true. (origin2_redirect.php, line 0)
[Error] XMLHttpRequest cannot load http://192.168.1.75:8001/origin2_redirect.php due to access control checks.

The attached wildcard-cors.zip package contains test files that reproduce this issue. Please see the included readme.txt file for reproduction steps.

This was tested only on wpe-2.38. Same test passes on Chrome and Firefox.

Potential fix:
The patch below is a fix candidate (Thanks to Alkis Gkouzias for the investigation and proposal):

diff --git a/Source/WebCore/xml/XMLHttpRequest.cpp b/Source/WebCore/xml/XMLHttpRequest.cpp
index 283754477322..035239e8a879 100644
--- a/Source/WebCore/xml/XMLHttpRequest.cpp
+++ b/Source/WebCore/xml/XMLHttpRequest.cpp
@@ -642,7 +642,7 @@ ExceptionOr<void> XMLHttpRequest::createRequest()
     // The presence of upload event listeners forces us to use preflighting because POSTing to an URL that does not
     // permit cross origin requests should look exactly like POSTing to an URL that does not respond at all.
     options.preflightPolicy = m_uploadListenerFlag ? PreflightPolicy::Force : PreflightPolicy::Consider;
-    options.credentials = m_includeCredentials ? FetchOptions::Credentials::Include : FetchOptions::Credentials::SameOrigin;
+    options.credentials = m_includeCredentials ? FetchOptions::Credentials::Include : FetchOptions::Credentials::Omit;
     options.mode = FetchOptions::Mode::Cors;
     options.contentSecurityPolicyEnforcement = scriptExecutionContext()->shouldBypassMainWorldContentSecurityPolicy() ? ContentSecurityPolicyEnforcement::DoNotEnforce : ContentSecurityPolicyEnforcement::EnforceConnectSrcDirective;
     options.initiator = cachedResourceRequestInitiators().xmlhttprequest;

Rational
In Source/WebCore/loader/CrossOriginAccessControl.cpp it can be seen that the reason that the "Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true" error happens is that

bool starAllowed = storedCredentialsPolicy == StoredCredentialsPolicy::DoNotUse; 

is false. This means that while access control allow origin is set to *, storedCredentialsPolicy is not set to DoNotUse. This results from Source/WebCore/xml/XMLHttpRequest.cpp in method ExceptionOr XMLHttpRequest::createRequest()

options.credentials = m_includeCredentials ? FetchOptions::Credentials::Include : FetchOptions::Credentials::SameOrigin; 

This line will essentially set options.credentials to SameOrigin in case that they are not included. However such option will result into StoredCredentialsPolicy::Use in Source/WebCore/loader/DocumentThreadableLoader.cpp

m_options.storedCredentialsPolicy = (m_options.credentials == FetchOptions::Credentials::Include || (m_options.credentials == FetchOptions::Credentials::SameOrigin && m_sameOriginRequest)) ? StoredCredentialsPolicy::Use : StoredCredentialsPolicy::DoNotUse; 

[pgorszkowski-igalia]

It also fails with upstream GTK.

[pgorszkowski-igalia]

I also raised a ticket in upstream about this issue: https://bugs.webkit.org/show_bug.cgi?id=276364

[pgorszkowski-igalia]

@filipe-norte-red : can you verify the fix from WebKit/WebKit#30638? I think we can use it in downstream before it will be merged in upstream.

[filipe-norte-red]

@pgorszkowski-igalia , I tried it and it works. I haven't done further smoke testing yet though to see if any regression was introduced

[pgorszkowski-igalia]

@filipe-norte-red : can you do more smoke testing before we merge it?

[filipe-norte-red]

@pgorszkowski-igalia , I did some initial smoke testing with success, but we'll be running additional tests with additional devices to try to flush out any potential regressions. I'll keep you updated.

[filipe-norte-red]

@pgorszkowski-igalia , we tested multiple applications across a few devices to try to flush out any issues. No issues were observed with this fix.
Note: The tests were not focused on CORS specific functionality per se. However, app behavior is dependent on correct functioning of CORS implementation, so these results should give more confidence that there isn't any obvious regression when using the fix.

[pgorszkowski-igalia]

@filipe-norte-red : thanks for your feedback and tests. I will prepare a PR with this fix for wpe-2.38 for merge.