net/http: handle 3xx responses with no Location

RFC 7231 does not require that a 3xx response contain a Location header.
When receiving such a response, just return it to the caller rather than
treating it as an error.

Fixes #49281.

Change-Id: I66c06d81b0922016384a0f4ff32bf52e3a3d5983
Reviewed-on: https://go-review.googlesource.com/c/go/+/375354
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/client.go b/src/net/http/client.go
index 22db96b2674c9037fd8620b38ea6c1f03ef85c87..5fd184bcb167d4107512a62ac17569deee75c5ee 100644 (file)
--- a/src/net/http/client.go
+++ b/src/net/http/client.go
@@ -519,17 +519,6 @@ func redirectBehavior(reqMethod string, resp *Response, ireq *Request) (redirect
                shouldRedirect = true
                includeBody = true
 
-               // Treat 307 and 308 specially, since they're new in
-               // Go 1.8, and they also require re-sending the request body.
-               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
-                       // of returning an error.
-                       // See Issue 17773.
-                       shouldRedirect = false
-                       break
-               }
                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
@@ -641,8 +630,10 @@ func (c *Client) do(req *Request) (retres *Response, reterr error) {
                if len(reqs) > 0 {
                        loc := resp.Header.Get("Location")
                        if loc == "" {
-                               resp.closeBody()
-                               return nil, uerr(fmt.Errorf("%d response missing Location header", resp.StatusCode))
+                               // While most 3xx responses include a Location, it is not
+                               // required and 3xx responses without a Location have been
+                               // observed in the wild. See issues #17773 and #49281.
+                               return resp, nil
                        }
                        u, err := req.URL.Parse(loc)
                        if err != nil {

diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go
index e91d5268244db8a66d94f9c33f4cc6eadec5e8f2..5e5bf8f2bbf0c0f435061253e746522d77e3c619 100644 (file)
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -531,27 +531,31 @@ func TestClientRedirectUseResponse(t *testing.T) {
        }
 }
 
-// Issue 17773: don't follow a 308 (or 307) if the response doesn't
+// Issues 17773 and 49281: don't follow a 3xx if the response doesn't
 // have a Location header.
-func TestClientRedirect308NoLocation(t *testing.T) {
-       setParallel(t)
-       defer afterTest(t)
-       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
-               w.Header().Set("Foo", "Bar")
-               w.WriteHeader(308)
-       }))
-       defer ts.Close()
-       c := ts.Client()
-       res, err := c.Get(ts.URL)
-       if err != nil {
-               t.Fatal(err)
-       }
-       res.Body.Close()
-       if res.StatusCode != 308 {
-               t.Errorf("status = %d; want %d", res.StatusCode, 308)
-       }
-       if got := res.Header.Get("Foo"); got != "Bar" {
-               t.Errorf("Foo header = %q; want Bar", got)
+func TestClientRedirectNoLocation(t *testing.T) {
+       for _, code := range []int{301, 308} {
+               t.Run(fmt.Sprint(code), func(t *testing.T) {
+                       setParallel(t)
+                       defer afterTest(t)
+                       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+                               w.Header().Set("Foo", "Bar")
+                               w.WriteHeader(code)
+                       }))
+                       defer ts.Close()
+                       c := ts.Client()
+                       res, err := c.Get(ts.URL)
+                       if err != nil {
+                               t.Fatal(err)
+                       }
+                       res.Body.Close()
+                       if res.StatusCode != code {
+                               t.Errorf("status = %d; want %d", res.StatusCode, code)
+                       }
+                       if got := res.Header.Get("Foo"); got != "Bar" {
+                               t.Errorf("Foo header = %q; want Bar", got)
+                       }
+               })
        }
 }