net/http: preserve original HTTP method when possible

In Go1.7, a 301, 302, or 303 redirect on a HEAD method, would still
cause the following redirects to still use a HEAD.
In CL/29852 this behavior was changed such that those codes always
caused a redirect with the GET method. Fix this such that both
GET and HEAD will preserve the method.

Fixes #18570

Change-Id: I4bfe69872a30799419e3fad9178f907fe439b449
Reviewed-on: https://go-review.googlesource.com/34981
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: 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 7eb87c6d10d8ccd69b70a1fe7710328128e9d187..d368bae861e8e349f6a71090bd0399babd023afa 100644 (file)
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -413,19 +413,26 @@ 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, resp *Response, via []*Request) (redirectMethod string, shouldRedirect bool) {
+func redirectBehavior(reqMethod string, resp *Response, ireq *Request) (redirectMethod string, shouldRedirect bool) {
        switch resp.StatusCode {
        case 301, 302, 303:
-               redirectMethod = "GET"
+               redirectMethod = reqMethod
                shouldRedirect = true
+
+               // RFC 2616 allowed automatic redirection only with GET and
+               // HEAD requests. RFC 7231 lifts this restriction, but we still
+               // restrict other methods to GET to maintain compatibility.
+               // See Issue 18570.
+               if reqMethod != "GET" && reqMethod != "HEAD" {
+                       redirectMethod = "GET"
+               }
        case 307, 308:
                redirectMethod = reqMethod
                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 == "" {
+               if resp.Header.Get("Location") == "" {
                        // 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
@@ -434,7 +441,6 @@ func redirectBehavior(reqMethod string, resp *Response, via []*Request) (redirec
                        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
@@ -443,7 +449,6 @@ func redirectBehavior(reqMethod string, resp *Response, via []*Request) (redirec
                        shouldRedirect = false
                }
        }
-
        return redirectMethod, shouldRedirect
 }
 
@@ -474,7 +479,8 @@ func redirectBehavior(reqMethod string, resp *Response, via []*Request) (redirec
 // If the server replies with a redirect, the Client first uses the
 // CheckRedirect function to determine whether the redirect should be
 // followed. If permitted, a 301, 302, or 303 redirect causes
-// subsequent requests to use HTTP method "GET", with no body.
+// subsequent requests to use HTTP method GET
+// (or HEAD if the original request was HEAD), with no body.
 // A 307 or 308 redirect preserves the original HTTP method and body,
 // provided that the Request.GetBody function is defined.
 // The NewRequest function automatically sets GetBody for common
@@ -592,7 +598,7 @@ func (c *Client) Do(req *Request) (*Response, error) {
                }
 
                var shouldRedirect bool
-               redirectMethod, shouldRedirect = redirectBehavior(req.Method, resp, reqs)
+               redirectMethod, shouldRedirect = redirectBehavior(req.Method, resp, reqs[0])
                if !shouldRedirect {
                        return resp, nil
                }

diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go
index ca6e9180f1c7d84ec8343d7bba571856a11f57f1..eaf2cdca8ee3afc84722b949e7515916927b2855 100644 (file)
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -1665,9 +1665,9 @@ func TestClientRedirectTypes(t *testing.T) {
                3: {method: "POST", serverStatus: 307, wantMethod: "POST"},
                4: {method: "POST", serverStatus: 308, wantMethod: "POST"},
 
-               5: {method: "HEAD", serverStatus: 301, wantMethod: "GET"},
-               6: {method: "HEAD", serverStatus: 302, wantMethod: "GET"},
-               7: {method: "HEAD", serverStatus: 303, wantMethod: "GET"},
+               5: {method: "HEAD", serverStatus: 301, wantMethod: "HEAD"},
+               6: {method: "HEAD", serverStatus: 302, wantMethod: "HEAD"},
+               7: {method: "HEAD", serverStatus: 303, wantMethod: "HEAD"},
                8: {method: "HEAD", serverStatus: 307, wantMethod: "HEAD"},
                9: {method: "HEAD", serverStatus: 308, wantMethod: "HEAD"},