Verify IP addresses

Fix lint errors
Add more test

AUTHORS

@@ -13,6 +13,8 @@ Frederick F. Kautz IV <fkautz@alumni.cmu.edu>
 Josh Hawn <josh.hawn@docker.com>
 Nghia Tran <tcnghia@gmail.com>
 Olivier Gambier <olivier@docker.com>
+Richard <richard.scothern@gmail.com>
+Shreyas Karnik <karnik.shreyas@gmail.com>
 Stephen J Day <stephen.day@docker.com>
 Tianon Gravi <admwiggin@gmail.com>
 xiekeyang <xiekeyang@huawei.com>

context/http.go

@@ -2,6 +2,7 @@ package context
 
 import (
 	"errors"
+	"net"
 	"net/http"
 	"strings"
 	"sync"
@@ -17,26 +18,27 @@ var (
 	ErrNoRequestContext = errors.New("no http request in context")
 )
 
-// http.Request.RemoteAddr is not the originating IP if the request
-// came through a reverse proxy.
-// X-Forwarded-For and X-Real-IP provide this information with decreased
-// support, so check in that order.  There may be multiple 'X-Forwarded-For'
-// headers, but go maps don't retain order so just pick one.
+// RemoteAddr extracts the remote address of the request, taking into
+// account proxy headers.
 func RemoteAddr(r *http.Request) string {
-	remoteAddr := r.RemoteAddr
-
-	if prior, ok := r.Header["X-Forwarded-For"]; ok {
-		proxies := strings.Split(prior[0], ",")
+	if prior := r.Header.Get("X-Forwarded-For"); prior != "" {
+		proxies := strings.Split(prior, ",")
 		if len(proxies) > 0 {
-			remoteAddr = strings.Trim(proxies[0], " ")
+			remoteAddr := strings.Trim(proxies[0], " ")
+			if net.ParseIP(remoteAddr) != nil {
+				return remoteAddr
 			}
-	} else if realip, ok := r.Header["X-Real-Ip"]; ok {
-		if len(realip) > 0 {
-			remoteAddr = realip[0]
+		}
+	}
+	// X-Real-Ip is less supported, but worth checking in the
+	// absence of X-Forwarded-For
+	if realIP := r.Header.Get("X-Real-Ip"); realIP != "" {
+		if net.ParseIP(realIP) != nil {
+			return realIP
 		}
 	}
 
-	return remoteAddr
+	return r.RemoteAddr
 }
 
 // WithRequest places the request on the context. The context of the request
@@ -44,7 +46,6 @@ func RemoteAddr(r *http.Request) string {
 // is available at "http.request". Other common attributes are available under
 // the prefix "http.request.". If a request is already present on the context,
 // this method will panic.
-
 func WithRequest(ctx context.Context, r *http.Request) context.Context {
 	if ctx.Value("http.request") != nil {
 		// NOTE(stevvooe): This needs to be considered a programming error. It

context/http_test.go

@@ -214,16 +214,15 @@ func TestWithVars(t *testing.T) {
 // at the transport layer to 127.0.0.1:<port> .  However, as the X-Forwarded-For header
 // just contains the IP address, it is different enough for testing.
 func TestRemoteAddr(t *testing.T) {
-	expectedRemote := "127.0.0.1"
-	var actualRemote string
+	var expectedRemote string
 	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		defer r.Body.Close()
 
 		if r.RemoteAddr == expectedRemote {
 			t.Errorf("Unexpected matching remote addresses")
 		}
-		actualRemote = RemoteAddr(r)
 
+		actualRemote := RemoteAddr(r)
 		if expectedRemote != actualRemote {
 			t.Errorf("Mismatching remote hosts: %v != %v", expectedRemote, actualRemote)
 		}
@@ -241,14 +240,35 @@ func TestRemoteAddr(t *testing.T) {
 	frontend := httptest.NewServer(proxy)
 	defer frontend.Close()
 
-	getReq, err := http.NewRequest("GET", frontend.URL, nil)
+	// X-Forwarded-For set by proxy
+	expectedRemote = "127.0.0.1"
+	proxyReq, err := http.NewRequest("GET", frontend.URL, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 
+	_, err = http.DefaultClient.Do(proxyReq)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// RemoteAddr in X-Real-Ip
+	getReq, err := http.NewRequest("GET", backend.URL, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expectedRemote = "1.2.3.4"
+	getReq.Header["X-Real-ip"] = []string{expectedRemote}
 	_, err = http.DefaultClient.Do(getReq)
 	if err != nil {
 		t.Fatal(err)
 	}
 
+	// Valid X-Real-Ip and invalid X-Forwarded-For
+	getReq.Header["X-forwarded-for"] = []string{"1.2.3"}
+	_, err = http.DefaultClient.Do(getReq)
+	if err != nil {
+		t.Fatal(err)
+	}
 }

notifications/bridge.go

@@ -43,7 +43,6 @@ func NewBridge(ub URLBuilder, source SourceRecord, actor ActorRecord, request Re
 
 // NewRequestRecord builds a RequestRecord for use in NewBridge from an
 // http.Request, associating it with a request id.
-
 func NewRequestRecord(id string, r *http.Request) RequestRecord {
 	return RequestRecord{
 		ID:        id,