From 08b0254ed9fc8810f32272cc0ce730c8f5667f8a Mon Sep 17 00:00:00 2001 From: Robin Robin Date: Tue, 31 Jul 2018 21:41:33 -0400 Subject: [PATCH 1/3] fix: To support multiple XFF headers we need to use r.Header.Add r.Header.Set simply overwrites the values. The expected public IP should be the first public IP encountered. --- realip_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/realip_test.go b/realip_test.go index e80efe0..f537ed8 100644 --- a/realip_test.go +++ b/realip_test.go @@ -52,7 +52,7 @@ func TestRealIP(t *testing.T) { h := http.Header{} h.Set("X-Real-IP", xRealIP) for _, address := range xForwardedFor { - h.Set("X-Forwarded-For", address) + h.Add("X-Forwarded-For", address) } return &http.Request{ @@ -78,7 +78,7 @@ func TestRealIP(t *testing.T) { }, { name: "Has multiple X-Forwarded-For", request: newRequest("", "", localAddr, publicAddr1, publicAddr2), - expected: publicAddr2, + expected: publicAddr1, }, { name: "Has X-Real-IP", request: newRequest("", publicAddr1), From d4c26401621761852f843b8c29bfe19ac98952df Mon Sep 17 00:00:00 2001 From: Robin Robin Date: Tue, 31 Jul 2018 21:42:40 -0400 Subject: [PATCH 2/3] fix: to truly support xff headers we need to query Header directly When we use Header.Get only the first entry is fetched. Per https://golang.org/src/net/http/request.go, when there are multiple entries of the same header, accessing the map directly allows us to get those multiple entries in an array. // Header contains the request header fields either received // by the server or to be sent by the client. // // If a server received a request with header lines, // // Host: example.com // accept-encoding: gzip, deflate // Accept-Language: en-us // fOO: Bar // foo: two // // then // // Header = map[string][]string{ // "Accept-Encoding": {"gzip, deflate"}, // "Accept-Language": {"en-us"}, // "Foo": {"Bar", "two"}, --- realip.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/realip.go b/realip.go index e2803a2..dd6b3ca 100644 --- a/realip.go +++ b/realip.go @@ -53,10 +53,10 @@ func isPrivateAddress(address string) (bool, error) { func FromRequest(r *http.Request) string { // Fetch header value xRealIP := r.Header.Get("X-Real-Ip") - xForwardedFor := r.Header.Get("X-Forwarded-For") + xForwardedFor, _ := r.Header["X-Forwarded-For"] // If both empty, return IP from remote address - if xRealIP == "" && xForwardedFor == "" { + if xRealIP == "" && len(xForwardedFor) == 0 { var remoteIP string // If there are colon in remote address, remove the port number @@ -71,7 +71,7 @@ func FromRequest(r *http.Request) string { } // Check list of IP in X-Forwarded-For and return the first global address - for _, address := range strings.Split(xForwardedFor, ",") { + for _, address := range xForwardedFor { address = strings.TrimSpace(address) isPrivate, err := isPrivateAddress(address) if !isPrivate && err == nil { From 1a25333e81977e08740df6b339e7e563ca5c2ddd Mon Sep 17 00:00:00 2001 From: Robin Robin Date: Wed, 1 Aug 2018 09:11:52 -0400 Subject: [PATCH 3/3] fix: handles the case of xff headers containing comma-sep items XFF can contain IP1,IP2,IP3 XFF can contain IP1, IP2 (note the space) --- realip.go | 12 +++++++----- realip_test.go | 9 +++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/realip.go b/realip.go index dd6b3ca..f07fd35 100644 --- a/realip.go +++ b/realip.go @@ -71,11 +71,13 @@ func FromRequest(r *http.Request) string { } // Check list of IP in X-Forwarded-For and return the first global address - for _, address := range xForwardedFor { - address = strings.TrimSpace(address) - isPrivate, err := isPrivateAddress(address) - if !isPrivate && err == nil { - return address + for _, a := range xForwardedFor { + for _, b := range strings.Split(a, ",") { + address := strings.TrimSpace(b) + isPrivate, err := isPrivateAddress(address) + if !isPrivate && err == nil { + return address + } } } diff --git a/realip_test.go b/realip_test.go index f537ed8..4da3488 100644 --- a/realip_test.go +++ b/realip_test.go @@ -1,6 +1,7 @@ package realip import ( + "fmt" "net/http" "testing" ) @@ -75,6 +76,14 @@ func TestRealIP(t *testing.T) { name: "Has X-Forwarded-For", request: newRequest("", "", publicAddr1), expected: publicAddr1, + }, { + name: "Has X-Forwarded-For multiple IPs (comma separated)(", + request: newRequest("", "", fmt.Sprintf("%s,%s", localAddr, publicAddr1)), + expected: publicAddr1, + }, { + name: "Has X-Forwarded-For multiple IPs (comma and then space)", + request: newRequest("", "", fmt.Sprintf("%s, %s", localAddr, publicAddr1)), + expected: publicAddr1, }, { name: "Has multiple X-Forwarded-For", request: newRequest("", "", localAddr, publicAddr1, publicAddr2),