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

[jixunmoe]

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 GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\-snip-\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=-snip-
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\-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:

GET http://192.168.2.1/html/footer.html HTTP/1.1
Host: 192.168.2.1
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;

GET http://192.168.2.1/api/user/state-login HTTP/1.1
Host: 192.168.2.1
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])
                                continue

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.

[jixunmoe]

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

package cookiejar

import (
    "net/http"
    "net/http/cookiejar"
    "net/url"
)

type Jar struct {
    *cookiejar.Jar
}

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]

/cc @bradfitz

[vdobler]

/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.