net/http/httputil: remove proxied headers mentioned in connection-tokens

RFC 2616, section 14.10 says:

>>>
HTTP/1.1 proxies MUST parse the Connection header field before a message
is forwarded and, for each connection-token in this field, remove any
header field(s) from the message with the same name as the
connection-token. Connection options are signaled by the presence of a
connection-token in the Connection header field, not by any
corresponding additional header field(s), since the additional header
field may not be sent if there are no parameters associated with that
connection option.
<<<

The same requirement was included in RFC 7230, section 6.1.

Fixes #16875

Change-Id: I57ad4a4a17775537c8810d0edd7de1604317b5fa
Reviewed-on: https://go-review.googlesource.com/27970Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/net/http/httputil/reverseproxy.go

@@ -184,6 +184,16 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 	outreq.ProtoMinor = 1
 	outreq.Close = 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 != "" {
+				outreq.Header.Del(f)
+			}
+		}
+	}
+
 	// 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

src/net/http/httputil/reverseproxy_test.go

@@ -135,6 +135,47 @@ func TestReverseProxy(t *testing.T) {
 
 }
 
+// Issue 16875: remove any proxied headers mentioned in the "Connection"
+// header value.
+func TestReverseProxyStripHeadersPresentInConnection(t *testing.T) {
+	const fakeConnectionToken = "X-Fake-Connection-Token"
+	const backendResponse = "I am the backend"
+	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if c := r.Header.Get(fakeConnectionToken); c != "" {
+			t.Errorf("handler got header %q = %q; want empty", fakeConnectionToken, c)
+		}
+		if c := r.Header.Get("Upgrade"); c != "" {
+			t.Errorf("handler got header %q = %q; want empty", "Upgrade", c)
+		}
+		io.WriteString(w, backendResponse)
+	}))
+	defer backend.Close()
+	backendURL, err := url.Parse(backend.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	proxyHandler := NewSingleHostReverseProxy(backendURL)
+	frontend := httptest.NewServer(proxyHandler)
+	defer frontend.Close()
+
+	getReq, _ := http.NewRequest("GET", frontend.URL, nil)
+	getReq.Header.Set("Connection", "Upgrade, "+fakeConnectionToken)
+	getReq.Header.Set("Upgrade", "foo")
+	getReq.Header.Set(fakeConnectionToken, "should be deleted")
+	res, err := http.DefaultClient.Do(getReq)
+	if err != nil {
+		t.Fatalf("Get: %v", err)
+	}
+	defer res.Body.Close()
+	bodyBytes, err := ioutil.ReadAll(res.Body)
+	if err != nil {
+		t.Fatalf("reading body: %v", err)
+	}
+	if got, want := string(bodyBytes), backendResponse; got != want {
+		t.Errorf("got body %q; want %q", got, want)
+	}
+}
+
 func TestXForwardedFor(t *testing.T) {
 	const prevForwardedFor = "client ip"
 	const backendResponse = "I am the backend"