net/http: move extra redirect logic for 307/308 into redirectBehavior

Follow up of CL https://go-review.googlesource.com/32595.

Change-Id: I2b3ff7e6b2c764bb6bc5e9aa692d0aed79eb5626
Reviewed-on: https://go-review.googlesource.com/32750
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/client.go b/src/net/http/client.go
index 6780e3ee7a7e1392887c11a7d778f9a1dafd2d15..1af33af93778621eec3f2aef65eee9928248b2e1 100644 (file)
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -388,17 +388,38 @@ func (c *Client) checkRedirect(req *Request, via []*Request) error {
 
 // redirectBehavior describes what should happen when the
 // client encounters a 3xx status code from the server
-func redirectBehavior(reqMethod string, serverStatus int) (redirectMethod string, canRedirect bool) {
-       switch serverStatus {
+func redirectBehavior(reqMethod string, resp *Response, via []*Request) (redirectMethod string, shouldRedirect bool) {
+       switch resp.StatusCode {
        case 301, 302, 303:
                redirectMethod = "GET"
-               canRedirect = true
+               shouldRedirect = true
        case 307, 308:
                redirectMethod = reqMethod
-               canRedirect = true
+               shouldRedirect = true
+
+               // Treat 307 and 308 specially, since they're new in
+               // Go 1.8, and they also require re-sending the request body.
+               loc := resp.Header.Get("Location")
+               if loc == "" {
+                       // 308s have been observed in the wild being served
+                       // without Location headers. Since Go 1.7 and earlier
+                       // didn't follow these codes, just stop here instead
+                       // of returning an error.
+                       // See Issue 17773.
+                       shouldRedirect = false
+                       break
+               }
+               ireq := via[0]
+               if ireq.GetBody == nil && ireq.outgoingLength() != 0 {
+                       // We had a request body, and 307/308 require
+                       // re-sending it, but GetBody is not defined. So just
+                       // return this response to the user instead of an
+                       // error, like we did in Go 1.7 and earlier.
+                       shouldRedirect = false
+               }
        }
 
-       return redirectMethod, canRedirect
+       return redirectMethod, shouldRedirect
 }
 
 // Do sends an HTTP request and returns an HTTP response, following
@@ -536,35 +557,7 @@ func (c *Client) Do(req *Request) (*Response, error) {
                }
 
                var shouldRedirect bool
-               redirectMethod, shouldRedirect = redirectBehavior(req.Method, resp.StatusCode)
-
-               // Treat 307 and 308 specially, since they're new in
-               // Go 1.8, and they also require re-sending the
-               // request body.
-               //
-               // TODO: move this logic into func redirectBehavior?
-               // It would need to take a bunch more things then.
-               switch resp.StatusCode {
-               case 307, 308:
-                       loc := resp.Header.Get("Location")
-                       if loc == "" {
-                               // 308s have been observed in the wild being served
-                               // without Location headers. Since Go 1.7 and earlier
-                               // didn't follow these codes, just stop here instead
-                               // of returning an error.
-                               shouldRedirect = false
-                               break
-                       }
-                       ireq := reqs[0]
-                       if ireq.GetBody == nil && ireq.outgoingLength() != 0 {
-                               // We had a request body, and 307/308 require
-                               // re-sending it, but GetBody is not defined. So just
-                               // return this response to the user instead of an
-                               // error, like we did in Go 1.7 and earlier.
-                               shouldRedirect = false
-                       }
-               }
-
+               redirectMethod, shouldRedirect = redirectBehavior(req.Method, resp, reqs)
                if !shouldRedirect {
                        return resp, nil
                }