net/http: support cookie values with comma

[shanemhansen]

Go has a RFC 6265 compliant cookie parser in net/http that is used for the builtin http
server. Which means commas aren't allowed in cookie values (what the rfc specified in
the BNF as "cookie-octet").

Despite not being RFC compliant, all browsers that I'm aware of allow commas. When
replatforming an existing system to Golang, I'm unable to read in cookies set by the
another server (nginx) or set cookies in a way that mimics the legacy app.

I'd like to see net/http/cookie.go do less validation on cookie values so that it
matches the behavior of firefox and chrome, because this will make it easier to replace
legacy systems with Go.

I can produce a patch. The relevant lines of code in the go parser are:
http://golang.org/src/pkg/net/http/cookie.go#L308
http://golang.org/src/pkg/net/http/cookie.go#L350


Existing browser implementations:
http://mxr.mozilla.org/mozilla-central/source/netwerk/cookie/nsCookieService.cpp#3095

http://src.chromium.org/viewvc/chrome/trunk/src/net/cookies/parsed_cookie.cc#l143

Which compiler are you using (5g, 6g, 8g, gccgo)?
Affects all compilers

Which operating system are you using?
Affects all OS's

Which version are you using?  (run 'go version')
go 1.2

[nigeltao]

Comment 2:

RFC 6265 distinguishes between the two roles of Servers and User Agents. Servers should
adhere to stricter requirements; User Agents have to play nice with looser,
non-compliant servers.
Section 5.2. "The Set-Cookie Header" says that "The algorithm below is more permissive
than the grammar in Section 4.1 (Syntax)", and implicitly allows commas (or really, any
bytes up to the first ';' or end-of-line, I think).
I think that when generating a Set-Cookie header and consuming a Cookie header, Go's
net/http library should use the stricter rules. When consuming a Set-Cookie header and
generating a Cookie header, we should use the looser rules.

[nigeltao]

Comment 3:

Section 5.2 implicitly allows commas, and the 0x7f DEL byte, and non-ASCII bytes above
0x7f, but I think we should still block control characters less than 0x20:
http://crbug.com/238041

[vdobler]

Comment 4:

Two remarks.
First: Currently
    cookie := http.Cookie{Name: "n", Value: "a,b"}
    fmt.Println(cookie.String()) // n=ab
    cookie.MaxAge = 60
    fmt.Println(cookie.String()) // n=ab; MaxAge=60
yields
  n=ab
  n=ab; MaxAge=60
With Nigel's suggestion it would print
  n=a,b             <-- Cookie header with looser rules
  n=ab; MaxAge=60   <-- Set-Cookie header with stricter rules
This is behaviour of the String method of http.Cookie not
really intuitive.
Package net/http would behave almost "schizophrenic": Sometimes
it acts as a strict server, sometimes as a liberal user agent.
Second: How would you write a proxy sitting between a
broken server and a forgiving browser? The server might
send malformed Set-Cookie header (which we would accept
on base of looser rules) but we wouldn't be able to
pass them through as the stricter rules apply for generating
the Set-Cookie header which has to be sent to the browser.
Admittedly such a proxy would not work in Go 1.2 without
manually and deliberately moving raw HTTP headers. But at least
it is clear and explicit: We are willing to handle broken stuff.
On the other hand RFC 6265 requires the behaviour of section
5.2 as a MUST, but then only for user agents...
Is package net/http a server or a user agent (or both)
if viewed under the light of RFC 6265?

[rsc]

Comment 5:

We need to support cookies with commas, one way or another.

Labels changed: added release-go1.3.

Status changed to Accepted.

[gopherbot]

Comment 6 by richard.quirk:

While we're making requests for a more lenient policy ;-) can the client side also
please accept cookies that are sent with spaces? I'm talking to a 3rd party server out
of my control that uses spaces in its Set-Cookie header ("Date=2014-03-21 15:00:00") and
without local hacks to the go source these cookies are silently dropped in my go client
application.

[shanemhansen]

Comment 7:

Proposed CL here:
https://golang.org/cl/81680043

[nigeltao]

Comment 8:

I propose to have a single policy for clients and servers: RFC 6265 4.1.1's cookie
octet, as well as allowing spaces and commas (0x20 and 0x2C). In other words, the
*inclusive* range [0x20, 0x7e] minus the double-quote 0x22, semi-colon 0x3b and
backslash 0x5c. Furthermore, whenever a Go net/http server *writes* Cookie or Set-Cookie
headers, it quotes the values.
Allowing spaces and commas appears to be the minimal amount to address what Go
programmers are encountering from non-compliant servers in the real world. Mandating
quotes seems to keep the big four browsers (Chrome, FF, IE, Safari) happy even with
spaces and commas (Volker has some useful data in that proposed CL discussion at
https://golang.org/cl/81680043#msg4).
It means that the Go  library would then implement neither the server spec of 4.1.1 or
the user-agent spec of 5.2 (cue https://xkcd.com/927/) but I talked to abarth, the
author of RFC 6265, and he agreed that there wasn't an obvious answer, as the Go library
is trying to address all three use cases of client, server and proxy. User-agents
outside of the big four browsers may or may not have problems with this proposal, but I
don't know of any actual (as opposed to hypothetical) problems.
The "mandate quotes when writing cookie values" thing is worth highlighting. I don't
expect it to cause any problems but even so, I'll air it on the golang-dev mailing list.
Once we reach consensus, patching the actual code is trivial. Somewhat harder is fixing
up all the tests, and writing a good comment.

[vdobler]

Comment 9:

Just did some check on Opera 12 (on Linux): Double-quoting values works like a charm.
Even characters < 0x20 are properly handled by Opera if double-quoted.

[dsymonds]

Comment 10:

Is uniformly quoting cookie values going to cause problems with cookie parsers written
in other places? I'm sure there's lots of situations that may have a very restricted
cookie value range and someone's hand-written a trivial parser for it; quoting when it's
not needed would break those situations.

[nigeltao]

Comment 11:

Are you sure people are hand-writing parsers and not using standard cookie libraries
(whether in Go or in other languages) that will understand quoting? That's why I asked
on golang-dev, and I still don't know of a single case.

[dsymonds]

Comment 12:

Here's one: https://gist.github.com/pokeb/10590
Here's another: http://www.hashbangcode.com/blog/netscape-http-cooke-file-parser-php

[nigeltao]

Comment 13:

Alternative proposal. Split bytes into three sets:
white: [0x21, 0x7e] minus double-quote, comma, semi-colon and backslash.
gray: 0x20 space and 0x2c comma.
black: everything else.
Drop any black bytes. If there are any gray bytes, quote the entire cookie value.
Otherwise (white bytes only), do not quote the value.

[vdobler]

Comment 14:

The number of quoted cookie values could be reduced even further:
Partitioning of byte range like in Nigel's proposal.
Drop any black bytes.
If the cookie value starts or ends with a gray bytes, quote the entire cookie value.
Otherwise, do not quote the value.
This would not quote a value like '3,141' or 'cats and dogs'. 
But I am fine with Nigel's proposal.

[gopherbot]

Comment 15:

CL https://golang.org/cl/81680043 references this issue.

[gopherbot]

Comment 16:

CL https://golang.org/cl/86050045 mentions this issue.

[nigeltao]

Comment 17:

This issue was closed by revision ed88076.

Status changed to Fixed.

[gopherbot]

Comment 18 by whitegolem:

Quick question on the side, this proposal still doesn't allow backslashes in cookie
values. But I came across a similar situation to #6 today, where I'm talking to a
3rd-party server, that's sending me a cookie containing backslashes.
So what would you recommend doing in this case, as I suppose there's good reasons for
disallowing backslashes, double-quotes, etc. from cookies by default? Is there a better
way than just extracting/duplicating all the relevant code from net/http?

[nigeltao]

Comment 19:

You're probably going to have to copy/paste the net/http code into your own package. As
you said, there are good reasons for disallowing backslashes. I think that your
3rd-party server is simply sending bad cookies, and I wouldn't be surprised if the
popular browsers also reject them.

[gopherbot]

Comment 20 by whitegolem:

I actually found a hack to keep me from having to copy/paste the whole package. It's not
an ideal solution, but it works.
Basically, I just manually replace all the invalid characters in the set-cookie header
of the response and then store them in a jar. And for requests, when the cookies are
loaded I just replace the placeholder characters with the "bad" ones directly in the
cookie header.
It's obviously not perfect, since you can't have all characters in the cookie, that
would otherwise be supported.
P.S. In fact, the Android http client doesn't reject these malformed cookies. They
integrate flawlessly.