[Webkit-unassigned] [Bug 41210] Cross Origin XMLHttpRequest can not expose headers indicated in Access-Control-Expose-Headers HTTP Response Header

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=41210


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #122473|review?                     |review-
               Flag|                            |




--- Comment #17 from Alexey Proskuryakov <ap at webkit.org>  2012-01-13 11:57:33 PST ---
(From update of attachment 122473)
View in context: https://bugs.webkit.org/attachment.cgi?id=122473&action=review

Thanks, this looks much closer already.

> Source/WebCore/xml/XMLHttpRequest.cpp:155
> +static void parseAccessControlAllowList(const String& headerValue, Vector<String>& headersSet)

It's somewhat weird to see this in XMLHttpRequest.cpp. It might be the only CORS client that exposes headers, but the behavior itself is not tied to XHR. You might find it cleaner to have the checks in CrossOriginAccessControl.cpp - please consider that possibility.

> Source/WebCore/xml/XMLHttpRequest.cpp:159
> +    headerValue.stripWhiteSpace().upper();
> +    headerValue.split(',', false, headersSet);

I suspect that you need to strip whitespace afterwards, not before:

Access-Control-Expose-Headers: X-FOO,    X-BAR

Also, is whitespace definition in this function the same as in RFC 2616? There are many whitespace characters besides plain space, and different specs disagree on which to ignore.

Please use HTTPHeaderSet for consistency and performance, like isOnAccessControlResponseHeaderWhitelist does.

> LayoutTests/http/tests/xmlhttprequest/access-control-response-with-expose-headers-expected.txt:7
> +PASS
> +PASS
> +PASS

It would be very helpful to have explanations of what fails and passes. You can use shouldBe from script test machinery to easily do that, and simplify test code at the same time.

Try our make-new-script-test script, which will prepare boilerplate for a script test.

> LayoutTests/http/tests/xmlhttprequest/resources/access-control-response-with-expose-headers.php:6
> +    header("X-FOO: BAR");
> +    header("X-TEST: TEST");
> +    header("Access-Control-Expose-Headers: X-FOO");

Another case sensitivity test to add is where actual HTTP header has a different case than the value in Access-Control-Expose-Headers.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.