net/http: readCookies unable to parse out cookies that are not well written

[shidawuhen]

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

$ go version
go version go1.13.10 darwin/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output

$ go env
GO111MODULE="auto"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/pangzhiqiang/Library/Caches/go-build"
GOENV="/Users/pangzhiqiang/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY="micode.be.xiaomi.com"
GONOSUMDB="micode.be.xiaomi.com"
GOOS="darwin"
GOPATH="/Users/pangzhiqiang/data/code/golang/myproject"
GOPRIVATE="micode.be.xiaomi.com"
GOROOT="/usr/local/Cellar/[email protected]/1.13.10_1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/[email protected]/1.13.10_1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/pangzhiqiang/data/code/golang/myproject/asap/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 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/0h/652yzkf17qd7n446g954_1j40000gn/T/go-build435664031=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

1. server: create a go server, and create an api, this api is to display the cookies passed by the client

package main

import (
	"net/http"

	_ "asap/docs"

	"github.com/gin-gonic/gin"
)

func setupRouter() *gin.Engine {
	r := gin.Default()

	// Ping test
	r.GET("/ping", ping)

	return r
}

// @Summary 接口探活
// @Produce  json
// @Param lang query string false "en"
// @Success 200 {string} string "ok"
// @Router /ping [get]
func ping(c *gin.Context) {
	cookies := c.Request.Cookies()
	cookieInfo := ""
	for _, cookie := range cookies{
		cookieInfo += cookie.Name + ":" + cookie.Value + "\n"
	}
	c.String(http.StatusOK, cookieInfo )

}

func main() {
	r := setupRouter()
	// Listen and Server in 0.0.0.0:8080
	r.Run(":9090")
}

2.client：

<?php
$url = "http://127.0.0.1:9090/ping";
$cookie = "; xmuuid=XMGUEST-FCF117BF-4D1B-272F-829D-25E19826D4F8;type=protobuf";
$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, $url);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
curl_setopt($ch, CURLOPT_COOKIE, $cookie);
$output = curl_exec($ch);
curl_close($ch);
var_dump($output) ;

What did you expect to see?

string(66) "xmuuid:XMGUEST-FCF117BF-4D1B-272F-829D-25E19826D4F8
type:protobuf
"

What did you see instead?

string(0) ""

Reason

readCookies can't parse this formate

if splitIndex := strings.Index(line, ";"); splitIndex > 0 {
				part, line = line[:splitIndex], line[splitIndex+1:]
			} else {
				part, line = line, ""
			}

[gopherbot]

Change https://golang.org/cl/233978 mentions this issue: net/http: fix readCookies unable to parse out cookies that are not well written

[odeke-em]

Thank you for filing this issue @shidawuhen and welcome to the Go project!

So firstly, what's the reason why we should be allowing malformed cookies? Do other servers allow these malformed cookies? What does Node.js do? Python? Java (Netty or Jetty) do?

Kindly pinging some cookie and security experts because this could use their eyes @vdobler @katiehockman @FiloSottile.

[vdobler]

This is one of several issues reported about our strict handling of cookie-list with empty elements.

The Cookie header set with the PHP client code is malformed according to https://tools.ietf.org/html/rfc6265#section-5.4 .
We try to parse malformed stuff if it is very common in the following sense: It must be working on A) all modern browsers and it must be handled by B) all dominant HTTP libraries in other languages.

@shidawuhen Your Cookie header is handcrafted and thus would not qualify for A and not for B. Can you provide evidence that any browser sends such Cookie headers based on normally set cookies or that any HTTP library produces such Cookie headers?

[shidawuhen]

@odeke-em @vdobler
We have an Android app with a large number of users.

When the APP requests the api of our server, the format of the cookie is "token = abc; xmuuid = def;".

The token is the authentication information of the user login.

When the user is not logged in, the cookie is "; xmuuid = def;"

Of course, this is indeed because Android developer did not write it very well, but the version <= go1.12 is supported, and the problem has recently been discovered by the service upgrade go version to go 1.13. The old version supports but the new version does not support, which does bring about this problem.

And we use the beego framework, it is not easy to handle cookies in the service.

[gopherbot]

Change https://golang.org/cl/234961 mentions this issue: net/http: fix readCookies unable to parse out cookies that are not well written

[gopherbot]

Change https://golang.org/cl/235141 mentions this issue: net/http: clarify that AddCookie appends to whatever the Cookie header contains

[vdobler]

Was fixed by https://golang.org/cl/235141 .

[odeke-em]

Thanks for the reminder @vdobler, and for working on it too.