Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(3)

Issue 20735002: CORS: Fix the handling of redirected request containing Origin null. (Closed)

Created:
7 years, 3 months ago by ancilgeorge
Modified:
7 years, 3 months ago
CC:
blink-reviews, dglazkov+blink, Nate Chapin, eae+blinkwatch, gavinp+loader_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

CORS: Fix the handling of redirected request containing Origin null. Removed the check for securityOrigin->isUnique() in passesAccessControlCheck(). This check was preventing redirected request with "Origin: null" from being successful even when the response contained "Access-Control-Allow-Origin: null" The case where the server responds with "Access-Control-Allow-Origin: null" for a request with "Origin: null" is not a failure case as per the W3C CORS Resource Sharing alogrithm (http://www.w3.org/TR/cors/#resource-sharing-check-0). The specification also mentions the following line below Resource Sharing alogrithm: "The above algorithm also functions when the ASCII serialization of an origin is the string 'null'." This additional check was added in passesAccessControlCheck() during the implementation of HTML5 sandbox attribute for iframes. As per the WHATWG (http://www.whatwg.org/specs/web-apps/current-work/multipage/origin-0.html#sandboxed-origin-browsing-context-flag) specification when "sandboxed origin browsing context flag" is set it forces content into a unique origin. For XHR reqests made from the sandboxed iframe CORS specfication is applicable. Updated the error description in expected.txt of these related layout test to match the change. Added another allow test for "Access-Control-Allow-Origin: null" in addition to the wildcard test ("Access-Control-Allow-Origin: *"). This makes the behavior same as Mozilla Firefox Browser. Also clears the failed tests in http://w3c-test.org/webappsec/tests/cors/submitted/opera/staging/redirect-origin.htm R=abarth@chromium.org, mkwst@chromium.org BUG=263835 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=155002

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -7 lines) Patch
A LayoutTests/http/tests/xmlhttprequest/access-control-sandboxed-iframe-allow-origin-null.html View 1 chunk +18 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/xmlhttprequest/access-control-sandboxed-iframe-allow-origin-null-expected.txt View 1 chunk +9 lines, -0 lines 0 comments Download
M LayoutTests/http/tests/xmlhttprequest/access-control-sandboxed-iframe-denied-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/xmlhttprequest/access-control-sandboxed-iframe-denied-without-wildcard-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
A LayoutTests/http/tests/xmlhttprequest/redirect-cors-origin-null.html View 1 chunk +33 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/xmlhttprequest/redirect-cors-origin-null-expected.txt View 1 chunk +5 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/xmlhttprequest/resources/access-control-sandboxed-iframe-allow-origin-null.cgi View 1 chunk +8 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/xmlhttprequest/resources/access-control-sandboxed-iframe-allow-origin-null-iframe.html View 1 chunk +25 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/xmlhttprequest/resources/redirect-cors-origin-null.php View 1 chunk +11 lines, -0 lines 0 comments Download
A LayoutTests/http/tests/xmlhttprequest/resources/redirect-cors-origin-null-pass.php View 1 chunk +7 lines, -0 lines 0 comments Download
M Source/core/loader/CrossOriginAccessControl.cpp View 1 chunk +0 lines, -5 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
ancilgeorge
Requesting review.
7 years, 3 months ago (2013-07-26 14:49:30 UTC) #1
abarth-chromium
Looks great. Thanks for the detailed description and spec links. LGTM
7 years, 3 months ago (2013-07-26 16:39:15 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ancilgeorge@samsung.com/20735002/1
7 years, 3 months ago (2013-07-26 16:43:10 UTC) #3
commit-bot: I haz the power
7 years, 3 months ago (2013-07-26 19:02:38 UTC) #4
Message was sent while issue was closed.
Change committed as 155002

Powered by Google App Engine
This is Rietveld 408576698