[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





--- Comment #19 from Joe Thomas <joethomas at motorola.com>  2012-01-14 09:35:19 PST ---
(In reply to comment #17)
> (From update of attachment 122473 [details])
> 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.
> 
Done

> > 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
> 
Done.

> 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.
> 
Yes, it strips the trailing and leading whitespace and newline. Also I tested with tabs too.

> Please use HTTPHeaderSet for consistency and performance, like isOnAccessControlResponseHeaderWhitelist does.
> 
Done. Using HashSet<String, CaseFoldingHash>.

> > 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.
>
Tried with make-new-script-test to create new test html. But while running layout tests, it fails with "Reference error: variable not defined" for the functions in js-test-pre.js file. The file-path is mentioned correctly, but not sure why. 

> > 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.

Modified the cases of actual header and field value of 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.