Closed shanemhansen closed 9 years ago
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.
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
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?
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.
Proposed CL here: https://golang.org/cl/81680043
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.
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.
Here's one: https://gist.github.com/pokeb/10590 Here's another: http://www.hashbangcode.com/blog/netscape-http-cooke-file-parser-php
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.
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.
CL https://golang.org/cl/81680043 references this issue.
CL https://golang.org/cl/86050045 mentions this issue.
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?
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.