net/http: revert back to (and test) Go 1 CheckRedirect behavior

If a Client's CheckRedirect function returns an error, we
again return both a non-nil *Response and a non-nil error.

Fixes #3795

R=golang-dev, n13m3y3r
CC=golang-dev
https://golang.org/cl/6429044

diff --git a/src/pkg/net/http/client.go b/src/pkg/net/http/client.go
index 89441424e1d8829dbeb757a1afbc4d81bdfd12ed..ad06fde0359ec7faca9364855cd00fe12e7429fd 100644 (file)
--- a/src/pkg/net/http/client.go
+++ b/src/pkg/net/http/client.go
@@ -33,10 +33,11 @@ type Client struct {
 
        // CheckRedirect specifies the policy for handling redirects.
        // If CheckRedirect is not nil, the client calls it before
-       // following an HTTP redirect. The arguments req and via
-       // are the upcoming request and the requests made already,
-       // oldest first. If CheckRedirect returns an error, the client
-       // returns that error (wrapped in a url.Error) instead of
+       // following an HTTP redirect. The arguments req and via are
+       // the upcoming request and the requests made already, oldest
+       // first. If CheckRedirect returns an error, the Client's Get
+       // method returns both the previous Response and
+       // CheckRedirect's error (wrapped in a url.Error) instead of
        // issuing the Request req.
        //
        // If CheckRedirect is nil, the Client uses its default policy,
@@ -221,6 +222,7 @@ func (c *Client) doFollowingRedirects(ireq *Request) (resp *Response, err error)
 
        req := ireq
        urlStr := "" // next relative or absolute URL to fetch (after first request)
+       redirectFailed := false
        for redirect := 0; ; redirect++ {
                if redirect != 0 {
                        req = new(Request)
@@ -239,6 +241,7 @@ func (c *Client) doFollowingRedirects(ireq *Request) (resp *Response, err error)
 
                                err = redirectChecker(req, via)
                                if err != nil {
+                                       redirectFailed = true
                                        break
                                }
                        }
@@ -268,16 +271,24 @@ func (c *Client) doFollowingRedirects(ireq *Request) (resp *Response, err error)
                return
        }
 
-       if resp != nil {
-               resp.Body.Close()
-       }
-
        method := ireq.Method
-       return nil, &url.Error{
+       urlErr := &url.Error{
                Op:  method[0:1] + strings.ToLower(method[1:]),
                URL: urlStr,
                Err: err,
        }
+
+       if redirectFailed {
+               // Special case for Go 1 compatibility: return both the response
+               // and an error if the CheckRedirect function failed.
+               // See http://golang.org/issue/3795
+               return resp, urlErr
+       }
+
+       if resp != nil {
+               resp.Body.Close()
+       }
+       return nil, urlErr
 }
 
 func defaultCheckRedirect(req *Request, via []*Request) error {

diff --git a/src/pkg/net/http/client_test.go b/src/pkg/net/http/client_test.go
index fe4b626a31e97c2e5357a74ae3024f284cc0a060..da7a44da7a50857b8b237acca33dbae8933eb8a4 100644 (file)
--- a/src/pkg/net/http/client_test.go
+++ b/src/pkg/net/http/client_test.go
@@ -234,6 +234,12 @@ func TestRedirects(t *testing.T) {
        if urlError, ok := err.(*url.Error); !ok || urlError.Err != checkErr {
                t.Errorf("with redirects forbidden, expected a *url.Error with our 'no redirects allowed' error inside; got %#v (%q)", err, err)
        }
+       if res == nil {
+               t.Fatalf("Expected a non-nil Response on CheckRedirect failure (http://golang.org/issue/3795)")
+       }
+       if res.Header.Get("Location") == "" {
+               t.Errorf("no Location header in Response")
+       }
 }
 
 var expectedCookies = []*Cookie{