code review 148890043: net/http: allow double-quotes only on cookie values, no...

net/http: allow double-quotes only on cookie values, not cookie
attribute values, a la RFC 6265 section 4.1.1 "Syntax".

Fixes issue 7751.

[nigeltao, 2014-09-23 07:58:42]

Hello dr.volker.dobler@gmail.com (cc: bradfitz@golang.org,
golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[volker.dobler, 2014-09-23 12:11:29]

https://codereview.appspot.com/148890043/diff/40001/src/net/http/cookie.go
File src/net/http/cookie.go (right):

https://codereview.appspot.com/148890043/diff/40001/src/net/http/cookie.go#ne...
src/net/http/cookie.go:208: val, success := parseCookieValue(val, i == 0)
Why the i==0 here?  Shouldn't it be just true?
According to RFC 6265 section 4.2.1 a Cookie header is a ;-delimited list
of cookie-pairs and a cookie-pair may have a quoted value (section 4.1.1).

With i==0 reading a Cookie header like
  Cookie: foo="fooVal"; bar="barVal"
will drop the bar cookie which seems wrong.

https://codereview.appspot.com/148890043/diff/40001/src/net/http/cookie_test.go
File src/net/http/cookie_test.go (right):

https://codereview.appspot.com/148890043/diff/40001/src/net/http/cookie_test....
src/net/http/cookie_test.go:316: }
Maybe a test for quoted cookie values should be added here 
while you're at it. Something like:

{
    Header{"Cookie": {`Cookie-1="v$1"; c2="v2"`}},
    "",
    []*Cookie{
        {Name: "Cookie-1", Value: "v$1"},
        {Name: "c2", Value: "v2"},
    },
}

With i==0 (see above) in readCookies the c2 cookie is lost.

[nigeltao, 2014-09-24 08:32:48]

https://codereview.appspot.com/148890043/diff/40001/src/net/http/cookie.go
File src/net/http/cookie.go (right):

https://codereview.appspot.com/148890043/diff/40001/src/net/http/cookie.go#ne...
src/net/http/cookie.go:208: val, success := parseCookieValue(val, i == 0)
On 2014/09/23 12:11:29, volker.dobler wrote:
> Why the i==0 here?  Shouldn't it be just true?
> According to RFC 6265 section 4.2.1 a Cookie header is a ;-delimited list
> of cookie-pairs and a cookie-pair may have a quoted value (section 4.1.1).
> 
> With i==0 reading a Cookie header like
>   Cookie: foo="fooVal"; bar="barVal"
> will drop the bar cookie which seems wrong.

Done.

https://codereview.appspot.com/148890043/diff/40001/src/net/http/cookie_test.go
File src/net/http/cookie_test.go (right):

https://codereview.appspot.com/148890043/diff/40001/src/net/http/cookie_test....
src/net/http/cookie_test.go:316: }
On 2014/09/23 12:11:29, volker.dobler wrote:
> Maybe a test for quoted cookie values should be added here 
> while you're at it. Something like:
> 
> {
>     Header{"Cookie": {`Cookie-1="v$1"; c2="v2"`}},
>     "",
>     []*Cookie{
>         {Name: "Cookie-1", Value: "v$1"},
>         {Name: "c2", Value: "v2"},
>     },
> }
> 
> With i==0 (see above) in readCookies the c2 cookie is lost.

Done.

[volker.dobler, 2014-09-24 12:04:02]

LGTM

[nigeltao, 2014-09-25 00:22:13]

*** Submitted as https://code.google.com/p/go/source/detail?r=c1d3de1db78e ***

net/http: allow double-quotes only on cookie values, not cookie
attribute values, a la RFC 6265 section 4.1.1 "Syntax".

Fixes issue 7751.

LGTM=dr.volker.dobler
R=dr.volker.dobler
CC=bradfitz, golang-codereviews
https://codereview.appspot.com/148890043