]> Cypherpunks repositories - gostls13.git/commitdiff
http: make Headers be source of truth
authorBrad Fitzpatrick <bradfitz@golang.org>
Thu, 16 Jun 2011 20:02:28 +0000 (13:02 -0700)
committerBrad Fitzpatrick <bradfitz@golang.org>
Thu, 16 Jun 2011 20:02:28 +0000 (13:02 -0700)
Previously Request and Response had redundant fields for
Referer, UserAgent, and cookies which caused confusion and
bugs.  It also didn't allow us to expand the package over
time, since the way to access fields would be in the Headers
one day and promoted to a field the next day.  That would be
hard to gofix, especially with code ranging over Headers.

After a discussion on the mail package's design with a similar
problem, we've designed to make the Headers be the source of
truth and add accessors instead.

Request:
change: Referer -> Referer()
change: UserAgent -> UserAgent()
change: Cookie -> Cookies()
new: Cookie(name) *Cookie
new: AddCookie(*Cookie)

Response:
change: Cookie -> Cookies()

Cookie:
new: String() string

R=rsc
CC=golang-dev
https://golang.org/cl/4620049

18 files changed:
src/cmd/gofix/Makefile
src/cmd/gofix/httpheaders.go [new file with mode: 0644]
src/cmd/gofix/httpheaders_test.go [new file with mode: 0644]
src/cmd/gofix/main.go
src/pkg/http/cgi/child.go
src/pkg/http/cgi/child_test.go
src/pkg/http/cgi/host.go
src/pkg/http/client.go
src/pkg/http/client_test.go
src/pkg/http/cookie.go
src/pkg/http/cookie_test.go
src/pkg/http/readrequest_test.go
src/pkg/http/request.go
src/pkg/http/requestwrite_test.go
src/pkg/http/response.go
src/pkg/http/reverseproxy.go
src/pkg/http/reverseproxy_test.go
src/pkg/http/server.go

index d19de5c4f6aef0e2aee127e8576407ed311d1576..7504ddcbdf7441581afe4e72e488faaf05009ff1 100644 (file)
@@ -11,6 +11,7 @@ GOFILES=\
        main.go\
        osopen.go\
        httpfinalurl.go\
+       httpheaders.go\
        httpserver.go\
        procattr.go\
        reflect.go\
diff --git a/src/cmd/gofix/httpheaders.go b/src/cmd/gofix/httpheaders.go
new file mode 100644 (file)
index 0000000..8a9080e
--- /dev/null
@@ -0,0 +1,66 @@
+// Copyright 2011 The Go Authors.  All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+       "go/ast"
+)
+
+var httpHeadersFix = fix{
+       "httpheaders",
+       httpheaders,
+       `Rename http Referer, UserAgent, Cookie, SetCookie, which are now methods.
+
+http://codereview.appspot.com/4620049/
+`,
+}
+
+func init() {
+       register(httpHeadersFix)
+}
+
+func httpheaders(f *ast.File) bool {
+       if !imports(f, "http") {
+               return false
+       }
+
+       called := make(map[ast.Node]bool)
+       walk(f, func(ni interface{}) {
+               switch n := ni.(type) {
+               case *ast.CallExpr:
+                       called[n.Fun] = true
+               }
+       })
+
+       fixed := false
+       typeof := typecheck(headerTypeConfig, f)
+       walk(f, func(ni interface{}) {
+               switch n := ni.(type) {
+               case *ast.SelectorExpr:
+                       if called[n] {
+                               break
+                       }
+                       if t := typeof[n.X]; t != "*http.Request" && t != "*http.Response" {
+                               break
+                       }
+                       switch n.Sel.Name {
+                       case "Referer", "UserAgent":
+                               n.Sel.Name += "()"
+                               fixed = true
+                       case "Cookie":
+                               n.Sel.Name = "Cookies()"
+                               fixed = true
+                       }
+               }
+       })
+       return fixed
+}
+
+var headerTypeConfig = &TypeConfig{
+       Type: map[string]*Type{
+               "*http.Request":  &Type{},
+               "*http.Response": &Type{},
+       },
+}
diff --git a/src/cmd/gofix/httpheaders_test.go b/src/cmd/gofix/httpheaders_test.go
new file mode 100644 (file)
index 0000000..cc82b58
--- /dev/null
@@ -0,0 +1,73 @@
+// Copyright 2011 The Go Authors.  All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+func init() {
+       addTestCases(httpHeadersTests)
+}
+
+var httpHeadersTests = []testCase{
+       {
+               Name: "httpheaders.0",
+               In: `package headertest
+
+import (
+       "http"
+)
+
+type Other struct {
+       Referer   string
+       UserAgent string
+       Cookie    []*http.Cookie
+}
+
+func f(req *http.Request, res *http.Response, other *Other) {
+       _ = req.Referer
+       _ = req.UserAgent
+       _ = req.Cookie
+
+       _ = res.Cookie
+
+       _ = other.Referer
+       _ = other.UserAgent
+       _ = other.Cookie
+
+       _ = req.Referer()
+       _ = req.UserAgent()
+       _ = req.Cookies()
+       _ = res.Cookies()
+}
+`,
+               Out: `package headertest
+
+import (
+       "http"
+)
+
+type Other struct {
+       Referer   string
+       UserAgent string
+       Cookie    []*http.Cookie
+}
+
+func f(req *http.Request, res *http.Response, other *Other) {
+       _ = req.Referer()
+       _ = req.UserAgent()
+       _ = req.Cookies()
+
+       _ = res.Cookies()
+
+       _ = other.Referer
+       _ = other.UserAgent
+       _ = other.Cookie
+
+       _ = req.Referer()
+       _ = req.UserAgent()
+       _ = req.Cookies()
+       _ = res.Cookies()
+}
+`,
+       },
+}
index 1b091c18aa5e1ba7141880d3504821c1b15e2d2c..05495bc0d822f3f1ee2f9aef66abcfa0bace39eb 100644 (file)
@@ -123,7 +123,7 @@ func processFile(filename string, useStdin bool) os.Error {
        newFile := file
        fixed := false
        for _, fix := range fixes {
-               if allowed != nil && !allowed[fix.desc] {
+               if allowed != nil && !allowed[fix.name] {
                        continue
                }
                if fix.f(newFile) {
index e1ad7ad32211ace01b3c3263d1c129d995e08f6b..8b74d705483625d2f76460cea8bfb49736ff1ec6 100644 (file)
@@ -45,13 +45,6 @@ func envMap(env []string) map[string]string {
        return m
 }
 
-// These environment variables are manually copied into Request
-var skipHeader = map[string]bool{
-       "HTTP_HOST":       true,
-       "HTTP_REFERER":    true,
-       "HTTP_USER_AGENT": true,
-}
-
 // RequestFromMap creates an http.Request from CGI variables.
 // The returned Request's Body field is not populated.
 func RequestFromMap(params map[string]string) (*http.Request, os.Error) {
@@ -73,8 +66,6 @@ func RequestFromMap(params map[string]string) (*http.Request, os.Error) {
        r.Header = http.Header{}
 
        r.Host = params["HTTP_HOST"]
-       r.Referer = params["HTTP_REFERER"]
-       r.UserAgent = params["HTTP_USER_AGENT"]
 
        if lenstr := params["CONTENT_LENGTH"]; lenstr != "" {
                clen, err := strconv.Atoi64(lenstr)
@@ -90,7 +81,7 @@ func RequestFromMap(params map[string]string) (*http.Request, os.Error) {
 
        // Copy "HTTP_FOO_BAR" variables to "Foo-Bar" Headers
        for k, v := range params {
-               if !strings.HasPrefix(k, "HTTP_") || skipHeader[k] {
+               if !strings.HasPrefix(k, "HTTP_") || k == "HTTP_HOST" {
                        continue
                }
                r.Header.Add(strings.Replace(k[5:], "_", "-", -1), v)
index d12947814e1e799f38c9794c7fb3012424ae8e4c..eee043bc90df4a9b9c024f299f9d68a48249b8a3 100644 (file)
@@ -28,23 +28,19 @@ func TestRequest(t *testing.T) {
        if err != nil {
                t.Fatalf("RequestFromMap: %v", err)
        }
-       if g, e := req.UserAgent, "goclient"; e != g {
+       if g, e := req.UserAgent(), "goclient"; e != g {
                t.Errorf("expected UserAgent %q; got %q", e, g)
        }
        if g, e := req.Method, "GET"; e != g {
                t.Errorf("expected Method %q; got %q", e, g)
        }
-       if g, e := req.Header.Get("User-Agent"), ""; e != g {
-               // Tests that we don't put recognized headers in the map
-               t.Errorf("expected User-Agent %q; got %q", e, g)
-       }
        if g, e := req.Header.Get("Content-Type"), "text/xml"; e != g {
                t.Errorf("expected Content-Type %q; got %q", e, g)
        }
        if g, e := req.ContentLength, int64(123); e != g {
                t.Errorf("expected ContentLength %d; got %d", e, g)
        }
-       if g, e := req.Referer, "elsewhere"; e != g {
+       if g, e := req.Referer(), "elsewhere"; e != g {
                t.Errorf("expected Referer %q; got %q", e, g)
        }
        if req.Header == nil {
index 7ab3f9247aa0b56dfb2f04ef04a730d2bd086166..2be3ede774a8deced42a8867053e9e17a86b6184 100644 (file)
@@ -16,7 +16,6 @@ package cgi
 
 import (
        "bufio"
-       "bytes"
        "exec"
        "fmt"
        "http"
@@ -106,20 +105,13 @@ func (h *Handler) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
                env = append(env, "HTTPS=on")
        }
 
-       if len(req.Cookie) > 0 {
-               b := new(bytes.Buffer)
-               for idx, c := range req.Cookie {
-                       if idx > 0 {
-                               b.Write([]byte("; "))
-                       }
-                       fmt.Fprintf(b, "%s=%s", c.Name, c.Value)
-               }
-               env = append(env, "HTTP_COOKIE="+b.String())
-       }
-
        for k, v := range req.Header {
                k = strings.Map(upperCaseAndUnderscore, k)
-               env = append(env, "HTTP_"+k+"="+strings.Join(v, ", "))
+               joinStr := ", "
+               if k == "COOKIE" {
+                       joinStr = "; "
+               }
+               env = append(env, "HTTP_"+k+"="+strings.Join(v, joinStr))
        }
 
        if req.ContentLength > 0 {
index 71b0370422e53adbfc515733ef070daf80cff016..9478cfae1db24d2cb98adc10d99714bd6cb4f636 100644 (file)
@@ -173,7 +173,7 @@ func (c *Client) doFollowingRedirects(ireq *Request) (r *Response, err os.Error)
                                // Add the Referer header.
                                lastReq := via[len(via)-1]
                                if lastReq.URL.Scheme != "https" {
-                                       req.Referer = lastReq.URL.String()
+                                       req.Header.Set("Referer", lastReq.URL.String())
                                }
 
                                err = redirectChecker(req, via)
index 9ef81d9d4f679ccfb4e408e422a71c6af7950523..d6a9dec3513b6ea0d78da94496460c3fdadbda25 100644 (file)
@@ -149,7 +149,7 @@ func TestRedirects(t *testing.T) {
                n, _ := strconv.Atoi(r.FormValue("n"))
                // Test Referer header. (7 is arbitrary position to test at)
                if n == 7 {
-                       if g, e := r.Referer, ts.URL+"/?n=6"; e != g {
+                       if g, e := r.Referer(), ts.URL+"/?n=6"; e != g {
                                t.Errorf("on request ?n=7, expected referer of %q; got %q", e, g)
                        }
                }
index eb61a7001ef4805c2bafd6f833578f6b2f1528bc..29c4ea7e9a1bc4aca67b25f67eba4701e6048ced 100644 (file)
@@ -7,9 +7,6 @@ package http
 import (
        "bytes"
        "fmt"
-       "io"
-       "os"
-       "sort"
        "strconv"
        "strings"
        "time"
@@ -40,11 +37,9 @@ type Cookie struct {
 }
 
 // readSetCookies parses all "Set-Cookie" values from
-// the header h, removes the successfully parsed values from the 
-// "Set-Cookie" key in h and returns the parsed Cookies.
+// the header h and returns the successfully parsed Cookies.
 func readSetCookies(h Header) []*Cookie {
        cookies := []*Cookie{}
-       var unparsedLines []string
        for _, line := range h["Set-Cookie"] {
                parts := strings.Split(strings.TrimSpace(line), ";", -1)
                if len(parts) == 1 && parts[0] == "" {
@@ -53,17 +48,14 @@ func readSetCookies(h Header) []*Cookie {
                parts[0] = strings.TrimSpace(parts[0])
                j := strings.Index(parts[0], "=")
                if j < 0 {
-                       unparsedLines = append(unparsedLines, line)
                        continue
                }
                name, value := parts[0][:j], parts[0][j+1:]
                if !isCookieNameValid(name) {
-                       unparsedLines = append(unparsedLines, line)
                        continue
                }
                value, success := parseCookieValue(value)
                if !success {
-                       unparsedLines = append(unparsedLines, line)
                        continue
                }
                c := &Cookie{
@@ -134,75 +126,54 @@ func readSetCookies(h Header) []*Cookie {
                }
                cookies = append(cookies, c)
        }
-       h["Set-Cookie"] = unparsedLines, unparsedLines != nil
        return cookies
 }
 
 // SetCookie adds a Set-Cookie header to the provided ResponseWriter's headers.
 func SetCookie(w ResponseWriter, cookie *Cookie) {
-       var b bytes.Buffer
-       writeSetCookieToBuffer(&b, cookie)
-       w.Header().Add("Set-Cookie", b.String())
+       w.Header().Add("Set-Cookie", cookie.String())
 }
 
-func writeSetCookieToBuffer(buf *bytes.Buffer, c *Cookie) {
-       fmt.Fprintf(buf, "%s=%s", sanitizeName(c.Name), sanitizeValue(c.Value))
+// String returns the serialization of the cookie for use in a Cookie
+// header (if only Name and Value are set) or a Set-Cookie response
+// header (if other fields are set).
+func (c *Cookie) String() string {
+       var b bytes.Buffer
+       fmt.Fprintf(&b, "%s=%s", sanitizeName(c.Name), sanitizeValue(c.Value))
        if len(c.Path) > 0 {
-               fmt.Fprintf(buf, "; Path=%s", sanitizeValue(c.Path))
+               fmt.Fprintf(&b, "; Path=%s", sanitizeValue(c.Path))
        }
        if len(c.Domain) > 0 {
-               fmt.Fprintf(buf, "; Domain=%s", sanitizeValue(c.Domain))
+               fmt.Fprintf(&b, "; Domain=%s", sanitizeValue(c.Domain))
        }
        if len(c.Expires.Zone) > 0 {
-               fmt.Fprintf(buf, "; Expires=%s", c.Expires.Format(time.RFC1123))
+               fmt.Fprintf(&b, "; Expires=%s", c.Expires.Format(time.RFC1123))
        }
        if c.MaxAge > 0 {
-               fmt.Fprintf(buf, "; Max-Age=%d", c.MaxAge)
+               fmt.Fprintf(&b, "; Max-Age=%d", c.MaxAge)
        } else if c.MaxAge < 0 {
-               fmt.Fprintf(buf, "; Max-Age=0")
+               fmt.Fprintf(&b, "; Max-Age=0")
        }
        if c.HttpOnly {
-               fmt.Fprintf(buf, "; HttpOnly")
+               fmt.Fprintf(&b, "; HttpOnly")
        }
        if c.Secure {
-               fmt.Fprintf(buf, "; Secure")
-       }
-}
-
-// writeSetCookies writes the wire representation of the set-cookies
-// to w. Each cookie is written on a separate "Set-Cookie: " line.
-// This choice is made because HTTP parsers tend to have a limit on
-// line-length, so it seems safer to place cookies on separate lines.
-func writeSetCookies(w io.Writer, kk []*Cookie) os.Error {
-       if kk == nil {
-               return nil
+               fmt.Fprintf(&b, "; Secure")
        }
-       lines := make([]string, 0, len(kk))
-       var b bytes.Buffer
-       for _, c := range kk {
-               b.Reset()
-               writeSetCookieToBuffer(&b, c)
-               lines = append(lines, "Set-Cookie: "+b.String()+"\r\n")
-       }
-       sort.SortStrings(lines)
-       for _, l := range lines {
-               if _, err := io.WriteString(w, l); err != nil {
-                       return err
-               }
-       }
-       return nil
+       return b.String()
 }
 
-// readCookies parses all "Cookie" values from
-// the header h, removes the successfully parsed values from the 
-// "Cookie" key in h and returns the parsed Cookies.
-func readCookies(h Header) []*Cookie {
+// readCookies parses all "Cookie" values from the header h and
+// returns the successfully parsed Cookies.
+//
+// if filter isn't empty, only cookies of that name are returned
+func readCookies(h Header, filter string) []*Cookie {
        cookies := []*Cookie{}
        lines, ok := h["Cookie"]
        if !ok {
                return cookies
        }
-       unparsedLines := []string{}
+Lines:
        for _, line := range lines {
                parts := strings.Split(strings.TrimSpace(line), ";", -1)
                if len(parts) == 1 && parts[0] == "" {
@@ -215,50 +186,27 @@ func readCookies(h Header) []*Cookie {
                        if len(parts[i]) == 0 {
                                continue
                        }
-                       attr, val := parts[i], ""
-                       if j := strings.Index(attr, "="); j >= 0 {
-                               attr, val = attr[:j], attr[j+1:]
+                       name, val := parts[i], ""
+                       if j := strings.Index(name, "="); j >= 0 {
+                               name, val = name[:j], name[j+1:]
                        }
-                       if !isCookieNameValid(attr) {
+                       if !isCookieNameValid(name) {
                                continue
                        }
+                       if filter != "" && filter != name {
+                               continue Lines
+                       }
                        val, success := parseCookieValue(val)
                        if !success {
                                continue
                        }
-                       cookies = append(cookies, &Cookie{Name: attr, Value: val})
+                       cookies = append(cookies, &Cookie{Name: name, Value: val})
                        parsedPairs++
                }
-               if parsedPairs == 0 {
-                       unparsedLines = append(unparsedLines, line)
-               }
        }
-       h["Cookie"] = unparsedLines, len(unparsedLines) > 0
        return cookies
 }
 
-// writeCookies writes the wire representation of the cookies to
-// w. According to RFC 6265 section 5.4, writeCookies does not
-// attach more than one Cookie header field.  That means all
-// cookies, if any, are written into the same line, separated by
-// semicolon.
-func writeCookies(w io.Writer, kk []*Cookie) os.Error {
-       if len(kk) == 0 {
-               return nil
-       }
-       var buf bytes.Buffer
-       fmt.Fprintf(&buf, "Cookie: ")
-       for i, c := range kk {
-               if i > 0 {
-                       fmt.Fprintf(&buf, "; ")
-               }
-               fmt.Fprintf(&buf, "%s=%s", sanitizeName(c.Name), sanitizeValue(c.Value))
-       }
-       fmt.Fprintf(&buf, "\r\n")
-       _, err := w.Write(buf.Bytes())
-       return err
-}
-
 func sanitizeName(n string) string {
        n = strings.Replace(n, "\n", "-", -1)
        n = strings.Replace(n, "\r", "-", -1)
index 02e42226bd72dc3af28b378712433d67ec780293..9aad167e613a7f8a827a6ff94f35eddb73a6a78a 100644 (file)
@@ -5,7 +5,6 @@
 package http
 
 import (
-       "bytes"
        "fmt"
        "json"
        "os"
@@ -15,30 +14,31 @@ import (
 )
 
 var writeSetCookiesTests = []struct {
-       Cookies []*Cookie
-       Raw     string
+       Cookie *Cookie
+       Raw    string
 }{
        {
-               []*Cookie{
-                       &Cookie{Name: "cookie-1", Value: "v$1"},
-                       &Cookie{Name: "cookie-2", Value: "two", MaxAge: 3600},
-                       &Cookie{Name: "cookie-3", Value: "three", Domain: ".example.com"},
-                       &Cookie{Name: "cookie-4", Value: "four", Path: "/restricted/"},
-               },
-               "Set-Cookie: cookie-1=v$1\r\n" +
-                       "Set-Cookie: cookie-2=two; Max-Age=3600\r\n" +
-                       "Set-Cookie: cookie-3=three; Domain=.example.com\r\n" +
-                       "Set-Cookie: cookie-4=four; Path=/restricted/\r\n",
+               &Cookie{Name: "cookie-1", Value: "v$1"},
+               "cookie-1=v$1",
+       },
+       {
+               &Cookie{Name: "cookie-2", Value: "two", MaxAge: 3600},
+               "cookie-2=two; Max-Age=3600",
+       },
+       {
+               &Cookie{Name: "cookie-3", Value: "three", Domain: ".example.com"},
+               "cookie-3=three; Domain=.example.com",
+       },
+       {
+               &Cookie{Name: "cookie-4", Value: "four", Path: "/restricted/"},
+               "cookie-4=four; Path=/restricted/",
        },
 }
 
 func TestWriteSetCookies(t *testing.T) {
        for i, tt := range writeSetCookiesTests {
-               var w bytes.Buffer
-               writeSetCookies(&w, tt.Cookies)
-               seen := string(w.Bytes())
-               if seen != tt.Raw {
-                       t.Errorf("Test %d, expecting:\n%s\nGot:\n%s\n", i, tt.Raw, seen)
+               if g, e := tt.Cookie.String(), tt.Raw; g != e {
+                       t.Errorf("Test %d, expecting:\n%s\nGot:\n%s\n", i, e, g)
                        continue
                }
        }
@@ -73,7 +73,7 @@ func TestSetCookie(t *testing.T) {
        }
 }
 
-var writeCookiesTests = []struct {
+var addCookieTests = []struct {
        Cookies []*Cookie
        Raw     string
 }{
@@ -83,7 +83,7 @@ var writeCookiesTests = []struct {
        },
        {
                []*Cookie{&Cookie{Name: "cookie-1", Value: "v$1"}},
-               "Cookie: cookie-1=v$1\r\n",
+               "cookie-1=v$1",
        },
        {
                []*Cookie{
@@ -91,17 +91,18 @@ var writeCookiesTests = []struct {
                        &Cookie{Name: "cookie-2", Value: "v$2"},
                        &Cookie{Name: "cookie-3", Value: "v$3"},
                },
-               "Cookie: cookie-1=v$1; cookie-2=v$2; cookie-3=v$3\r\n",
+               "cookie-1=v$1; cookie-2=v$2; cookie-3=v$3",
        },
 }
 
-func TestWriteCookies(t *testing.T) {
-       for i, tt := range writeCookiesTests {
-               var w bytes.Buffer
-               writeCookies(&w, tt.Cookies)
-               seen := string(w.Bytes())
-               if seen != tt.Raw {
-                       t.Errorf("Test %d, expecting:\n%s\nGot:\n%s\n", i, tt.Raw, seen)
+func TestAddCookie(t *testing.T) {
+       for i, tt := range addCookieTests {
+               req, _ := NewRequest("GET", "http://example.com/", nil)
+               for _, c := range tt.Cookies {
+                       req.AddCookie(c)
+               }
+               if g := req.Header.Get("Cookie"); g != tt.Raw {
+                       t.Errorf("Test %d:\nwant: %s\n got: %s\n", i, tt.Raw, g)
                        continue
                }
        }
@@ -140,30 +141,46 @@ func toJSON(v interface{}) string {
 
 func TestReadSetCookies(t *testing.T) {
        for i, tt := range readSetCookiesTests {
-               c := readSetCookies(tt.Header)
-               if !reflect.DeepEqual(c, tt.Cookies) {
-                       t.Errorf("#%d readSetCookies: have\n%s\nwant\n%s\n", i, toJSON(c), toJSON(tt.Cookies))
-                       continue
+               for n := 0; n < 2; n++ { // to verify readSetCookies doesn't mutate its input
+                       c := readSetCookies(tt.Header)
+                       if !reflect.DeepEqual(c, tt.Cookies) {
+                               t.Errorf("#%d readSetCookies: have\n%s\nwant\n%s\n", i, toJSON(c), toJSON(tt.Cookies))
+                               continue
+                       }
                }
        }
 }
 
 var readCookiesTests = []struct {
        Header  Header
+       Filter  string
        Cookies []*Cookie
 }{
        {
-               Header{"Cookie": {"Cookie-1=v$1"}},
-               []*Cookie{&Cookie{Name: "Cookie-1", Value: "v$1"}},
+               Header{"Cookie": {"Cookie-1=v$1", "c2=v2"}},
+               "",
+               []*Cookie{
+                       &Cookie{Name: "Cookie-1", Value: "v$1"},
+                       &Cookie{Name: "c2", Value: "v2"},
+               },
+       },
+       {
+               Header{"Cookie": {"Cookie-1=v$1", "c2=v2"}},
+               "c2",
+               []*Cookie{
+                       &Cookie{Name: "c2", Value: "v2"},
+               },
        },
 }
 
 func TestReadCookies(t *testing.T) {
        for i, tt := range readCookiesTests {
-               c := readCookies(tt.Header)
-               if !reflect.DeepEqual(c, tt.Cookies) {
-                       t.Errorf("#%d readCookies: have\n%s\nwant\n%s\n", i, toJSON(c), toJSON(tt.Cookies))
-                       continue
+               for n := 0; n < 2; n++ { // to verify readCookies doesn't mutate its input                                                  
+                       c := readCookies(tt.Header, tt.Filter)
+                       if !reflect.DeepEqual(c, tt.Cookies) {
+                               t.Errorf("#%d readCookies: have\n%s\nwant\n%s\n", i, toJSON(c), toJSON(tt.Cookies))
+                               continue
+                       }
                }
        }
 }
index d93e573f58bd4fad4dac0887ecabfc5b3a6a1d10..0b92b7942698c98734178c948685c38174398069 100644 (file)
@@ -58,12 +58,11 @@ var reqTests = []reqTest{
                                "Keep-Alive":       {"300"},
                                "Proxy-Connection": {"keep-alive"},
                                "Content-Length":   {"7"},
+                               "User-Agent":       {"Fake"},
                        },
                        Close:         false,
                        ContentLength: 7,
                        Host:          "www.techcrunch.com",
-                       Referer:       "",
-                       UserAgent:     "Fake",
                        Form:          Values{},
                },
 
@@ -97,8 +96,6 @@ var reqTests = []reqTest{
                        Close:         false,
                        ContentLength: -1,
                        Host:          "test",
-                       Referer:       "",
-                       UserAgent:     "",
                        Form:          Values{},
                },
 
index 9ed051b13bcf12d7338a5da43e06cc58b67b7b50..2845f17799e716e99da22ab673df9a724af25806 100644 (file)
@@ -60,10 +60,10 @@ type badStringError struct {
 
 func (e *badStringError) String() string { return fmt.Sprintf("%s %q", e.what, e.str) }
 
-var reqExcludeHeader = map[string]bool{
+// Headers that Request.Write handles itself and should be skipped.
+var reqWriteExcludeHeader = map[string]bool{
        "Host":              true,
        "User-Agent":        true,
-       "Referer":           true,
        "Content-Length":    true,
        "Transfer-Encoding": true,
        "Trailer":           true,
@@ -102,9 +102,6 @@ type Request struct {
        // following a hyphen uppercase and the rest lowercase.
        Header Header
 
-       // Cookie records the HTTP cookies sent with the request.
-       Cookie []*Cookie
-
        // The message body.
        Body io.ReadCloser
 
@@ -125,21 +122,6 @@ type Request struct {
        // or the host name given in the URL itself.
        Host string
 
-       // The referring URL, if sent in the request.
-       //
-       // Referer is misspelled as in the request itself,
-       // a mistake from the earliest days of HTTP.
-       // This value can also be fetched from the Header map
-       // as Header["Referer"]; the benefit of making it
-       // available as a structure field is that the compiler
-       // can diagnose programs that use the alternate
-       // (correct English) spelling req.Referrer but cannot
-       // diagnose programs that use Header["Referrer"].
-       Referer string
-
-       // The User-Agent: header string, if sent in the request.
-       UserAgent string
-
        // The parsed form. Only available after ParseForm is called.
        Form Values
 
@@ -176,6 +158,52 @@ func (r *Request) ProtoAtLeast(major, minor int) bool {
                r.ProtoMajor == major && r.ProtoMinor >= minor
 }
 
+// UserAgent returns the client's User-Agent, if sent in the request.
+func (r *Request) UserAgent() string {
+       return r.Header.Get("User-Agent")
+}
+
+// Cookies parses and returns the HTTP cookies sent with the request.
+func (r *Request) Cookies() []*Cookie {
+       return readCookies(r.Header, "")
+}
+
+var ErrNoCookie = os.NewError("http: named cookied not present")
+
+// Cookie returns the named cookie provided in the request or
+// ErrNoCookie if not found.
+func (r *Request) Cookie(name string) (*Cookie, os.Error) {
+       for _, c := range readCookies(r.Header, name) {
+               return c, nil
+       }
+       return nil, ErrNoCookie
+}
+
+// AddCookie adds a cookie to the request.  Per RFC 6265 section 5.4,
+// AddCookie does not attach more than one Cookie header field.  That
+// means all cookies, if any, are written into the same line,
+// separated by semicolon.
+func (r *Request) AddCookie(c *Cookie) {
+       s := fmt.Sprintf("%s=%s", sanitizeName(c.Name), sanitizeValue(c.Value))
+       if c := r.Header.Get("Cookie"); c != "" {
+               r.Header.Set("Cookie", c+"; "+s)
+       } else {
+               r.Header.Set("Cookie", s)
+       }
+}
+
+// Referer returns the referring URL, if sent in the request.
+//
+// Referer is misspelled as in the request itself, a mistake from the
+// earliest days of HTTP.  This value can also be fetched from the
+// Header map as Header["Referer"]; the benefit of making it available
+// as a method is that the compiler can diagnose programs that use the
+// alternate (correct English) spelling req.Referrer() but cannot
+// diagnose programs that use Header["Referrer"].
+func (r *Request) Referer() string {
+       return r.Header.Get("Referer")
+}
+
 // multipartByReader is a sentinel value.
 // Its presence in Request.MultipartForm indicates that parsing of the request
 // body has been handed off to a MultipartReader instead of ParseMultipartFrom.
@@ -230,10 +258,7 @@ const defaultUserAgent = "Go http package"
 //     Host
 //     RawURL, if non-empty, or else URL
 //     Method (defaults to "GET")
-//     UserAgent (defaults to defaultUserAgent)
-//     Referer
-//     Header (only keys not already in this list)
-//     Cookie
+//     Header
 //     ContentLength
 //     TransferEncoding
 //     Body
@@ -281,9 +306,17 @@ func (req *Request) write(w io.Writer, usingProxy bool) os.Error {
 
        // Header lines
        fmt.Fprintf(w, "Host: %s\r\n", host)
-       fmt.Fprintf(w, "User-Agent: %s\r\n", valueOrDefault(req.UserAgent, defaultUserAgent))
-       if req.Referer != "" {
-               fmt.Fprintf(w, "Referer: %s\r\n", req.Referer)
+
+       // Use the defaultUserAgent unless the Header contains one, which
+       // may be blank to not send the header.
+       userAgent := defaultUserAgent
+       if req.Header != nil {
+               if ua := req.Header["User-Agent"]; len(ua) > 0 {
+                       userAgent = ua[0]
+               }
+       }
+       if userAgent != "" {
+               fmt.Fprintf(w, "User-Agent: %s\r\n", userAgent)
        }
 
        // Process Body,ContentLength,Close,Trailer
@@ -297,21 +330,11 @@ func (req *Request) write(w io.Writer, usingProxy bool) os.Error {
        }
 
        // TODO: split long values?  (If so, should share code with Conn.Write)
-       // TODO: if Header includes values for Host, User-Agent, or Referer, this
-       // may conflict with the User-Agent or Referer headers we add manually.
-       // One solution would be to remove the Host, UserAgent, and Referer fields
-       // from Request, and introduce Request methods along the lines of
-       // Response.{GetHeader,AddHeader} and string constants for "Host",
-       // "User-Agent" and "Referer".
-       err = req.Header.WriteSubset(w, reqExcludeHeader)
+       err = req.Header.WriteSubset(w, reqWriteExcludeHeader)
        if err != nil {
                return err
        }
 
-       if err = writeCookies(w, req.Cookie); err != nil {
-               return err
-       }
-
        io.WriteString(w, "\r\n")
 
        // Write body and trailer
@@ -559,13 +582,6 @@ func ReadRequest(b *bufio.Reader) (req *Request, err os.Error) {
 
        fixPragmaCacheControl(req.Header)
 
-       // Pull out useful fields as a convenience to clients.
-       req.Referer = req.Header.Get("Referer")
-       req.Header.Del("Referer")
-
-       req.UserAgent = req.Header.Get("User-Agent")
-       req.Header.Del("User-Agent")
-
        // TODO: Parse specific header values:
        //      Accept
        //      Accept-Encoding
@@ -597,8 +613,6 @@ func ReadRequest(b *bufio.Reader) (req *Request, err os.Error) {
                return nil, err
        }
 
-       req.Cookie = readCookies(req.Header)
-
        return req, nil
 }
 
index 98fbcf459bef36f81752302617a108667d96184e..43ad5252d3bef34c84b310a4be27bcd97b9bf77c 100644 (file)
@@ -47,13 +47,12 @@ var reqWriteTests = []reqWriteTest{
                                "Accept-Language":  {"en-us,en;q=0.5"},
                                "Keep-Alive":       {"300"},
                                "Proxy-Connection": {"keep-alive"},
+                               "User-Agent":       {"Fake"},
                        },
-                       Body:      nil,
-                       Close:     false,
-                       Host:      "www.techcrunch.com",
-                       Referer:   "",
-                       UserAgent: "Fake",
-                       Form:      map[string][]string{},
+                       Body:  nil,
+                       Close: false,
+                       Host:  "www.techcrunch.com",
+                       Form:  map[string][]string{},
                },
 
                nil,
@@ -233,6 +232,9 @@ func TestRequestWrite(t *testing.T) {
                if tt.Body != nil {
                        tt.Req.Body = ioutil.NopCloser(bytes.NewBuffer(tt.Body))
                }
+               if tt.Req.Header == nil {
+                       tt.Req.Header = make(Header)
+               }
                var braw bytes.Buffer
                err := tt.Req.Write(&braw)
                if err != nil {
index 42e60c1f6701a072928f80952a890eace086c118..6c0c441a9446be09a71ca4643c5555792f6ff977 100644 (file)
@@ -40,9 +40,6 @@ type Response struct {
        // Keys in the map are canonicalized (see CanonicalHeaderKey).
        Header Header
 
-       // SetCookie records the Set-Cookie requests sent with the response.
-       SetCookie []*Cookie
-
        // Body represents the response body.
        Body io.ReadCloser
 
@@ -71,6 +68,11 @@ type Response struct {
        Request *Request
 }
 
+// Cookies parses and returns the cookies set in the Set-Cookie headers.
+func (r *Response) Cookies() []*Cookie {
+       return readSetCookies(r.Header)
+}
+
 // ReadResponse reads and returns an HTTP response from r.  The
 // req parameter specifies the Request that corresponds to
 // this Response.  Clients must call resp.Body.Close when finished
@@ -127,8 +129,6 @@ func ReadResponse(r *bufio.Reader, req *Request) (resp *Response, err os.Error)
                return nil, err
        }
 
-       resp.SetCookie = readSetCookies(resp.Header)
-
        return resp, nil
 }
 
@@ -200,10 +200,6 @@ func (resp *Response) Write(w io.Writer) os.Error {
                return err
        }
 
-       if err = writeSetCookies(w, resp.SetCookie); err != nil {
-               return err
-       }
-
        // End-of-header
        io.WriteString(w, "\r\n")
 
index 9a9e21599b7adedaf6dfdc85fe5af30b5640991e..e4ce1e34c79b54081237817a5b1949fe21204407 100644 (file)
@@ -92,10 +92,6 @@ func (p *ReverseProxy) ServeHTTP(rw ResponseWriter, req *Request) {
                }
        }
 
-       for _, cookie := range res.SetCookie {
-               SetCookie(rw, cookie)
-       }
-
        rw.WriteHeader(res.StatusCode)
 
        if res.Body != nil {
index d7bcde90d3af84206824691462124c117b8ba1f9..bc08614814e774d3c93564bf76728e9b4e0a8f0c 100644 (file)
@@ -49,10 +49,10 @@ func TestReverseProxy(t *testing.T) {
        if g, e := res.Header.Get("X-Foo"), "bar"; g != e {
                t.Errorf("got X-Foo %q; expected %q", g, e)
        }
-       if g, e := len(res.SetCookie), 1; g != e {
+       if g, e := len(res.Header["Set-Cookie"]), 1; g != e {
                t.Fatalf("got %d SetCookies, want %d", g, e)
        }
-       if cookie := res.SetCookie[0]; cookie.Name != "flavor" {
+       if cookie := res.Cookies()[0]; cookie.Name != "flavor" {
                t.Errorf("unexpected cookie %q", cookie.Name)
        }
        bodyBytes, _ := ioutil.ReadAll(res.Body)
index c697ef0d3e2864b3f9ae65ff8ded1a31018d41fc..2f545f7102ae51ad4bd17135f0503dfed0a6f4bf 100644 (file)
@@ -405,7 +405,7 @@ func errorKludge(w *response) {
 
        // Is it a broken browser?
        var msg string
-       switch agent := w.req.UserAgent; {
+       switch agent := w.req.UserAgent(); {
        case strings.Contains(agent, "MSIE"):
                msg = "Internet Explorer"
        case strings.Contains(agent, "Chrome/"):