net/http: isCookieNameValid in net/http/cookie.go seems overly restrictive

[dprime]

Note: I attempted to post this to the golang nuts list, but my message was rejected twice for unspecified reasons.

What version of Go are you using (go version)?

go version go1.10.1 windows/amd64

Does this issue reproduce with the latest release?

Code in current https://github.com/golang/go/blob/master/src/net/http/cookie.go is the same as my release.

What operating system and processor architecture are you using (go env)?

go env Output

set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\david\AppData\Local\go-build
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=I:\golang
set GORACE=
set GOROOT=I:\Go
set GOTMPDIR=
set GOTOOLDIR=I:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\david\AppData\Local\Temp\go-build742879430=/tmp/go-build -gno-record-gcc-switches

What did you do?

• I make a http post call to a server ( happens to be https://www.ealing.gov.uk/site/custom_scripts/waste_collection/waste_collection.aspx )
• I receive this Set-Cookie: Set-Cookie: ISAWPLB{48BCE7DA-ADD0-4237-A5B8-816663CFDD23}={70F05AF6-A2D2-4861-9D98-B73CEF42E642}; HttpOnly; Path=/

What did you expect to see?

I expected the http package to tolerate sloppy cookie names that exist in the wild on the internet and are supported by all major browsers. This cookie name works on firefox and chrome latest. I've not tested with anything else, but given my local council is using it, and it's produced by a Microsoft application stack, I suspect it works everywhere.

What did you see instead?

net/http/client.go attempts to handle the Set-Cookie header by calling net/http/cookie.go readSetCookies() , but fails silently, swallowing the Set-Cookie without any warning, because it deems the cookie name ISAWPLB{48BCE7DA-ADD0-4237-A5B8-816663CFDD23} invalid because it contains, as far as I can tell, { and }.

This actually means that it's impossible for me to use go (without hacking net/http) to communicate properly with this web server, because the stringer on a Cookie returns "" unless the name is valid, which stops the cookie being included with outbound requests. So, even if I manually handled this badly named cookie, the http client will refuse to send it, regardless.

[agnivade]

/cc @bradfitz

[bradfitz]

If browsers support it, that's a pretty strong argument to change.

But I defer to @nigeltao & @vdobler.

[vdobler]

@dprime You can manually handle these types of cookies:
You are right that the convenience functions like net/http.Request.Cookies and net/http.SetCookies will ignore these cookies. But you can read and set them manually from/to net/http.{Request,Response}.Header directly. Unfortunately this requires you to do all the complicated stuff like parsing the Set-Cookie header yourself. Generating a Cookie header is simple (but as you correctly noted you cannot use net/http.Cookie.String)
So manually working with the raw headers will let you "communicate properly with this web server" "without hacking net/http".

[vdobler]

I just checked:

• Safari Version 12.0.2 (12606.3.4.1.4) (on Mac),
• Opera Version:57.0.3098.110 (on Mac) and
• Internet Explorer Version: 11.0.9600.19129

allow this type of cookie name name. (With Opera being based on Chromium this was expected.)

[dprime]

@vdobler Ah, of course, fair point. I was forgetting you don't have to use a cookie jar, it's possible to manually handle the Set-Cookie responses and set the outbound Cookie header directly. The default impl of the cookie jar can't handle these names at all, though.

[vdobler]

@dprime I might misremember the details but net/http/cookiejar.Jar should be agnostic to the names. I think you can you the default Jar and populate it with SetCookies and retrieve the cookies with Cookies. Of course you loos the automatic integration with the Client, but if you manually parse the Set-Cookie header, you can construct a Cookie, store that in in a Jar with SetCookies and retrieve it back with Cookies. Now create a Cookie header manually from that Cookie.
The Jar would still do the complicated stuff of domain and path matching and expiration.
It is a hack, but it should work.

[dprime]

There's a decent writeup here of the state of cookie names: https://stackoverflow.com/questions/1969232/allowed-characters-in-cookies

Perhaps we should just copy whatever firefox or chromium do?
This is one such spec:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#Directives

[vdobler]

I'm afraid of opening a can of worms:
If we accept more characters in a cookie name we have to support them everywhere, i.e. in Set-Cookie headers and in Cookie headers. That is we not only accept such cookies we receive but we would also would allow to send such cookies. As long a Server and Client share the same types this cannot be fixed (easily). And I doubt that we want e.g. net/http.SetCookie to generate formally invalid cookies.

If we are going to "fix" this we probably have to abandon any spec and just accept everything. Treat (Set-)Cookie headers as UTF-8 encoded and allow anything in name and value, maybe even '=' in values. Adding to the mess people do with cookies "because it works in my browser".

I would suggest "won't fix" as not broken.

[dprime]

I would say that's a reasonable way of thinking about it. However "because it works in my browser" here is equivalent to "this is how pretty much 100% of people currently access http services". Your proposed "fix" is obviously too far - the mozilla spec I linked to is pretty much the standard RFC for tokens in http plus a couple of extras and is likely what Chrome et al use as well.

There is no need for this to be a binary situation of perfect spec adherence or utter chaos. There is an accepted, real world, broad usage of the middle ground. The specs and reality disagree here.

[vdobler]

@dprime The Mozilla spec you linked (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#Directives) clearly forbids the use of { and } characters in cookie names. You see there is not a real "accepted, real world, broad usage of the middle ground", even Mozilla does not implement their own specs.

Browsers are very lax. If I read the Chromium tests in https://chromium.googlesource.com/chromium/src.git/+/master/net/cookies/parsed_cookie_unittest.cc correctly than Chromium will accept stuff like name=普通話 and ภาษาไทย=value as valid cookies. (While still rejecting such names if set through the API, see: https://chromium.googlesource.com/chromium/src.git/+/master/net/cookies/parsed_cookie.cc#168 )

If you know of any document which describes what you call "accepted, real world, broad usage of the middle ground" I would be happy if you could provide a copy or a link to it. Your "middle ground" is muddy: While all agree that you cannot have \r or \000 and = in a cookie-name the rest is (to my knowledge) not formalized at all: All browsers try to mimic the behavior of older ones even reproducing strange bugs and counterintuitive behaviour .

• If this issue is about allowing the two characters { and } in addition to what is currently allowed then I would vote for "won't fix" as this is just kludge.

• If this issue is about "allow any valid Unicode code point which is not obviously unfit in cookie names" then I'd like to hand back over to @bradfitz and @nigeltao .

• If this issue is about allowing some additional bytes then we first have to decide on which ones. I would see @dprime in charge of providing the set.

[nigeltao]

Perhaps we should just copy whatever firefox or chromium do?

Can somebody (@dprime ??) figure out what Firefox and Chromium actually do in terms of code (and what they still reject)? You linked to a Mozilla spec, above, but as @vdobler said, if Mozilla (Firefox) accepts { and } as per the original post, Mozilla's code does not implement their own spec.