net/http: add field Cookie.Quoted bool

The current implementation of the http package strips double quotes from the cookie-value during parsing, resulting in the serialized cookie not including them. This patch addresses this limitation by introducing a new field to track whether the original value was enclosed in quotes. Additionally, the internal representation of a cookie in the cookiejar package has been adjusted to align with the new representation. The syntax of cookies is outlined in RFC 6265 Section 4.1.1: https://datatracker.ietf.org/doc/html/rfc6265\#section-4.1.1 Fixes #46443 Co-authored-by: Fábio Mata <fabio.mata@tecnico.ulisboa.pt>
2024-09-23 07:23:18 -06:00 · 2024-04-01 18:56:13 +01:00 · 2024-04-01 18:56:13 +01:00 · a76440e741
commit a76440e741
parent 68c0ad3a68
7 changed files with 124 additions and 53 deletions
--- a/api/next/46443.txt
+++ b/api/next/46443.txt
@ -0,0 +1 @@
+pkg net/http, type Cookie struct, Quoted bool #46443
--- a/doc/next/6-stdlib/99-minor/net/http/46443.md
+++ b/doc/next/6-stdlib/99-minor/net/http/46443.md
@ -0,0 +1,3 @@
+[`Cookie`](/pkg/net/http#Cookie) now preserves double quotes surrounding
+a cookie value. The new `Cookie.Quoted` field indicates whether the
+`Cookie.Value` was originally quoted.
--- a/src/net/http/cookie.go
+++ b/src/net/http/cookie.go
@ -21,8 +21,9 @@ import (
 //
 // See https://tools.ietf.org/html/rfc6265 for details.
 type Cookie struct {
-	Name  string
-	Value string
+	Name   string
+	Value  string
+	Quoted bool // indicates whether the Value was originally quoted

 	Path       string    // optional
 	Domain     string    // optional
@ -80,11 +81,11 @@ func ParseCookie(line string) ([]*Cookie, error) {
 		if !isCookieNameValid(name) {
 			return nil, errInvalidCookieName
 		}
-		value, found = parseCookieValue(value, true)
+		value, quoted, found := parseCookieValue(value, true)
 		if !found {
 			return nil, errInvalidCookieValue
 		}
-		cookies = append(cookies, &Cookie{Name: name, Value: value})
+		cookies = append(cookies, &Cookie{Name: name, Value: value, Quoted: quoted})
 	}
 	return cookies, nil
 }
@ -105,14 +106,15 @@ func ParseSetCookie(line string) (*Cookie, error) {
 	if !isCookieNameValid(name) {
 		return nil, errInvalidCookieName
 	}
-	value, ok = parseCookieValue(value, true)
+	value, quoted, ok := parseCookieValue(value, true)
 	if !ok {
 		return nil, errInvalidCookieValue
 	}
 	c := &Cookie{
-		Name:  name,
-		Value: value,
-		Raw:   line,
+		Name:   name,
+		Value:  value,
+		Quoted: quoted,
+		Raw:    line,
 	}
 	for i := 1; i < len(parts); i++ {
 		parts[i] = textproto.TrimString(parts[i])
@ -125,7 +127,7 @@ func ParseSetCookie(line string) (*Cookie, error) {
 		if !isASCII {
 			continue
 		}
-		val, ok = parseCookieValue(val, false)
+		val, _, ok = parseCookieValue(val, false)
 		if !ok {
 			c.Unparsed = append(c.Unparsed, parts[i])
 			continue
@ -229,7 +231,7 @@ func (c *Cookie) String() string {
 	b.Grow(len(c.Name) + len(c.Value) + len(c.Domain) + len(c.Path) + extraCookieLength)
 	b.WriteString(c.Name)
 	b.WriteRune('=')
-	b.WriteString(sanitizeCookieValue(c.Value))
+	b.WriteString(sanitizeCookieValue(c.Value, c.Quoted))

 	if len(c.Path) > 0 {
 		b.WriteString("; Path=")
@ -341,11 +343,11 @@ func readCookies(h Header, filter string) []*Cookie {
 			if filter != "" && filter != name {
 				continue
 			}
-			val, ok := parseCookieValue(val, true)
+			val, quoted, ok := parseCookieValue(val, true)
 			if !ok {
 				continue
 			}
-			cookies = append(cookies, &Cookie{Name: name, Value: val})
+			cookies = append(cookies, &Cookie{Name: name, Value: val, Quoted: quoted})
 		}
 	}
 	return cookies
@ -430,6 +432,8 @@ func sanitizeCookieName(n string) string {
 }

 // sanitizeCookieValue produces a suitable cookie-value from v.
+// It receives a quoted bool indicating whether the value was originally
+// quoted.
 // https://tools.ietf.org/html/rfc6265#section-4.1.1
 //
 //	cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
@ -439,15 +443,14 @@ func sanitizeCookieName(n string) string {
 //	          ; and backslash
 //
 // We loosen this as spaces and commas are common in cookie values
-// but we produce a quoted cookie-value if and only if v contains
-// commas or spaces.
+// thus we produce a quoted cookie-value if v contains commas or spaces.
 // See https://golang.org/issue/7243 for the discussion.
-func sanitizeCookieValue(v string) string {
+func sanitizeCookieValue(v string, quoted bool) string {
 	v = sanitizeOrWarn("Cookie.Value", validCookieValueByte, v)
 	if len(v) == 0 {
 		return v
 	}
-	if strings.ContainsAny(v, " ,") {
+	if strings.ContainsAny(v, " ,") || quoted {
 		return `"` + v + `"`
 	}
 	return v
@ -489,17 +492,27 @@ func sanitizeOrWarn(fieldName string, valid func(byte) bool, v string) string {
 	return string(buf)
 }

-func parseCookieValue(raw string, allowDoubleQuote bool) (string, bool) {
+// parseCookieValue parses a cookie value according to RFC 6265.
+// If allowDoubleQuote is true, parseCookieValue will consider that it
+// is parsing the cookie-value;
+// otherwise, it will consider that it is parsing a cookie-av value
+// (cookie attribute-value).
+//
+// It returns the parsed cookie value, a boolean indicating whether the
+// parsing was successful, and a boolean indicating whether the parsed
+// value was enclosed in double quotes.
+func parseCookieValue(raw string, allowDoubleQuote bool) (value string, quoted, ok bool) {
 	// Strip the quotes, if present.
 	if allowDoubleQuote && len(raw) > 1 && raw[0] == '"' && raw[len(raw)-1] == '"' {
 		raw = raw[1 : len(raw)-1]
+		quoted = true
 	}
 	for i := 0; i < len(raw); i++ {
 		if !validCookieValueByte(raw[i]) {
-			return "", false
+			return "", quoted, false
 		}
 	}
-	return raw, true
+	return raw, quoted, true
 }

 func isCookieNameValid(raw string) bool {
--- a/src/net/http/cookie_test.go
+++ b/src/net/http/cookie_test.go
@ -147,6 +147,19 @@ var writeSetCookiesTests = []struct {
 		&Cookie{Name: "a\rb", Value: "v"},
 		``,
 	},
+	// Quoted values (issue #46443)
+	{
+		&Cookie{Name: "cookie", Value: "quoted", Quoted: true},
+		`cookie="quoted"`,
+	},
+	{
+		&Cookie{Name: "cookie", Value: "quoted with spaces", Quoted: true},
+		`cookie="quoted with spaces"`,
+	},
+	{
+		&Cookie{Name: "cookie", Value: "quoted,with,commas", Quoted: true},
+		`cookie="quoted,with,commas"`,
+	},
 }

 func TestWriteSetCookies(t *testing.T) {
@ -215,6 +228,15 @@ var addCookieTests = []struct {
 		},
 		"cookie-1=v$1; cookie-2=v$2; cookie-3=v$3",
 	},
+	// Quoted values (issue #46443)
+	{
+		[]*Cookie{
+			{Name: "cookie-1", Value: "quoted", Quoted: true},
+			{Name: "cookie-2", Value: "quoted with spaces", Quoted: true},
+			{Name: "cookie-3", Value: "quoted,with,commas", Quoted: true},
+		},
+		`cookie-1="quoted"; cookie-2="quoted with spaces"; cookie-3="quoted,with,commas"`,
+	},
 }

 func TestAddCookie(t *testing.T) {
@ -326,15 +348,15 @@ var readSetCookiesTests = []struct {
 	},
 	{
 		Header{"Set-Cookie": {`special-2=" z"`}},
-		[]*Cookie{{Name: "special-2", Value: " z", Raw: `special-2=" z"`}},
+		[]*Cookie{{Name: "special-2", Value: " z", Quoted: true, Raw: `special-2=" z"`}},
 	},
 	{
 		Header{"Set-Cookie": {`special-3="a "`}},
-		[]*Cookie{{Name: "special-3", Value: "a ", Raw: `special-3="a "`}},
+		[]*Cookie{{Name: "special-3", Value: "a ", Quoted: true, Raw: `special-3="a "`}},
 	},
 	{
 		Header{"Set-Cookie": {`special-4=" "`}},
-		[]*Cookie{{Name: "special-4", Value: " ", Raw: `special-4=" "`}},
+		[]*Cookie{{Name: "special-4", Value: " ", Quoted: true, Raw: `special-4=" "`}},
 	},
 	{
 		Header{"Set-Cookie": {`special-5=a,z`}},
@ -342,7 +364,7 @@ var readSetCookiesTests = []struct {
 	},
 	{
 		Header{"Set-Cookie": {`special-6=",z"`}},
-		[]*Cookie{{Name: "special-6", Value: ",z", Raw: `special-6=",z"`}},
+		[]*Cookie{{Name: "special-6", Value: ",z", Quoted: true, Raw: `special-6=",z"`}},
 	},
 	{
 		Header{"Set-Cookie": {`special-7=a,`}},
@ -350,13 +372,18 @@ var readSetCookiesTests = []struct {
 	},
 	{
 		Header{"Set-Cookie": {`special-8=","`}},
-		[]*Cookie{{Name: "special-8", Value: ",", Raw: `special-8=","`}},
+		[]*Cookie{{Name: "special-8", Value: ",", Quoted: true, Raw: `special-8=","`}},
 	},
 	// Make sure we can properly read back the Set-Cookie headers
 	// for names containing spaces:
 	{
 		Header{"Set-Cookie": {`special-9 =","`}},
-		[]*Cookie{{Name: "special-9", Value: ",", Raw: `special-9 =","`}},
+		[]*Cookie{{Name: "special-9", Value: ",", Quoted: true, Raw: `special-9 =","`}},
+	},
+	// Quoted values (issue #46443)
+	{
+		Header{"Set-Cookie": {`cookie="quoted"`}},
+		[]*Cookie{{Name: "cookie", Value: "quoted", Quoted: true, Raw: `cookie="quoted"`}},
 	},

 	// TODO(bradfitz): users have reported seeing this in the
@ -425,15 +452,15 @@ var readCookiesTests = []struct {
 		Header{"Cookie": {`Cookie-1="v$1"; c2="v2"`}},
 		"",
 		[]*Cookie{
-			{Name: "Cookie-1", Value: "v$1"},
-			{Name: "c2", Value: "v2"},
+			{Name: "Cookie-1", Value: "v$1", Quoted: true},
+			{Name: "c2", Value: "v2", Quoted: true},
 		},
 	},
 	{
 		Header{"Cookie": {`Cookie-1="v$1"; c2=v2;`}},
 		"",
 		[]*Cookie{
-			{Name: "Cookie-1", Value: "v$1"},
+			{Name: "Cookie-1", Value: "v$1", Quoted: true},
 			{Name: "c2", Value: "v2"},
 		},
 	},
@ -486,23 +513,26 @@ func TestCookieSanitizeValue(t *testing.T) {
 	log.SetOutput(&logbuf)

 	tests := []struct {
-		in, want string
+		in     string
+		quoted bool
+		want   string
 	}{
-		{"foo", "foo"},
-		{"foo;bar", "foobar"},
-		{"foo\\bar", "foobar"},
-		{"foo\"bar", "foobar"},
-		{"\x00\x7e\x7f\x80", "\x7e"},
-		{`"withquotes"`, "withquotes"},
-		{"a z", `"a z"`},
-		{" z", `" z"`},
-		{"a ", `"a "`},
-		{"a,z", `"a,z"`},
-		{",z", `",z"`},
-		{"a,", `"a,"`},
+		{"foo", false, "foo"},
+		{"foo;bar", false, "foobar"},
+		{"foo\\bar", false, "foobar"},
+		{"foo\"bar", false, "foobar"},
+		{"\x00\x7e\x7f\x80", false, "\x7e"},
+		{`withquotes`, true, `"withquotes"`},
+		{`"withquotes"`, true, `"withquotes"`}, // double quotes are not valid octets
+		{"a z", false, `"a z"`},
+		{" z", false, `" z"`},
+		{"a ", false, `"a "`},
+		{"a,z", false, `"a,z"`},
+		{",z", false, `",z"`},
+		{"a,", false, `"a,"`},
 	}
 	for _, tt := range tests {
-		if got := sanitizeCookieValue(tt.in); got != tt.want {
+		if got := sanitizeCookieValue(tt.in, tt.quoted); got != tt.want {
 			t.Errorf("sanitizeCookieValue(%q) = %q; want %q", tt.in, got, tt.want)
 		}
 	}
@ -668,7 +698,7 @@ func TestParseCookie(t *testing.T) {
 		},
 		{
 			line:    `Cookie-1="v$1";c2="v2"`,
-			cookies: []*Cookie{{Name: "Cookie-1", Value: "v$1"}, {Name: "c2", Value: "v2"}},
+			cookies: []*Cookie{{Name: "Cookie-1", Value: "v$1", Quoted: true}, {Name: "c2", Value: "v2", Quoted: true}},
 		},
 		{
 			line:    "k1=",
@ -800,15 +830,15 @@ func TestParseSetCookie(t *testing.T) {
 		},
 		{
 			line:   `special-2=" z"`,
-			cookie: &Cookie{Name: "special-2", Value: " z", Raw: `special-2=" z"`},
+			cookie: &Cookie{Name: "special-2", Value: " z", Quoted: true, Raw: `special-2=" z"`},
 		},
 		{
 			line:   `special-3="a "`,
-			cookie: &Cookie{Name: "special-3", Value: "a ", Raw: `special-3="a "`},
+			cookie: &Cookie{Name: "special-3", Value: "a ", Quoted: true, Raw: `special-3="a "`},
 		},
 		{
 			line:   `special-4=" "`,
-			cookie: &Cookie{Name: "special-4", Value: " ", Raw: `special-4=" "`},
+			cookie: &Cookie{Name: "special-4", Value: " ", Quoted: true, Raw: `special-4=" "`},
 		},
 		{
 			line:   `special-5=a,z`,
@ -816,7 +846,7 @@ func TestParseSetCookie(t *testing.T) {
 		},
 		{
 			line:   `special-6=",z"`,
-			cookie: &Cookie{Name: "special-6", Value: ",z", Raw: `special-6=",z"`},
+			cookie: &Cookie{Name: "special-6", Value: ",z", Quoted: true, Raw: `special-6=",z"`},
 		},
 		{
 			line:   `special-7=a,`,
@ -824,13 +854,13 @@ func TestParseSetCookie(t *testing.T) {
 		},
 		{
 			line:   `special-8=","`,
-			cookie: &Cookie{Name: "special-8", Value: ",", Raw: `special-8=","`},
+			cookie: &Cookie{Name: "special-8", Value: ",", Quoted: true, Raw: `special-8=","`},
 		},
 		// Make sure we can properly read back the Set-Cookie headers
 		// for names containing spaces:
 		{
 			line:   `special-9 =","`,
-			cookie: &Cookie{Name: "special-9", Value: ",", Raw: `special-9 =","`},
+			cookie: &Cookie{Name: "special-9", Value: ",", Quoted: true, Raw: `special-9 =","`},
 		},
 		{
 			line: "",
--- a/src/net/http/cookiejar/jar.go
+++ b/src/net/http/cookiejar/jar.go
@ -92,6 +92,7 @@ func New(o *Options) (*Jar, error) {
 type entry struct {
 	Name       string
 	Value      string
+	Quoted     bool
 	Domain     string
 	Path       string
 	SameSite   string
@ -220,7 +221,7 @@ func (j *Jar) cookies(u *url.URL, now time.Time) (cookies []*http.Cookie) {
 		return s[i].seqNum < s[j].seqNum
 	})
 	for _, e := range selected {
-		cookies = append(cookies, &http.Cookie{Name: e.Name, Value: e.Value})
+		cookies = append(cookies, &http.Cookie{Name: e.Name, Value: e.Value, Quoted: e.Quoted})
 	}

 	return cookies
@ -429,6 +430,7 @@ func (j *Jar) newEntry(c *http.Cookie, now time.Time, defPath, host string) (e e
 	}

 	e.Value = c.Value
+	e.Quoted = c.Quoted
 	e.Secure = c.Secure
 	e.HttpOnly = c.HttpOnly

--- a/src/net/http/cookiejar/jar_test.go
+++ b/src/net/http/cookiejar/jar_test.go
@ -404,7 +404,12 @@ func (test jarTest) run(t *testing.T, jar *Jar) {
 			if !cookie.Expires.After(now) {
 				continue
 			}
-			cs = append(cs, cookie.Name+"="+cookie.Value)
+
+			v := cookie.Value
+			if strings.ContainsAny(v, " ,") || cookie.Quoted {
+				v = `"` + v + `"`
+			}
+			cs = append(cs, cookie.Name+"="+v)
 		}
 	}
 	sort.Strings(cs)
@ -421,7 +426,7 @@ func (test jarTest) run(t *testing.T, jar *Jar) {
 		now = now.Add(1001 * time.Millisecond)
 		var s []string
 		for _, c := range jar.cookies(mustParseURL(query.toURL), now) {
-			s = append(s, c.Name+"="+c.Value)
+			s = append(s, c.String())
 		}
 		if got := strings.Join(s, " "); got != query.want {
 			t.Errorf("Test %q #%d\ngot  %q\nwant %q", test.description, i, got, query.want)
@ -639,6 +644,23 @@ var basicsTests = [...]jarTest{
 			{"https://[::1%25.example.com]:80/", ""},
 		},
 	},
+	{
+		"Retrieval of cookies with quoted values", // issue #46443
+		"http://www.host.test/",
+		[]string{
+			`cookie-1="quoted"`,
+			`cookie-2="quoted with spaces"`,
+			`cookie-3="quoted,with,commas"`,
+			`cookie-4= ,`,
+		},
+		`cookie-1="quoted" cookie-2="quoted with spaces" cookie-3="quoted,with,commas" cookie-4=" ,"`,
+		[]query{
+			{
+				"http://www.host.test",
+				`cookie-1="quoted" cookie-2="quoted with spaces" cookie-3="quoted,with,commas" cookie-4=" ,"`,
+			},
+		},
+	},
 }

 func TestBasics(t *testing.T) {
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@ -464,7 +464,7 @@ func (r *Request) Cookie(name string) (*Cookie, error) {
 // AddCookie only sanitizes c's name and value, and does not sanitize
 // a Cookie header already present in the request.
 func (r *Request) AddCookie(c *Cookie) {
-	s := fmt.Sprintf("%s=%s", sanitizeCookieName(c.Name), sanitizeCookieValue(c.Value))
+	s := fmt.Sprintf("%s=%s", sanitizeCookieName(c.Name), sanitizeCookieValue(c.Value, c.Quoted))
 	if c := r.Header.Get("Cookie"); c != "" {
 		r.Header.Set("Cookie", c+"; "+s)
 	} else {
				`@ -0,0 +1 @@`
				`pkg net/http, type Cookie struct, Quoted bool #46443`