Bug 162819 – Improve error message for Access-Control-Allow-Origin violation due to misconfigured server

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chasen Le Hara]

While writing a script that makes an XHR across domains, I ran into an issue where the server was responding with multiple Access-Control-Allow-Origin headers.

Safari unhelpfully reports this error: Origin http://example.com is not allowed by Access-Control-Allow-Origin.

This doesn’t provide a lot of information about what’s going on or how to fix it.

This was fixed in Chromium a while back: https://bugs.chromium.org/p/chromium/issues/detail?id=321517

[Daniel Bates]

If a server is sending more than one Access-Control-Allow-Origin header then it is misconfigured. I can see the value in helping developers identify such a misconfiguration by pointing it out in the CORS failure error message we emit.

[Daniel Bates]

<rdar://problem/28575938>

[Daniel Bates]

Created attachment 310532 [details]
Patch and layout test

[Joseph Pecoraro]

Comment on attachment 310532 [details]
Patch and layout test

View in context: https://bugs.webkit.org/attachment.cgi?id=310532&action=review

r=me

Looks like we will have to mark the WPT tests as failures?

    imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins.html
    imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins-worker.html

> Source/WebCore/loader/CrossOriginAccessControl.cpp:161
>              errorDescription = "Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true.";
> +        else if (accessControlOriginString.find(',') != notFound)
> +            errorDescription = "Access-Control-Allow-Origin cannot contain more than one origin.";

You can ASCIILiteral(...) these strings.

> LayoutTests/http/tests/xmlhttprequest/resources/origin-exact-matching-iframe.html:48
> +//shouldFail(pageOrigin + "\0"); // Doesn't fail in chromium-linux. See http://wkbug.com/88688 and http://wkbug.com/88139

Do we pass this? Seems like this was only an issue in chromium-linux, we should be able to enable this test.

> LayoutTests/http/tests/xmlhttprequest/resources/origin-exact-matching-iframe.html:49
> +shouldFail((pageOrigin).toUpperCase());

Nit: You can drop the parenthesis around pageOrigin now that it is not an expression.

> LayoutTests/http/tests/xmlhttprequest/resources/origin-exact-matching-iframe.html:71
> +shouldFail(pageOrigin + ", *");

How about adding cases starting with or ending with a comma:

    shouldFail(",");
    shouldFail(","+pageOrigin);
    shouldFail(pageOrigin+",");

[Build Bot]

Comment on attachment 310532 [details]
Patch and layout test

Attachment 310532 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3771290

New failing tests:
imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins.html
imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins-worker.html

[Build Bot]

Created attachment 310544 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 310532 [details]
Patch and layout test

Attachment 310532 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3771293

New failing tests:
imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins.html
imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins-worker.html

[Build Bot]

Created attachment 310545 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 310532 [details]
Patch and layout test

Attachment 310532 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3771262

New failing tests:
imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins.html
imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins-worker.html

[Build Bot]

Created attachment 310546 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 310532 [details]
Patch and layout test

Attachment 310532 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3771388

New failing tests:
imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins.html
imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins-worker.html

[Build Bot]

Created attachment 310549 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Daniel Bates]

(In reply to Joseph Pecoraro from comment #4)
> 
> Looks like we will have to mark the WPT tests as failures?
> 
>     imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins.html
>    
> imported/w3c/web-platform-tests/fetch/api/cors/cors-multiple-origins-worker.
> html
> 

Will update results to reflect the new error message.

> > Source/WebCore/loader/CrossOriginAccessControl.cpp:161
> >              errorDescription = "Cannot use wildcard in Access-Control-Allow-Origin when credentials flag is true.";
> > +        else if (accessControlOriginString.find(',') != notFound)
> > +            errorDescription = "Access-Control-Allow-Origin cannot contain more than one origin.";
> 
> You can ASCIILiteral(...) these strings.
> 

Will fix.

> > LayoutTests/http/tests/xmlhttprequest/resources/origin-exact-matching-iframe.html:48
> > +//shouldFail(pageOrigin + "\0"); // Doesn't fail in chromium-linux. See http://wkbug.com/88688 and http://wkbug.com/88139
> 
> Do we pass this? Seems like this was only an issue in chromium-linux, we
> should be able to enable this test.
> 

Will enable these tests as they pass as of the time of writing.

> > LayoutTests/http/tests/xmlhttprequest/resources/origin-exact-matching-iframe.html:49
> > +shouldFail((pageOrigin).toUpperCase());
> 
> Nit: You can drop the parenthesis around pageOrigin now that it is not an
> expression.
> 

Will remove parentheses.

> > LayoutTests/http/tests/xmlhttprequest/resources/origin-exact-matching-iframe.html:71
> > +shouldFail(pageOrigin + ", *");
> 
> How about adding cases starting with or ending with a comma:
> 
>     shouldFail(",");
>     shouldFail(","+pageOrigin);
>     shouldFail(pageOrigin+",");

Will add these tests.

[Daniel Bates]

Committed r217069: <http://trac.webkit.org/changeset/217069>