Bug 88461 - Access control allow lists starting with a comma are parsed incorrectly (CORS)
Summary: Access control allow lists starting with a comma are parsed incorrectly (CORS)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-06 15:13 PDT by Pablo Flouret
Modified: 2012-06-10 13:18 PDT (History)
7 users (show)

See Also:


Attachments
Patch (6.26 KB, patch)
2012-06-06 15:31 PDT, Pablo Flouret
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pablo Flouret 2012-06-06 15:13:41 PDT
For example:
Access-Control-Allow-Headers: ,blah,bleh

The parsing algorithm will stop parsing at the first comma, and the whole header will be effectively ignored.
Comment 1 Pablo Flouret 2012-06-06 15:31:01 PDT
Created attachment 146126 [details]
Patch
Comment 2 Alexey Proskuryakov 2012-06-06 15:37:08 PDT
Could you please explain why our behavior is incorrect? Are "empty entries" allowed here?
Comment 3 Pablo Flouret 2012-06-06 15:57:32 PDT
(In reply to comment #2)
> Could you please explain why our behavior is incorrect? Are "empty entries" allowed here?

Yeah, sorry, the http spec says that lists in headers are separated by one or more commas (perhaps empty entries was a misnomer on my part). Firefox and Opera behave like this.
Comment 4 Adam Barth 2012-06-07 15:20:38 PDT
Comment on attachment 146126 [details]
Patch

Which spec says you're allowed to have commas in this header?  CORS refers to http://tools.ietf.org/html/rfc6454#section-7.1 which indicates that the origins are separated by spaces, not by commas.
Comment 5 Pablo Flouret 2012-06-07 15:28:29 PDT
(In reply to comment #4)
> (From update of attachment 146126 [details])
> Which spec says you're allowed to have commas in this header?  CORS refers to http://tools.ietf.org/html/rfc6454#section-7.1 which indicates that the origins are separated by spaces, not by commas.

http://dvcs.w3.org/hg/cors/raw-file/tip/Overview.html#access-control-allow-headers-response-header

Access-Control-Allow-Headers: "Access-Control-Allow-Headers" ":" #field-name

field-name points to rfc-2616.

http://tools.ietf.org/html/rfc2616#page-15

   #rule
      A construct "#" is defined, similar to "*", for defining lists of
      elements. The full form is "<n>#<m>element" indicating at least
      <n> and at most <m> elements, each separated by one or more commas
      (",") and OPTIONAL linear white space (LWS). This makes the usual
      form of lists very easy; a rule such as
         ( *LWS element *( *LWS "," *LWS element ))
      can be shown as
         1#element
      Wherever this construct is used, null elements are allowed, but do
      not contribute to the count of elements present. That is,
      "(element), , (element) " is permitted, but counts as only two
      elements. Therefore, where at least one element is required, at
      least one non-null element MUST be present. Default values are 0
      and infinity so that "#element" allows any number, including zero;
      "1#element" requires at least one; and "1#2element" allows one or
      two.
Comment 6 Adam Barth 2012-06-07 17:06:01 PDT
Ah, my mistake!  I thought you were referring to Access-Control-Allow-Origin.
Comment 7 Adam Barth 2012-06-07 17:11:56 PDT
Doesn't ( *LWS element *( *LWS "," *LWS element )) require at least one element to precede the first comma?

first-header: , foo, bar
second-header: foo,, bar

It sounds like first-header is invalid but second-header is valid.  Maybe I'm still not quite understanding.
Comment 8 Pablo Flouret 2012-06-08 13:50:54 PDT
(In reply to comment #7)
> Doesn't ( *LWS element *( *LWS "," *LWS element )) require at least one element to precede the first comma?
> 
> first-header: , foo, bar
> second-header: foo,, bar
> 
> It sounds like first-header is invalid but second-header is valid.  Maybe I'm still not quite understanding.

Yeah, i'd say the first one is invalid per the definition. It works in Firefox and Opera, though.
The second one doesn't work with the current code either.

Would you prefer the first case to remain invalid?
Comment 9 Adam Barth 2012-06-08 14:04:20 PDT
If it works in Firefox and Opera, then perhaps we should follow suit.
Comment 10 Anne van Kesteren 2012-06-09 01:36:07 PDT
Per http://tools.ietf.org/html/rfc2616#section-2.1 null elements are allowed (with some constraints).
Comment 11 Adam Barth 2012-06-09 11:45:41 PDT
(In reply to comment #10)
> Per http://tools.ietf.org/html/rfc2616#section-2.1 null elements are allowed (with some constraints).

Anne, I think the question we're trying to resolve is whether to allow an empty element in the first position.  RFC 2616 seems to say that's not allowed, but Firefox and Opera seem to allow it.  Specifically, this is the case we're wondering about:

first-header: , foo, bar
Comment 12 Anne van Kesteren 2012-06-09 23:41:43 PDT
Where does it say that is not allowed? The prose I pointed to seems to allow that.
Comment 13 Adam Barth 2012-06-10 12:26:10 PDT
(In reply to comment #12)
> Where does it say that is not allowed? The prose I pointed to seems to allow that.

Yes.  You're right (as usual).  :)
Comment 14 WebKit Review Bot 2012-06-10 13:18:53 PDT
Comment on attachment 146126 [details]
Patch

Clearing flags on attachment: 146126

Committed r119945: <http://trac.webkit.org/changeset/119945>
Comment 15 WebKit Review Bot 2012-06-10 13:18:58 PDT
All reviewed patches have been landed.  Closing bug.