Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http/cookie: readSetCookies does not strip whitespace for key/value pair received #40414

jixunmoe opened this issue Jul 26, 2020 · 3 comments


Copy link

@jixunmoe jixunmoe commented Jul 26, 2020

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

$ go version
go version go1.14.6 windows/amd64

Does this issue reproduce with the latest release?

Yes. I'm using the latest go release for Windows.

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

go env Output
$ go env
set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\-snip-\AppData\Local\go-build
set GOENV=C:\Users\-snip-\AppData\Roaming\go\env
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\-snip-\go
set GOPROXY=,direct
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set GOMOD=-snip-
set CGO_CFLAGS=-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\-snip-\AppData\Local\Temp\go-build817710326=/tmp/go-build -gno-record-gcc-switches

What did you do?

Use http.Client with cookie jar to access site with valid Set-Cookie header, but the cookie did not persist.

The first two requests:

User-Agent: Go-http-client/1.1
Accept-Encoding: gzip

HTTP/1.1 200 OK
Date: Thu, 01 Jan 1970 00:00:00 GMT
Server: webserver
Connection: keep-alive
Keep-Alive: timeout=10, max=100
X-Download-Options: noopen
X-Frame-Options: deny
X-XSS-Protection: 1; mode=block
Strict-Transport-Security: max-age=31536000; includeSubdomains
Content-Length: 185
Content-Type: text/html
Expires: 0
Cache-Control: no-cache
Set-Cookie: SessionID=some_value;path =/;HttpOnly;

User-Agent: Go-http-client/1.1
Accept-Encoding: gzip

In the second request, the cookie did not persist. When looking at the cookie jar after the first request, the cookie has been stored under the path /html instead of expected /, which applies to the whole domain.

When I further examine the response header from the initial request, there's a white-space after the path key. This extra whitespace has caused readSetCookies to not recognise the key-pair, and the cookie has been stored relative to the document path (i.e. /html).

A quick read in RFC6265#5.2, step 4 states that the user-agent should strip leading or trailing whitespaces for the key/value string.

An attempt to fix this behaviour:

--- cookie.go   2020-07-16 22:30:44.000000000 +0100
+++ cookie_2.go 2020-07-26 11:33:31.710704300 +0100
@@ -92,8 +92,8 @@
                        if j := strings.Index(attr, "="); j >= 0 {
                                attr, val = attr[:j], attr[j+1:]
-                       lowerAttr := strings.ToLower(attr)
-                       val, ok = parseCookieValue(val, false)
+                       lowerAttr := strings.TrimSpace(strings.ToLower(attr))
+                       val, ok = parseCookieValue(strings.TrimSpace(val), false)
                        if !ok {
                                c.Unparsed = append(c.Unparsed, parts[i])

What did you expect to see?

The cookie is carried with subsequent requests.

What did you see instead?

The cookie was not set in subsequent requests, due to path mismatch.

Copy link

@jixunmoe jixunmoe commented Jul 26, 2020

As a workaround, I've created an alternative cookie jar which forces the path to be /.

package cookiejar

import (

type Jar struct {

func (j *Jar) SetCookies(u *url.URL, cookies []*http.Cookie) {
	for _, cookie := range cookies {
		cookie.Path = "/"

	j.Jar.SetCookies(u, cookies)

func New(opts *cookiejar.Options) (*Jar, error) {
	var err error
	jar := &Jar{}
	jar.Jar, err = cookiejar.New(opts)
	return jar, err
@cagedmantis cagedmantis added this to the Backlog milestone Jul 31, 2020
Copy link

@cagedmantis cagedmantis commented Jul 31, 2020

/cc @bradfitz

Copy link

@vdobler vdobler commented Aug 21, 2020

/cc @nigeltao

This seems like a real bug. The "Note" in RFC 6265 5.2 makes it pretty clear:

NOTE: The algorithm below is more permissive than the grammar in
Section 4.1. For example, the algorithm strips leading and trailing
whitespace from the cookie name and value (but maintains internal
whitespace), whereas the grammar in Section 4.1 forbids whitespace in
these positions. User agents use this algorithm so as to
interoperate with servers that do not follow the recommendations in
Section 4.

So while sending a Set-Cookie header of the form
Set-Cookie: SessionID=some_value;path =/;HttpOnly;
is a violation of RFC 6265 we probably should accept it.

@jixunmoe You write

with valid Set-Cookie header
The Set-Cookie header is technically not valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants