net/http/httputil: copy header map if necessary in ReverseProxy

We were already making a copy of the map before removing
hop-by-hop headers. This commit does the same for proxied
headers mentioned in the "Connection" header.

A test is added to ensure request headers are not modified.

Updates #16875

Change-Id: I85329d212787958d5ad818915eb0538580a4653a
Reviewed-on: https://go-review.googlesource.com/28493
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go
index 47cd0ae97d7877cac5d08f206a232a1111981966..2b38e0fdd87d95a690cfd90b01414ba381fe05fe 100644 (file)
--- a/src/net/http/httputil/reverseproxy.go
+++ b/src/net/http/httputil/reverseproxy.go
@@ -152,11 +152,20 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
        p.Director(outreq)
        outreq.Close = false
 
+       // We are modifying the same underlying map from req (shallow
+       // copied above) so we only copy it if necessary.
+       copiedHeaders := false
+
        // Remove headers with the same name as the connection-tokens.
        // See RFC 2616, section 14.10.
        if c := outreq.Header.Get("Connection"); c != "" {
                for _, f := range strings.Split(c, ",") {
                        if f = strings.TrimSpace(f); f != "" {
+                               if !copiedHeaders {
+                                       outreq.Header = make(http.Header)
+                                       copyHeader(outreq.Header, req.Header)
+                                       copiedHeaders = true
+                               }
                                outreq.Header.Del(f)
                        }
                }
@@ -164,10 +173,7 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 
        // Remove hop-by-hop headers to the backend. Especially
        // important is "Connection" because we want a persistent
-       // connection, regardless of what the client sent to us. This
-       // is modifying the same underlying map from req (shallow
-       // copied above) so we only copy it if necessary.
-       copiedHeaders := false
+       // connection, regardless of what the client sent to us.
        for _, h := range hopHeaders {
                if outreq.Header.Get(h) != "" {
                        if !copiedHeaders {

diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go
index 8ab57b4cbb020f4411f4f73067a90ba9f0ae706d..870df130b1cd47c211285723af532d9fc4b35116 100644 (file)
--- a/src/net/http/httputil/reverseproxy_test.go
+++ b/src/net/http/httputil/reverseproxy_test.go
@@ -156,12 +156,17 @@ func TestReverseProxyStripHeadersPresentInConnection(t *testing.T) {
                t.Fatal(err)
        }
        proxyHandler := NewSingleHostReverseProxy(backendURL)
-       frontend := httptest.NewServer(proxyHandler)
+       frontend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+               proxyHandler.ServeHTTP(w, r)
+               if c := r.Header.Get("Upgrade"); c != "original value" {
+                       t.Errorf("handler modified header %q = %q; want %q", "Upgrade", c, "original value")
+               }
+       }))
        defer frontend.Close()
 
        getReq, _ := http.NewRequest("GET", frontend.URL, nil)
        getReq.Header.Set("Connection", "Upgrade, "+fakeConnectionToken)
-       getReq.Header.Set("Upgrade", "foo")
+       getReq.Header.Set("Upgrade", "original value")
        getReq.Header.Set(fakeConnectionToken, "should be deleted")
        res, err := http.DefaultClient.Do(getReq)
        if err != nil {