net/http: does not handle semicolon-terminated cookie strings very well

[zjclyz]

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

$ go version
1.16.3

Does this issue reproduce with the latest release?

I tried version 1.18 and it did not solve my problem completely.

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

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/Users/xxx/go/bin"
GOCACHE="/Users/xxx/Library/Caches/go-build"
GOENV="/Users/xxx/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/xxx/go/pkg/mod"
GONOPROXY="git.xx.oa.com"
GONOSUMDB="git.xx.oa.com"
GOOS="darwin"
GOPATH="/Users/xxx/go"
GOPRIVATE="git.xx.oa.com"
GOPROXY="https://goproxy.cn,direct"
GOROOT="/Users/xxx/go/go1.16.3"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/xxx/go/go1.16.3/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/xxx/develop/doc_pro1/xxx-xxx/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/gg/4vpkn5v518g_bdwnnq4w6zpm0000gn/T/go-build3887804640=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

The following code has reproduction problems：

import (
	"fmt"
	"net/http"
)

func main() {
	r := &http.Request{Header: map[string][]string{"Cookie": {"test1=1;"}}} // This test cookie string ends with a semicolon
	r.AddCookie(&http.Cookie{Name: "test2",Value: "2"}) // Append a cookie
	for _, c := range r.Cookies() { // Only test1 is printed, test2 is missing
		fmt.Println("cookie:", c.Name, c.Value)
	}
        fmt.Println(r.Header.Get("Cookie"))
}

What did you expect to see?

The actual printed results of the demo code:

cookie: test1 1
test1=1;; test2=2

The result I expect：

cookie: test1 1
cookie: test2 2
test1=1; test2=2

Why can't the http.Request.AddCookie() method remove the semicolon at the end and then append the cookie string?

[dmitshur]

Thanks for the report. Do you know if the relevant specification says anything about this, or is this more related to nuances of the net/http API?

CC @neild.

[zjclyz]

Thanks for the report. Do you know if the relevant specification says anything about this, or is this more related to nuances of the net/http API?

CC @neild.

Sorry, I didn't find the specification either, it's just that we had a problem with it in our production environment, we were using version 1.16 and found that some of the cookie strings ended in a semicolon and then some of the cookies were lost after appending a new cookie. i looked at the http.Request.AddCookie() implementation and it doesn't start with Instead of removing the extra semicolon first, it goes on to add another one. Has the standard library considered correcting this beforehand to make it conform to the standard before adding a new cookie?

[ZekeLu]

FYI, here is the spec for the Cookie header: https://datatracker.ietf.org/doc/html/rfc6265#section-4.2.1

cookie-header = "Cookie:" OWS cookie-string OWS
cookie-string = cookie-pair *( ";" SP cookie-pair )

And cookie-pair is described here: https://datatracker.ietf.org/doc/html/rfc6265#section-4.1

cookie-pair       = cookie-name "=" cookie-value
cookie-name       = token
cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                       ; US-ASCII characters excluding CTLs,
                       ; whitespace DQUOTE, comma, semicolon,
                       ; and backslash

According to the spec, semicolon should be excluded from the cookie-value (US-ASCII characters excluding CTLs, whitespace DQUOTE, comma, semicolon, and backslash). And there is not trailing semicolon in the cookie-string.

[zjclyz]

FYI, here is the spec for the Cookie header: https://datatracker.ietf.org/doc/html/rfc6265#section-4.2.1

cookie-header = "Cookie:" OWS cookie-string OWS
cookie-string = cookie-pair *( ";" SP cookie-pair )

And cookie-pair is described here: https://datatracker.ietf.org/doc/html/rfc6265#section-4.1

cookie-pair       = cookie-name "=" cookie-value
cookie-name       = token
cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                       ; US-ASCII characters excluding CTLs,
                       ; whitespace DQUOTE, comma, semicolon,
                       ; and backslash

According to the spec, semicolon should be excluded from the cookie-value (US-ASCII characters excluding CTLs, whitespace DQUOTE, comma, semicolon, and backslash). And there is not trailing semicolon in the cookie-string.

In theory, it should indeed be as specified in the draft, but our production environment did encounter a cookie string ending in a semicolon, which caused us problems; I was hoping that the standard library would be compatible with this situation, after all, go 1.16 would simply lose the newly appended cookie, and although subsequent versions have fixed this problem, they have never removed that extra semicolon.

[neild]

Request.AddCookie is very simple in how it appends cookie values: If an existing Cookie header is present, it appends a semicolon and space before the new cookie value. This works correctly, so long as the existing Cookie header is valid.

It is not valid for a Cookie header to end in a semicolon. I don't think it makes sense for AddCookie to check for a specific form of invalid Cookie header. If it does make sense, then we need a cohesive understanding of what forms of invalid Cookie headers AddCookie will correct; would we also convert a=b;;b=c into a=b;b=c, for example? What do we do if the existing Cookie header is entirely unparsable?

Go 1.18's Request.Cookies function is more permissive about extraneous semicolons in cookies. If I run the example above with 1.18 (playground link), I see:

cookie: test1 1
cookie: test2 2
test1=1;; test2=2

[seankhliao]

This should also be considered a dup of #38437

[zjclyz]

Ok, thanks for the replies!