httputil: accumulate X-Forwarded-For header info

If the X-Forwarded-For header already exists on a request, we
should append our client's IP to it after a comma+space instead
of overwriting it.

Fixes #3846.

R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/6448053

Author: bpowers

src/pkg/net/http/httputil/reverseproxy.go

@@ -106,8 +106,14 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 		outreq.Header.Del("Connection")
 	}
 
-	if clientIp, _, err := net.SplitHostPort(req.RemoteAddr); err == nil {
-		outreq.Header.Set("X-Forwarded-For", clientIp)
+	if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil {
+		// If we aren't the first proxy retain prior
+		// X-Forwarded-For information as a comma+space
+		// separated list and fold multiple headers into one.
+		if prior, ok := outreq.Header["X-Forwarded-For"]; ok {
+			clientIP = strings.Join(prior, ", ") + ", " + clientIP
+		}
+		outreq.Header.Set("X-Forwarded-For", clientIP)
 	}
 
 	res, err := transport.RoundTrip(outreq)

src/pkg/net/http/httputil/reverseproxy_test.go

@@ -11,6 +11,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 	"time"
 )
@@ -71,6 +72,47 @@ func TestReverseProxy(t *testing.T) {
 	}
 }
 
+func TestXForwardedFor(t *testing.T) {
+	const prevForwardedFor = "client ip"
+	const backendResponse = "I am the backend"
+	const backendStatus = 404
+	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("X-Forwarded-For") == "" {
+			t.Errorf("didn't get X-Forwarded-For header")
+		}
+		if !strings.Contains(r.Header.Get("X-Forwarded-For"), prevForwardedFor) {
+			t.Errorf("X-Forwarded-For didn't contain prior data")
+		}
+		w.WriteHeader(backendStatus)
+		w.Write([]byte(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.Host = "some-name"
+	getReq.Header.Set("Connection", "close")
+	getReq.Header.Set("X-Forwarded-For", prevForwardedFor)
+	getReq.Close = true
+	res, err := http.DefaultClient.Do(getReq)
+	if err != nil {
+		t.Fatalf("Get: %v", err)
+	}
+	if g, e := res.StatusCode, backendStatus; g != e {
+		t.Errorf("got res.StatusCode %d; expected %d", g, e)
+	}
+	bodyBytes, _ := ioutil.ReadAll(res.Body)
+	if g, e := string(bodyBytes), backendResponse; g != e {
+		t.Errorf("got body %q; expected %q", g, e)
+	}
+}
+
 var proxyQueryTests = []struct {
 	baseSuffix string // suffix to add to backend URL
 	reqSuffix  string // suffix to add to frontend's request URL