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

Change-Id: Iac12a56397d77a6060a75757ab0daeacc60457f3
GitHub-Last-Rev: a76440e
GitHub-Pull-Request: #66752
Reviewed-on: https://go-review.googlesource.com/c/go/+/577755
Reviewed-by: Damien Neil <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>

Author: nunogoncalves03

api/next/46443.txt

@@ -0,0 +1 @@
+pkg net/http, type Cookie struct, Quoted bool #46443

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.

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 {

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,37 +348,42 @@ 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`}},
 		[]*Cookie{{Name: "special-5", Value: "a,z", Raw: `special-5=a,z`}},
 	},
 	{
 		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,`}},
 		[]*Cookie{{Name: "special-7", Value: "a,", Raw: `special-7=a,`}},
 	},
 	{
 		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,37 +830,37 @@ 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`,
 			cookie: &Cookie{Name: "special-5", Value: "a,z", Raw: `special-5=a,z`},
 		},
 		{
 			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,`,
 			cookie: &Cookie{Name: "special-7", Value: "a,", Raw: `special-7=a,`},
 		},
 		{
 			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: "",

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