]> Cypherpunks repositories - gostls13.git/commitdiff
http: adapt Cookie code to follow IETF draft
authorPetar Maymounkov <petarm@gmail.com>
Mon, 7 Mar 2011 17:08:39 +0000 (12:08 -0500)
committerRuss Cox <rsc@golang.org>
Mon, 7 Mar 2011 17:08:39 +0000 (12:08 -0500)
R=rsc, bradfitzwork
CC=golang-dev
https://golang.org/cl/4235055

src/pkg/http/cookie.go
src/pkg/http/cookie_test.go

index ff75c47c92d2f98df56ab2427bbc0f7d9f041760..2bb66e58e5c4c7c7072e2dce1a728ecc8cbf5506 100644 (file)
@@ -15,65 +15,28 @@ import (
        "time"
 )
 
-// A note on Version=0 vs. Version=1 cookies
+// This implementation is done according to IETF draft-ietf-httpstate-cookie-23, found at
 //
-// The difference between Set-Cookie and Set-Cookie2 is hard to discern from the
-// RFCs as it is not stated explicitly.  There seem to be three standards
-// lingering on the web: Netscape, RFC 2109 (aka Version=0) and RFC 2965 (aka
-// Version=1). It seems that Netscape and RFC 2109 are the same thing, hereafter
-// Version=0 cookies.
-//
-// In general, Set-Cookie2 is a superset of Set-Cookie. It has a few new
-// attributes like HttpOnly and Secure.  To be meticulous, if a server intends
-// to use these, it needs to send a Set-Cookie2.  However, it is most likely
-// most modern browsers will not complain seeing an HttpOnly attribute in a
-// Set-Cookie header.
-//
-// Both RFC 2109 and RFC 2965 use Cookie in the same way - two send cookie
-// values from clients to servers - and the allowable attributes seem to be the
-// same.
-// 
-// The Cookie2 header is used for a different purpose. If a client suspects that
-// the server speaks Version=1 (RFC 2965) then along with the Cookie header
-// lines, you can also send:
-//
-//   Cookie2: $Version="1"
-//
-// in order to suggest to the server that you understand Version=1 cookies. At
-// which point the server may continue responding with Set-Cookie2 headers.
-// When a client sends the (above) Cookie2 header line, it must be prepated to
-// understand incoming Set-Cookie2.
-//
-// This implementation of cookies supports neither Set-Cookie2 nor Cookie2
-// headers. However, it parses Version=1 Cookies (along with Version=0) as well
-// as Set-Cookie headers which utilize the full Set-Cookie2 syntax.
-
-// TODO(petar): Explicitly forbid parsing of Set-Cookie attributes
-// starting with '$', which have been used to hack into broken
-// servers using the eventual Request headers containing those
-// invalid attributes that may overwrite intended $Version, $Path, 
-// etc. attributes.
-// TODO(petar): Read 'Set-Cookie2' headers and prioritize them over equivalent
-// 'Set-Cookie' headers. 'Set-Cookie2' headers are still extremely rare.
+//    http://tools.ietf.org/html/draft-ietf-httpstate-cookie-23
 
-// A Cookie represents an RFC 2965 HTTP cookie as sent in
-// the Set-Cookie header of an HTTP response or the Cookie header
-// of an HTTP request.
-// The Set-Cookie2 and Cookie2 headers are unimplemented.
+// A Cookie represents an HTTP cookie as sent in the Set-Cookie header of an
+// HTTP response or the Cookie header of an HTTP request.
 type Cookie struct {
        Name       string
        Value      string
        Path       string
        Domain     string
-       Comment    string
-       Version    int
        Expires    time.Time
        RawExpires string
-       MaxAge     int // Max age in seconds
-       Secure     bool
-       HttpOnly   bool
-       Raw        string
-       Unparsed   []string // Raw text of unparsed attribute-value pairs
+
+       // MaxAge=0 means no 'Max-Age' attribute specified. 
+       // MaxAge<0 means delete cookie now, equivalently 'Max-Age: 0'
+       // MaxAge>0 means Max-Age attribute present and given in seconds
+       MaxAge   int
+       Secure   bool
+       HttpOnly bool
+       Raw      string
+       Unparsed []string // Raw text of unparsed attribute-value pairs
 }
 
 // readSetCookies parses all "Set-Cookie" values from
@@ -94,16 +57,19 @@ func readSetCookies(h Header) []*Cookie {
                        continue
                }
                name, value := parts[0][:j], parts[0][j+1:]
-               value, err := URLUnescape(value)
-               if err != nil {
+               if !isCookieNameValid(name) {
+                       unparsedLines = append(unparsedLines, line)
+                       continue
+               }
+               value, success := parseCookieValue(value)
+               if !success {
                        unparsedLines = append(unparsedLines, line)
                        continue
                }
                c := &Cookie{
-                       Name:   name,
-                       Value:  value,
-                       MaxAge: -1, // Not specified
-                       Raw:    line,
+                       Name:  name,
+                       Value: value,
+                       Raw:   line,
                }
                for i := 1; i < len(parts); i++ {
                        parts[i] = strings.TrimSpace(parts[i])
@@ -114,11 +80,11 @@ func readSetCookies(h Header) []*Cookie {
                        attr, val := parts[i], ""
                        if j := strings.Index(attr, "="); j >= 0 {
                                attr, val = attr[:j], attr[j+1:]
-                               val, err = URLUnescape(val)
-                               if err != nil {
-                                       c.Unparsed = append(c.Unparsed, parts[i])
-                                       continue
-                               }
+                       }
+                       val, success = parseCookieValue(val)
+                       if !success {
+                               c.Unparsed = append(c.Unparsed, parts[i])
+                               continue
                        }
                        switch strings.ToLower(attr) {
                        case "secure":
@@ -127,19 +93,20 @@ func readSetCookies(h Header) []*Cookie {
                        case "httponly":
                                c.HttpOnly = true
                                continue
-                       case "comment":
-                               c.Comment = val
-                               continue
                        case "domain":
                                c.Domain = val
                                // TODO: Add domain parsing
                                continue
                        case "max-age":
                                secs, err := strconv.Atoi(val)
-                               if err != nil || secs < 0 {
+                               if err != nil || secs < 0 || secs != 0 && val[0] == '0' {
                                        break
                                }
-                               c.MaxAge = secs
+                               if secs <= 0 {
+                                       c.MaxAge = -1
+                               } else {
+                                       c.MaxAge = secs
+                               }
                                continue
                        case "expires":
                                c.RawExpires = val
@@ -154,13 +121,6 @@ func readSetCookies(h Header) []*Cookie {
                                c.Path = val
                                // TODO: Add path parsing
                                continue
-                       case "version":
-                               c.Version, err = strconv.Atoi(val)
-                               if err != nil {
-                                       c.Version = 0
-                                       break
-                               }
-                               continue
                        }
                        c.Unparsed = append(c.Unparsed, parts[i])
                }
@@ -182,11 +142,7 @@ func writeSetCookies(w io.Writer, kk []*Cookie) os.Error {
        var b bytes.Buffer
        for _, c := range kk {
                b.Reset()
-               // TODO(petar): c.Value (below) should be unquoted if it is recognized as quoted
-               fmt.Fprintf(&b, "%s=%s", CanonicalHeaderKey(c.Name), c.Value)
-               if c.Version > 0 {
-                       fmt.Fprintf(&b, "Version=%d; ", c.Version)
-               }
+               fmt.Fprintf(&b, "%s=%s", c.Name, c.Value)
                if len(c.Path) > 0 {
                        fmt.Fprintf(&b, "; Path=%s", URLEscape(c.Path))
                }
@@ -196,8 +152,10 @@ func writeSetCookies(w io.Writer, kk []*Cookie) os.Error {
                if len(c.Expires.Zone) > 0 {
                        fmt.Fprintf(&b, "; Expires=%s", c.Expires.Format(time.RFC1123))
                }
-               if c.MaxAge >= 0 {
+               if c.MaxAge > 0 {
                        fmt.Fprintf(&b, "; Max-Age=%d", c.MaxAge)
+               } else if c.MaxAge < 0 {
+                       fmt.Fprintf(&b, "; Max-Age=0")
                }
                if c.HttpOnly {
                        fmt.Fprintf(&b, "; HttpOnly")
@@ -205,9 +163,6 @@ func writeSetCookies(w io.Writer, kk []*Cookie) os.Error {
                if c.Secure {
                        fmt.Fprintf(&b, "; Secure")
                }
-               if len(c.Comment) > 0 {
-                       fmt.Fprintf(&b, "; Comment=%s", URLEscape(c.Comment))
-               }
                lines = append(lines, "Set-Cookie: "+b.String()+"\r\n")
        }
        sort.SortStrings(lines)
@@ -235,63 +190,29 @@ func readCookies(h Header) []*Cookie {
                        continue
                }
                // Per-line attributes
-               var lineCookies = make(map[string]string)
-               var version int
-               var path string
-               var domain string
-               var comment string
-               var httponly bool
+               parsedPairs := 0
                for i := 0; i < len(parts); i++ {
                        parts[i] = strings.TrimSpace(parts[i])
                        if len(parts[i]) == 0 {
                                continue
                        }
                        attr, val := parts[i], ""
-                       var err os.Error
                        if j := strings.Index(attr, "="); j >= 0 {
                                attr, val = attr[:j], attr[j+1:]
-                               val, err = URLUnescape(val)
-                               if err != nil {
-                                       continue
-                               }
                        }
-                       switch strings.ToLower(attr) {
-                       case "$httponly":
-                               httponly = true
-                       case "$version":
-                               version, err = strconv.Atoi(val)
-                               if err != nil {
-                                       version = 0
-                                       continue
-                               }
-                       case "$domain":
-                               domain = val
-                               // TODO: Add domain parsing
-                       case "$path":
-                               path = val
-                               // TODO: Add path parsing
-                       case "$comment":
-                               comment = val
-                       default:
-                               lineCookies[attr] = val
+                       if !isCookieNameValid(attr) {
+                               continue
+                       }
+                       val, success := parseCookieValue(val)
+                       if !success {
+                               continue
                        }
+                       cookies = append(cookies, &Cookie{Name: attr, Value: val})
+                       parsedPairs++
                }
-               if len(lineCookies) == 0 {
+               if parsedPairs == 0 {
                        unparsedLines = append(unparsedLines, line)
                }
-               for n, v := range lineCookies {
-                       cookies = append(cookies, &Cookie{
-                               Name:     n,
-                               Value:    v,
-                               Path:     path,
-                               Domain:   domain,
-                               Comment:  comment,
-                               Version:  version,
-                               HttpOnly: httponly,
-                               MaxAge:   -1,
-                               Raw:      line,
-                       })
-               }
        }
        h["Cookie"] = unparsedLines, len(unparsedLines) > 0
        return cookies
@@ -303,28 +224,8 @@ func readCookies(h Header) []*Cookie {
 // line-length, so it seems safer to place cookies on separate lines.
 func writeCookies(w io.Writer, kk []*Cookie) os.Error {
        lines := make([]string, 0, len(kk))
-       var b bytes.Buffer
        for _, c := range kk {
-               b.Reset()
-               n := c.Name
-               if c.Version > 0 {
-                       fmt.Fprintf(&b, "$Version=%d; ", c.Version)
-               }
-               // TODO(petar): c.Value (below) should be unquoted if it is recognized as quoted
-               fmt.Fprintf(&b, "%s=%s", CanonicalHeaderKey(n), c.Value)
-               if len(c.Path) > 0 {
-                       fmt.Fprintf(&b, "; $Path=%s", URLEscape(c.Path))
-               }
-               if len(c.Domain) > 0 {
-                       fmt.Fprintf(&b, "; $Domain=%s", URLEscape(c.Domain))
-               }
-               if c.HttpOnly {
-                       fmt.Fprintf(&b, "; $HttpOnly")
-               }
-               if len(c.Comment) > 0 {
-                       fmt.Fprintf(&b, "; $Comment=%s", URLEscape(c.Comment))
-               }
-               lines = append(lines, "Cookie: "+b.String()+"\r\n")
+               lines = append(lines, fmt.Sprintf("Cookie: %s=%s\r\n", c.Name, c.Value))
        }
        sort.SortStrings(lines)
        for _, l := range lines {
@@ -334,3 +235,38 @@ func writeCookies(w io.Writer, kk []*Cookie) os.Error {
        }
        return nil
 }
+
+func unquoteCookieValue(v string) string {
+       if len(v) > 1 && v[0] == '"' && v[len(v)-1] == '"' {
+               return v[1 : len(v)-1]
+       }
+       return v
+}
+
+func isCookieByte(c byte) bool {
+       switch true {
+       case c == 0x21, 0x23 <= c && c <= 0x2b, 0x2d <= c && c <= 0x3a,
+               0x3c <= c && c <= 0x5b, 0x5d <= c && c <= 0x7e:
+               return true
+       }
+       return false
+}
+
+func parseCookieValue(raw string) (string, bool) {
+       raw = unquoteCookieValue(raw)
+       for i := 0; i < len(raw); i++ {
+               if !isCookieByte(raw[i]) {
+                       return "", false
+               }
+       }
+       return raw, true
+}
+
+func isCookieNameValid(raw string) bool {
+       for _, c := range raw {
+               if !isToken(byte(c)) {
+                       return false
+               }
+       }
+       return true
+}
index 363c841bb0538da7f8d791a24874a399613704b0..827f232c0065297acd7cf317aac36e323b1a8daf 100644 (file)
@@ -17,7 +17,7 @@ var writeSetCookiesTests = []struct {
 }{
        {
                []*Cookie{&Cookie{Name: "cookie-1", Value: "v$1", MaxAge: -1}},
-               "Set-Cookie: Cookie-1=v$1\r\n",
+               "Set-Cookie: cookie-1=v$1\r\n",
        },
 }
 
@@ -39,7 +39,7 @@ var writeCookiesTests = []struct {
 }{
        {
                []*Cookie{&Cookie{Name: "cookie-1", Value: "v$1", MaxAge: -1}},
-               "Cookie: Cookie-1=v$1\r\n",
+               "Cookie: cookie-1=v$1\r\n",
        },
 }