[webkit-reviews] review denied: [Bug 41210] Cross Origin XMLHttpRequest can not expose headers indicated in Access-Control-Expose-Headers HTTP Response Header : [Attachment 122549] Patch3
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Jan 15 19:48:36 PST 2012
Alexey Proskuryakov <ap at webkit.org> has denied Joe Thomas
<joethomas at motorola.com>'s request for review:
Bug 41210: Cross Origin XMLHttpRequest can not expose headers indicated in
Access-Control-Expose-Headers HTTP Response Header
https://bugs.webkit.org/show_bug.cgi?id=41210
Attachment 122549: Patch3
https://bugs.webkit.org/attachment.cgi?id=122549&action=review
------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=122549&action=review
THis is pretty close. I have a few minor nitpicks, and I think that you should
make another try converting this to a "shouldBe" test.
> Source/WebCore/loader/CrossOriginAccessControl.cpp:173
> +void parseAccessControlExposeHeadersAllowList(const String& headerValue,
HashSet<String, CaseFoldingHash>& headersSet)
Is "headers set" good English grammar? FWIW, existing code says "header set".
> Source/WebCore/loader/CrossOriginAccessControl.cpp:177
> + for (unsigned int headerCount = 0; headerCount < headers.size();
headerCount++) {
WebKit coding style is to use "unsigned", not "unsigned int".
> Source/WebCore/loader/CrossOriginAccessControl.h:50
> +void parseAccessControlExposeHeadersAllowList(const String& headerValue,
HashSet<String, CaseFoldingHash>& headersSet);
I'd suggest moving HTTPHeaderSet definition to the header, and using it for
consistency.
More information about the webkit-reviews
mailing list