Commit 4918e3a9 authored by Pawel Szczur's avatar Pawel Szczur Committed by Brad Fitzpatrick

net/http/client.go: fix cookie handling on (*Client) Do()

Fix the problem with no cookie handling when sending
other than GET or HEAD request through
(*Client) Do(*Request) (*Resposne, error).
https://code.google.com/p/go/issues/detail?id=3985

Adds a function (*Client) send(*Request) (*Reponse, error):
- sets cookies from CookieJar to request,
- sends request
- parses a reply cookies and updates CookieJar

Fixes #3985

R=bradfitz
CC=gobot, golang-dev
https://golang.org/cl/6653049
parent 8010a430
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
// license that can be found in the LICENSE file. // license that can be found in the LICENSE file.
// HTTP client. See RFC 2616. // HTTP client. See RFC 2616.
// //
// This is the high-level Client interface. // This is the high-level Client interface.
// The low-level implementation is in transport.go. // The low-level implementation is in transport.go.
...@@ -44,8 +44,8 @@ type Client struct { ...@@ -44,8 +44,8 @@ type Client struct {
// which is to stop after 10 consecutive requests. // which is to stop after 10 consecutive requests.
CheckRedirect func(req *Request, via []*Request) error CheckRedirect func(req *Request, via []*Request) error
// Jar specifies the cookie jar. // Jar specifies the cookie jar.
// If Jar is nil, cookies are not sent in requests and ignored // If Jar is nil, cookies are not sent in requests and ignored
// in responses. // in responses.
Jar CookieJar Jar CookieJar
} }
...@@ -87,6 +87,22 @@ type readClose struct { ...@@ -87,6 +87,22 @@ type readClose struct {
io.Closer io.Closer
} }
func (c *Client) send(req *Request) (*Response, error) {
if c.Jar != nil {
for _, cookie := range c.Jar.Cookies(req.URL) {
req.AddCookie(cookie)
}
}
resp, err := send(req, c.Transport)
if err != nil {
return nil, err
}
if c.Jar != nil {
c.Jar.SetCookies(req.URL, resp.Cookies())
}
return resp, err
}
// Do sends an HTTP request and returns an HTTP response, following // Do sends an HTTP request and returns an HTTP response, following
// policy (e.g. redirects, cookies, auth) as configured on the client. // policy (e.g. redirects, cookies, auth) as configured on the client.
// //
...@@ -106,7 +122,7 @@ func (c *Client) Do(req *Request) (resp *Response, err error) { ...@@ -106,7 +122,7 @@ func (c *Client) Do(req *Request) (resp *Response, err error) {
if req.Method == "GET" || req.Method == "HEAD" { if req.Method == "GET" || req.Method == "HEAD" {
return c.doFollowingRedirects(req) return c.doFollowingRedirects(req)
} }
return send(req, c.Transport) return c.send(req)
} }
// send issues an HTTP request. // send issues an HTTP request.
...@@ -215,11 +231,6 @@ func (c *Client) doFollowingRedirects(ireq *Request) (resp *Response, err error) ...@@ -215,11 +231,6 @@ func (c *Client) doFollowingRedirects(ireq *Request) (resp *Response, err error)
return nil, errors.New("http: nil Request.URL") return nil, errors.New("http: nil Request.URL")
} }
jar := c.Jar
if jar == nil {
jar = blackHoleJar{}
}
req := ireq req := ireq
urlStr := "" // next relative or absolute URL to fetch (after first request) urlStr := "" // next relative or absolute URL to fetch (after first request)
redirectFailed := false redirectFailed := false
...@@ -247,16 +258,10 @@ func (c *Client) doFollowingRedirects(ireq *Request) (resp *Response, err error) ...@@ -247,16 +258,10 @@ func (c *Client) doFollowingRedirects(ireq *Request) (resp *Response, err error)
} }
} }
for _, cookie := range jar.Cookies(req.URL) {
req.AddCookie(cookie)
}
urlStr = req.URL.String() urlStr = req.URL.String()
if resp, err = send(req, c.Transport); err != nil { if resp, err = c.send(req); err != nil {
break break
} }
if c := resp.Cookies(); len(c) > 0 {
jar.SetCookies(req.URL, c)
}
if shouldRedirect(resp.StatusCode) { if shouldRedirect(resp.StatusCode) {
resp.Body.Close() resp.Body.Close()
...@@ -316,16 +321,7 @@ func (c *Client) Post(url string, bodyType string, body io.Reader) (resp *Respon ...@@ -316,16 +321,7 @@ func (c *Client) Post(url string, bodyType string, body io.Reader) (resp *Respon
return nil, err return nil, err
} }
req.Header.Set("Content-Type", bodyType) req.Header.Set("Content-Type", bodyType)
if c.Jar != nil { return c.send(req)
for _, cookie := range c.Jar.Cookies(req.URL) {
req.AddCookie(cookie)
}
}
resp, err = send(req, c.Transport)
if err == nil && c.Jar != nil {
c.Jar.SetCookies(req.URL, resp.Cookies())
}
return
} }
// PostForm issues a POST to the specified URL, with data's keys and // PostForm issues a POST to the specified URL, with data's keys and
...@@ -339,7 +335,7 @@ func PostForm(url string, data url.Values) (resp *Response, err error) { ...@@ -339,7 +335,7 @@ func PostForm(url string, data url.Values) (resp *Response, err error) {
return DefaultClient.PostForm(url, data) return DefaultClient.PostForm(url, data)
} }
// PostForm issues a POST to the specified URL, // PostForm issues a POST to the specified URL,
// with data's keys and values urlencoded as the request body. // with data's keys and values urlencoded as the request body.
// //
// When err is nil, resp always contains a non-nil resp.Body. // When err is nil, resp always contains a non-nil resp.Body.
......
...@@ -285,6 +285,10 @@ func TestClientSendsCookieFromJar(t *testing.T) { ...@@ -285,6 +285,10 @@ func TestClientSendsCookieFromJar(t *testing.T) {
req, _ := NewRequest("GET", us, nil) req, _ := NewRequest("GET", us, nil)
client.Do(req) // Note: doesn't hit network client.Do(req) // Note: doesn't hit network
matchReturnedCookies(t, expectedCookies, tr.req.Cookies()) matchReturnedCookies(t, expectedCookies, tr.req.Cookies())
req, _ = NewRequest("POST", us, nil)
client.Do(req) // Note: doesn't hit network
matchReturnedCookies(t, expectedCookies, tr.req.Cookies())
} }
// Just enough correctness for our redirect tests. Uses the URL.Host as the // Just enough correctness for our redirect tests. Uses the URL.Host as the
......
...@@ -8,23 +8,18 @@ import ( ...@@ -8,23 +8,18 @@ import (
"net/url" "net/url"
) )
// A CookieJar manages storage and use of cookies in HTTP requests. // A CookieJar manages storage and use of cookies in HTTP requests.
// //
// Implementations of CookieJar must be safe for concurrent use by multiple // Implementations of CookieJar must be safe for concurrent use by multiple
// goroutines. // goroutines.
type CookieJar interface { type CookieJar interface {
// SetCookies handles the receipt of the cookies in a reply for the // SetCookies handles the receipt of the cookies in a reply for the
// given URL. It may or may not choose to save the cookies, depending // given URL. It may or may not choose to save the cookies, depending
// on the jar's policy and implementation. // on the jar's policy and implementation.
SetCookies(u *url.URL, cookies []*Cookie) SetCookies(u *url.URL, cookies []*Cookie)
// Cookies returns the cookies to send in a request for the given URL. // Cookies returns the cookies to send in a request for the given URL.
// It is up to the implementation to honor the standard cookie use // It is up to the implementation to honor the standard cookie use
// restrictions such as in RFC 6265. // restrictions such as in RFC 6265.
Cookies(u *url.URL) []*Cookie Cookies(u *url.URL) []*Cookie
} }
type blackHoleJar struct{}
func (blackHoleJar) SetCookies(u *url.URL, cookies []*Cookie) {}
func (blackHoleJar) Cookies(u *url.URL) []*Cookie { return nil }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment