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
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.
<rdar://problem/28575938>
Created attachment 310532 [details] Patch and layout test
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+",");
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
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
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
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
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
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
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
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
(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.
Committed r217069: <http://trac.webkit.org/changeset/217069>