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: a76440e741440cddaa05944b6828a14a32b5a44a
GitHub-Pull-Request: golang/go#66752
Reviewed-on: https://go-review.googlesource.com/c/go/+/577755
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
diff --git a/api/next/46443.txt b/api/next/46443.txt
new file mode 100644
index 0000000..a4e6fc4
--- /dev/null
+++ b/api/next/46443.txt
@@ -0,0 +1 @@
+pkg net/http, type Cookie struct, Quoted bool #46443
diff --git a/doc/next/6-stdlib/99-minor/net/http/46443.md b/doc/next/6-stdlib/99-minor/net/http/46443.md
new file mode 100644
index 0000000..7305820
--- /dev/null
+++ 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.
diff --git a/src/net/http/cookie.go b/src/net/http/cookie.go
index ab84625..2a81707 100644
--- a/src/net/http/cookie.go
+++ b/src/net/http/cookie.go
@@ -21,8 +21,9 @@
//
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
// 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 @@
// ; 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 @@
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 {
diff --git a/src/net/http/cookie_test.go b/src/net/http/cookie_test.go
index ce5093c..de47682 100644
--- a/src/net/http/cookie_test.go
+++ b/src/net/http/cookie_test.go
@@ -147,6 +147,19 @@
&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 @@
},
"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 @@
},
{
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 @@
},
{
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 @@
},
{
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 @@
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 @@
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 @@
},
{
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 @@
},
{
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 @@
},
{
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 @@
},
{
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: "",
diff --git a/src/net/http/cookiejar/jar.go b/src/net/http/cookiejar/jar.go
index e7f5ddd..280f465 100644
--- a/src/net/http/cookiejar/jar.go
+++ b/src/net/http/cookiejar/jar.go
@@ -92,6 +92,7 @@
type entry struct {
Name string
Value string
+ Quoted bool
Domain string
Path string
SameSite string
@@ -220,7 +221,7 @@
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 @@
}
e.Value = c.Value
+ e.Quoted = c.Quoted
e.Secure = c.Secure
e.HttpOnly = c.HttpOnly
diff --git a/src/net/http/cookiejar/jar_test.go b/src/net/http/cookiejar/jar_test.go
index 251f7c1..93b3518 100644
--- a/src/net/http/cookiejar/jar_test.go
+++ b/src/net/http/cookiejar/jar_test.go
@@ -404,7 +404,12 @@
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 @@
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 @@
{"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) {
diff --git a/src/net/http/request.go b/src/net/http/request.go
index 345ba3d..bdd18ad 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -464,7 +464,7 @@
// 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 {
