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>

2 files changed, 17 insertions, 11 deletions

diff --git a/src/net/http/client.go b/src/net/http/client.go
index 7eb87c6d10..d368bae861 100644
--- 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 ca6e9180f1..eaf2cdca8e 100644
--- 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"},