Bug 75089 - Access-Control-Request-Headers values should be lowercase
Summary: Access-Control-Request-Headers values should be lowercase
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-22 07:40 PST by Boris Zbarsky
Modified: 2012-01-18 15:22 PST (History)
8 users (show)

See Also:


Attachments
Patch1 (1.68 KB, patch)
2012-01-16 20:52 PST, Joe Thomas
abarth: review-
Details | Formatted Diff | Diff
Patch2 (5.78 KB, patch)
2012-01-17 18:05 PST, Joe Thomas
ap: review+
Details | Formatted Diff | Diff
patch3 (5.87 KB, patch)
2012-01-17 20:08 PST, Joe Thomas
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Boris Zbarsky 2011-12-22 07:40:35 PST
Spec says:

  If author request headers is not empty include an Access-Control-Request-Headers header
  with as header field value a comma-separated list of the header field names from author
  request headers in lexicographical order, each converted to ASCII lowercase (even when
  one or more are a simple header).

That's not what WebKit does, apparently.  See thread starting http://lists.w3.org/Archives/Public/public-webapps/2011OctDec/1672.html

This is causing a certain amount of author confusion, especially since other UAs get this right.
Comment 1 Alexey Proskuryakov 2011-12-22 10:50:07 PST
Julian disagrees with this spec provision: <https://www.w3.org/Bugs/Public/show_bug.cgi?id=15312>.

Seems OK to change WebKit behavior, although that should really make no difference, as servers must do case insensitive comparison anyway.
Comment 2 Joe Thomas 2012-01-16 20:52:25 PST
Created attachment 122708 [details]
Patch1

+        No new tests, as servers must do case-insensitive comparison.

Just wondering what kind of regression test can be added for this?
Comment 3 Alexey Proskuryakov 2012-01-16 22:20:47 PST
We have our own server for testing, so it should be possible to make it verify this.
Comment 4 Adam Barth 2012-01-17 01:22:25 PST
Comment on attachment 122708 [details]
Patch1

This should be testable.  If you look in the LayoutTests/http/tests directory, you should find some perl or PHP scripts that run on the test server.  They can see the case sensitive version of Access-Control-Request-Headers.
Comment 5 Joe Thomas 2012-01-17 18:05:33 PST
Created attachment 122853 [details]
Patch2

Added test case to verify that server receives the Access-Control-Request-Headers in lowercase.

Also verified it using the IP logs captured with Wireshark.
Comment 6 Alexey Proskuryakov 2012-01-17 18:37:48 PST
Comment on attachment 122853 [details]
Patch2

View in context: https://bugs.webkit.org/attachment.cgi?id=122853&action=review

> LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-lowercase.php:14
> +    if (strcasecmp($_SERVER["HTTP_X_CUSTOM_HEADER"], "fooBAR") == 0)

I'm not sure what the point of this case insensitive comparison is.
Comment 7 Joe Thomas 2012-01-17 20:08:31 PST
Created attachment 122864 [details]
patch3
Comment 8 Joe Thomas 2012-01-17 20:09:40 PST
(In reply to comment #6)
> (From update of attachment 122853 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122853&action=review
> 
> > LayoutTests/http/tests/xmlhttprequest/resources/access-control-preflight-request-header-lowercase.php:14
> > +    if (strcasecmp($_SERVER["HTTP_X_CUSTOM_HEADER"], "fooBAR") == 0)
> 
> I'm not sure what the point of this case insensitive comparison is.

No specific reason. I changed this to if (isset($_SERVER["HTTP_X_CUSTOM_HEADER"]))
Comment 9 Alexey Proskuryakov 2012-01-17 21:00:09 PST
Comment on attachment 122864 [details]
patch3

Thank you.
Comment 10 WebKit Review Bot 2012-01-17 22:06:23 PST
Comment on attachment 122864 [details]
patch3

Clearing flags on attachment: 122864

Committed r105242: <http://trac.webkit.org/changeset/105242>
Comment 11 Joe Thomas 2012-01-18 14:43:17 PST
Comment on attachment 122853 [details]
Patch2

making patch2 obsolete
Comment 12 Alexey Proskuryakov 2012-01-18 15:22:05 PST
Commit queue did not close this bug, but it doesn't appear that there is anything else to do here.