From a8871194f296383d313972da083e1b5f7513dfeb Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 9 Jan 2017 01:00:36 -0800 Subject: 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 Reviewed-by: Emmanuel Odeke Reviewed-by: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/client.go | 22 ++++++++++++++-------- src/net/http/client_test.go | 6 +++--- 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"}, -- cgit v1.2.3-54-g00ecf