Repository: 0xERR0R/blocky

Issue:
http incorrect parsing of x-forwarded-for
Blocky parses `x-forwarded-for` header incorrectly when used as a http service behind a reverse proxy.

While `x-forwaded-for` isn't exactly a standard, most of the time it works like documented on MDN https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-Forwarded-For.

Usually it's 1 or multiple IP addresses separated by a comma with the 1st being the IP address of the client in the initial request.

The way Blocky handles this here https://github.com/0xERR0R/blocky/blob/main/util/http.go#L58 has 2 issues:
1. it doesn't handle comma separated IP addresses
2. it expects host being there

All this results in client IP not being picked up correctly in setups where Blocky is behind a reverse proxy, breaking multiple client IP related functions.

Hints from issue comments:
@kotarac: Thanks for taking care of this quickly. Tested it with traefik in front of blocky and it works as expected now!@0xERR0R: Thanks for reporting!

Solution patch:
diff --git a/util/http.go b/util/http.go
index 94356d4..ee59010 100644
--- a/util/http.go
+++ b/util/http.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"strings"
 )
 
 //nolint:gochecknoglobals
@@ -49,15 +50,41 @@ func DefaultHTTPTransport() *http.Transport {
 	}
 }
 
+// splitCommaSeparated splits a comma-separated string and trims whitespace from each part
+func splitCommaSeparated(s string) []string {
+	parts := strings.Split(s, ",")
+	result := make([]string, 0, len(parts))
+
+	for _, part := range parts {
+		trimmed := strings.TrimSpace(part)
+		if trimmed != "" {
+			result = append(result, trimmed)
+		}
+	}
+
+	return result
+}
+
 func HTTPClientIP(r *http.Request) net.IP {
-	addr := r.Header.Get("X-Forwarded-For")
-	if addr == "" {
-		addr = r.RemoteAddr
+	// Try X-Forwarded-For header first (used when behind reverse proxy)
+	xff := r.Header.Get("X-Forwarded-For")
+	if xff != "" {
+		// X-Forwarded-For format: "<client>, <proxy1>, <proxy2>, ..."
+		// The leftmost IP is the original client
+		ips := splitCommaSeparated(xff)
+		if len(ips) > 0 {
+			// Parse the first IP (original client)
+			if ip := net.ParseIP(ips[0]); ip != nil {
+				return ip
+			}
+		}
 	}
 
-	ip, _, err := net.SplitHostPort(addr)
+	// Fallback to RemoteAddr (format: "host:port")
+	ip, _, err := net.SplitHostPort(r.RemoteAddr)
 	if err != nil {
-		return net.ParseIP(addr)
+		// RemoteAddr might not have a port in some cases
+		return net.ParseIP(r.RemoteAddr)
 	}
 
 	return net.ParseIP(ip)



Test patch:
diff --git a/util/http_test.go b/util/http_test.go
index 52a7197..0b10b74 100644
--- a/util/http_test.go
+++ b/util/http_test.go
@@ -66,6 +66,77 @@ var _ = Describe("HTTP Util", func() {
 
 			Expect(HTTPClientIP(r)).Should(Equal(ip))
 		})
+
+		It("extracts the first IP from comma-separated X-Forwarded-For header", func() {
+			r, err := http.NewRequest(http.MethodGet, "http://example.com", nil)
+			Expect(err).Should(Succeed())
+
+			clientIP := net.ParseIP("203.0.113.195")
+			proxy1IP := net.ParseIP("70.41.3.18")
+			proxy2IP := net.ParseIP("150.172.238.178")
+
+			r.RemoteAddr = net.JoinHostPort("192.168.1.1", "12345")
+			r.Header.Set("X-Forwarded-For", clientIP.String()+", "+proxy1IP.String()+", "+proxy2IP.String())
+
+			Expect(HTTPClientIP(r)).Should(Equal(clientIP))
+		})
+
+		It("extracts the first IP from X-Forwarded-For with extra spaces", func() {
+			r, err := http.NewRequest(http.MethodGet, "http://example.com", nil)
+			Expect(err).Should(Succeed())
+
+			clientIP := net.ParseIP("203.0.113.195")
+			r.RemoteAddr = net.JoinHostPort("192.168.1.1", "12345")
+			r.Header.Set("X-Forwarded-For", "  "+clientIP.String()+"  , 70.41.3.18 ,  150.172.238.178")
+
+			Expect(HTTPClientIP(r)).Should(Equal(clientIP))
+		})
+
+		It("handles IPv6 addresses in X-Forwarded-For", func() {
+			r, err := http.NewRequest(http.MethodGet, "http://example.com", nil)
+			Expect(err).Should(Succeed())
+
+			clientIP := net.ParseIP("2001:db8:85a3:8d3:1319:8a2e:370:7348")
+			proxy1IP := net.ParseIP("2001:db8::1")
+
+			r.RemoteAddr = net.JoinHostPort("192.168.1.1", "12345")
+			r.Header.Set("X-Forwarded-For", clientIP.String()+", "+proxy1IP.String())
+
+			Expect(HTTPClientIP(r)).Should(Equal(clientIP))
+		})
+
+		It("falls back to RemoteAddr when X-Forwarded-For is invalid", func() {
+			r, err := http.NewRequest(http.MethodGet, "http://example.com", nil)
+			Expect(err).Should(Succeed())
+
+			remoteIP := net.ParseIP("192.168.1.100")
+			r.RemoteAddr = net.JoinHostPort(remoteIP.String(), "12345")
+			r.Header.Set("X-Forwarded-For", "not-a-valid-ip, also-invalid")
+
+			Expect(HTTPClientIP(r)).Should(Equal(remoteIP))
+		})
+
+		It("falls back to RemoteAddr when X-Forwarded-For is empty string", func() {
+			r, err := http.NewRequest(http.MethodGet, "http://example.com", nil)
+			Expect(err).Should(Succeed())
+
+			remoteIP := net.ParseIP("192.168.1.100")
+			r.RemoteAddr = net.JoinHostPort(remoteIP.String(), "12345")
+			r.Header.Set("X-Forwarded-For", "")
+
+			Expect(HTTPClientIP(r)).Should(Equal(remoteIP))
+		})
+
+		It("handles X-Forwarded-For with only whitespace", func() {
+			r, err := http.NewRequest(http.MethodGet, "http://example.com", nil)
+			Expect(err).Should(Succeed())
+
+			remoteIP := net.ParseIP("192.168.1.100")
+			r.RemoteAddr = net.JoinHostPort(remoteIP.String(), "12345")
+			r.Header.Set("X-Forwarded-For", "   ,  , ")
+
+			Expect(HTTPClientIP(r)).Should(Equal(remoteIP))
+		})
 	})
 })