From f93234ad620cc34573bca56be9fcf55c975e0821 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Tue, 16 Jul 2019 17:40:20 -0700 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Andrew Bonventre --- src/net/http/httputil/reverseproxy.go | 3 +++ src/net/http/httputil/reverseproxy_test.go | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go index 1d7b0efa11..e8f7df29a1 100644 --- 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 e8cb814938..7f9dc0800f 100644 --- 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) { -- 2.48.1