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 (file)
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 (file)
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 ab84625ba0bdb9d49c22e3a2782dc59e10da1aee..2a8170709b4874c1c5641054b6814759bb0eb26a 100644 (file)
--- 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 {

diff --git a/src/net/http/cookie_test.go b/src/net/http/cookie_test.go
index ce5093c2ea4fa5b38f547b68e1eff6d3febfb674..de476825cf08956b90397a6f362bd0312e8af254 100644 (file)
--- 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: "",

diff --git a/src/net/http/cookiejar/jar.go b/src/net/http/cookiejar/jar.go
index e7f5ddd4d0096b9d036d6b7f4258d5695d9d9761..280f4650c1815f01ddfb0184e3e15b5efd98739d 100644 (file)
--- 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
 

diff --git a/src/net/http/cookiejar/jar_test.go b/src/net/http/cookiejar/jar_test.go
index 251f7c16171c3acc40be12a962eb7a48529699f8..93b351889f13c8a9514308ec075667b14004e8fa 100644 (file)
--- 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) {

diff --git a/src/net/http/request.go b/src/net/http/request.go
index 345ba3d4eb43cf8a136881a2e091d2d5c2444bad..bdd18adf3f3a9d0c53ccb211e1ee149fdb6e4704 100644 (file)
--- 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 {