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>

2 files changed, 28 insertions, 33 deletions

diff --git a/src/net/http/client.go b/src/net/http/client.go
index 22db96b267..5fd184bcb1 100644
--- 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 e91d526824..5e5bf8f2bb 100644
--- 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)
+			}
+		})
 	}
 }