net/http: send cookies in jar on redirect

Until a RFC 6265 jar lands, Request.AddCookie() will work incorrectly.

Fixes #2692

R=bradfitz, rsc
CC=golang-dev, r
https://golang.org/cl/5544069

diff --git a/src/pkg/net/http/client.go b/src/pkg/net/http/client.go
index a4f8f19aac84d4662d083616d3c4c89379f34791..1f16fcf86ddb48a2249f73dba7eb3a04c56716fe 100644 (file)
--- a/src/pkg/net/http/client.go
+++ b/src/pkg/net/http/client.go
@@ -213,11 +213,11 @@ func (c *Client) doFollowingRedirects(ireq *Request) (r *Response, err error) {
                                        break
                                }
                        }
-                       for _, cookie := range jar.Cookies(req.URL) {
-                               req.AddCookie(cookie)
-                       }
                }
 
+               for _, cookie := range jar.Cookies(req.URL) {
+                       req.AddCookie(cookie)
+               }
                urlStr = req.URL.String()
                if r, err = send(req, c.Transport); err != nil {
                        break

diff --git a/src/pkg/net/http/client_test.go b/src/pkg/net/http/client_test.go
index 57a9dd9574d1a6c201ed635a22c26a460196d038..c74611011a819f7661d882e2de843628f2ffb1f0 100644 (file)
--- a/src/pkg/net/http/client_test.go
+++ b/src/pkg/net/http/client_test.go
@@ -18,6 +18,7 @@ import (
        "net/url"
        "strconv"
        "strings"
+       "sync"
        "testing"
 )
 
@@ -236,6 +237,92 @@ func TestRedirects(t *testing.T) {
        }
 }
 
+var expectedCookies = []*Cookie{
+       &Cookie{Name: "ChocolateChip", Value: "tasty"},
+       &Cookie{Name: "First", Value: "Hit"},
+       &Cookie{Name: "Second", Value: "Hit"},
+}
+
+var echoCookiesRedirectHandler = HandlerFunc(func(w ResponseWriter, r *Request) {
+       for _, cookie := range r.Cookies() {
+               SetCookie(w, cookie)
+       }
+       if r.URL.Path == "/" {
+               SetCookie(w, expectedCookies[1])
+               Redirect(w, r, "/second", StatusMovedPermanently)
+       } else {
+               SetCookie(w, expectedCookies[2])
+               w.Write([]byte("hello"))
+       }
+})
+
+// Just enough correctness for our redirect tests. Uses the URL.Host as the
+// scope of all cookies.
+type TestJar struct {
+       m      sync.Mutex
+       perURL map[string][]*Cookie
+}
+
+func (j *TestJar) SetCookies(u *url.URL, cookies []*Cookie) {
+       j.m.Lock()
+       defer j.m.Unlock()
+       j.perURL[u.Host] = cookies
+}
+
+func (j *TestJar) Cookies(u *url.URL) []*Cookie {
+       j.m.Lock()
+       defer j.m.Unlock()
+       return j.perURL[u.Host]
+}
+
+func TestRedirectCookiesOnRequest(t *testing.T) {
+       var ts *httptest.Server
+       ts = httptest.NewServer(echoCookiesRedirectHandler)
+       defer ts.Close()
+       c := &Client{}
+       req, _ := NewRequest("GET", ts.URL, nil)
+       req.AddCookie(expectedCookies[0])
+       // TODO: Uncomment when an implementation of a RFC6265 cookie jar lands.
+       _ = c
+       // resp, _ := c.Do(req)
+       // matchReturnedCookies(t, expectedCookies, resp.Cookies())
+
+       req, _ = NewRequest("GET", ts.URL, nil)
+       // resp, _ = c.Do(req)
+       // matchReturnedCookies(t, expectedCookies[1:], resp.Cookies())
+}
+
+func TestRedirectCookiesJar(t *testing.T) {
+       var ts *httptest.Server
+       ts = httptest.NewServer(echoCookiesRedirectHandler)
+       defer ts.Close()
+       c := &Client{}
+       c.Jar = &TestJar{perURL: make(map[string][]*Cookie)}
+       u, _ := url.Parse(ts.URL)
+       c.Jar.SetCookies(u, []*Cookie{expectedCookies[0]})
+       resp, _ := c.Get(ts.URL)
+       matchReturnedCookies(t, expectedCookies, resp.Cookies())
+}
+
+func matchReturnedCookies(t *testing.T, expected, given []*Cookie) {
+       t.Logf("Received cookies: %v", given)
+       if len(given) != len(expected) {
+               t.Errorf("Expected %d cookies, got %d", len(expected), len(given))
+       }
+       for _, ec := range expected {
+               foundC := false
+               for _, c := range given {
+                       if ec.Name == c.Name && ec.Value == c.Value {
+                               foundC = true
+                               break
+                       }
+               }
+               if !foundC {
+                       t.Errorf("Missing cookie %v", ec)
+               }
+       }
+}
+
 func TestStreamingGet(t *testing.T) {
        say := make(chan string)
        ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {