Bug 88461 – Access control allow lists starting with a comma are parsed incorrectly (CORS)

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Pablo Flouret]

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.

[Pablo Flouret]

Created attachment 146126 [details]
Patch

[Alexey Proskuryakov]

Could you please explain why our behavior is incorrect? Are "empty entries" allowed here?

[Pablo Flouret]

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

[Adam Barth]

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.

[Pablo Flouret]

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

[Adam Barth]

Ah, my mistake!  I thought you were referring to Access-Control-Allow-Origin.

[Adam Barth]

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.

[Pablo Flouret]

(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?

[Adam Barth]

If it works in Firefox and Opera, then perhaps we should follow suit.

[Anne van Kesteren]

Per http://tools.ietf.org/html/rfc2616#section-2.1 null elements are allowed (with some constraints).

[Adam Barth]

(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

[Anne van Kesteren]

Where does it say that is not allowed? The prose I pointed to seems to allow that.

[Adam Barth]

(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).  :)

[WebKit Review Bot]

Comment on attachment 146126 [details]
Patch

Clearing flags on attachment: 146126

Committed r119945: <http://trac.webkit.org/changeset/119945>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.