net/http/httputil: fix regression in ReverseProxy.ServeHTTP

In Go1.12 and below, the logic in ReverseProxy.ServeHTTP would always
allocate request.Header even if it were not present in the incoming request.
CL 174324 added http.Request.Clone and re-factors ReverseProxy.ServeHTTP
to use the new Clone method. However, the new Clone logic is not equivalent
to the former logic. We preserve former semantics by explicitly allocating
the Header map if nil.

Fixes #33142

Change-Id: I356f94a915dd9779584ce3fe31e56e5474b9ad37
Reviewed-on: https://go-review.googlesource.com/c/go/+/186437
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>

diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go
index 1d7b0efa11b91ff7cc463497dc3270c33534569e..e8f7df29a14d41da098361f71105ae66a56c063c 100644 (file)
--- a/src/net/http/httputil/reverseproxy.go
+++ b/src/net/http/httputil/reverseproxy.go
@@ -199,6 +199,9 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
        if req.ContentLength == 0 {
                outreq.Body = nil // Issue 16036: nil Body for http.Transport retries
        }
+       if outreq.Header == nil {
+               outreq.Header = make(http.Header) // Issue 33142: historical behavior was to always allocate
+       }
 
        p.Director(outreq)
        outreq.Close = false

diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go
index e8cb8149387a453cdff5626be3753d4b20daec10..7f9dc0800f8d91e62be983ae45ee5bfbf5c134e7 100644 (file)
--- a/src/net/http/httputil/reverseproxy_test.go
+++ b/src/net/http/httputil/reverseproxy_test.go
@@ -659,6 +659,26 @@ func TestReverseProxy_NilBody(t *testing.T) {
        }
 }
 
+// Issue 33142: always allocate the request headers
+func TestReverseProxy_AllocatedHeader(t *testing.T) {
+       proxyHandler := new(ReverseProxy)
+       proxyHandler.ErrorLog = log.New(ioutil.Discard, "", 0) // quiet for tests
+       proxyHandler.Director = func(*http.Request) {}         // noop
+       proxyHandler.Transport = RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
+               if req.Header == nil {
+                       t.Error("Header == nil; want a non-nil Header")
+               }
+               return nil, errors.New("done testing the interesting part; so force a 502 Gateway error")
+       })
+
+       proxyHandler.ServeHTTP(httptest.NewRecorder(), &http.Request{
+               Method:     "GET",
+               URL:        &url.URL{Scheme: "http", Host: "fake.tld", Path: "/"},
+               Proto:      "HTTP/1.0",
+               ProtoMajor: 1,
+       })
+}
+
 // Issue 14237. Test ModifyResponse and that an error from it
 // causes the proxy to return StatusBadGateway, or StatusOK otherwise.
 func TestReverseProxyModifyResponse(t *testing.T) {