Merge pull request #2933 from aldas/LegacyIPExtractor

Remove legacy IP extraction logic from context.RealIP method

Author: aldas

context.go

@@ -180,29 +180,28 @@ func (c *Context) Scheme() string {
 	return "http"
 }
 
-// RealIP returns the client's network address based on `X-Forwarded-For`
-// or `X-Real-IP` request header.
-// The behavior can be configured using `Echo#IPExtractor`.
+// RealIP returns the client IP address using the configured extraction strategy.
+//
+// If Echo#IPExtractor is set, it is used to resolve the client IP from the incoming request (typically via proxy
+// headers such as X-Forwarded-For or X-Real-IP).
+// Look into the `ip.go` file for comments and examples.
+//
+// See:
+//   - Echo#ExtractIPFromXFFHeader for `X-Forwarded-For` handling with trust checks
+//   - Echo#ExtractIPFromRealIPHeader for `X-Real-IP` handling with trust checks
+//   - Echo#LegacyIPExtractor for `v4` compatibility (spoofable, no trust checks built in)
+//
+// If no extractor is configured, RealIP falls back to the request RemoteAddr, returning only the host portion.
+//
+// Notes:
+//   - No validation or trust enforcement is performed unless implemented by the configured IPExtractor.
+//   - When relying on proxy headers, ensure the application is deployed behind trusted intermediaries to avoid spoofing.
 func (c *Context) RealIP() string {
 	if c.echo != nil && c.echo.IPExtractor != nil {
 		return c.echo.IPExtractor(c.request)
 	}
-	// Fall back to legacy behavior
-	if ip := c.request.Header.Get(HeaderXForwardedFor); ip != "" {
-		i := strings.IndexAny(ip, ",")
-		if i > 0 {
-			xffip := strings.TrimSpace(ip[:i])
-			xffip = strings.TrimPrefix(xffip, "[")
-			xffip = strings.TrimSuffix(xffip, "]")
-			return xffip
-		}
-		return ip
-	}
-	if ip := c.request.Header.Get(HeaderXRealIP); ip != "" {
-		ip = strings.TrimPrefix(ip, "[")
-		ip = strings.TrimSuffix(ip, "]")
-		return ip
-	}
+	// req.RemoteAddr is the IP address of the remote end of the connection, which may be a proxy. It is populated by the
+	// http.conn.readRequest() method and uses net.Conn.RemoteAddr().String() which we trust.
 	ra, _, _ := net.SplitHostPort(c.request.RemoteAddr)
 	return ra
 }

context_test.go

@@ -14,6 +14,7 @@ import (
 	"log/slog"
 	"math"
 	"mime/multipart"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -1220,91 +1221,42 @@ func TestContext_Bind(t *testing.T) {
 }
 
 func TestContext_RealIP(t *testing.T) {
-	tests := []struct {
-		c *Context
-		s string
+	_, ipv6ForRemoteAddrExternalRange, _ := net.ParseCIDR("2001:db8::/64")
+
+	var testCases = []struct {
+		name            string
+		givenIPExtrator IPExtractor
+		whenReq         *http.Request
+		expect          string
 	}{
 		{
-			&Context{
-				request: &http.Request{
-					Header: http.Header{HeaderXForwardedFor: []string{"127.0.0.1, 127.0.1.1, "}},
-				},
-			},
-			"127.0.0.1",
+			name:            "ip from remote addr",
+			givenIPExtrator: nil,
+			whenReq:         &http.Request{RemoteAddr: "89.89.89.89:1654"},
+			expect:          "89.89.89.89",
 		},
 		{
-			&Context{
-				request: &http.Request{
-					Header: http.Header{HeaderXForwardedFor: []string{"127.0.0.1,127.0.1.1"}},
-				},
-			},
-			"127.0.0.1",
-		},
-		{
-			&Context{
-				request: &http.Request{
-					Header: http.Header{HeaderXForwardedFor: []string{"127.0.0.1"}},
+			name:            "ip from ip extractor",
+			givenIPExtrator: ExtractIPFromRealIPHeader(TrustIPRange(ipv6ForRemoteAddrExternalRange)),
+			whenReq: &http.Request{
+				Header: http.Header{
+					HeaderXRealIP:       []string{"[2001:db8::113:199]"},
+					HeaderXForwardedFor: []string{"[2001:db8::113:198], [2001:db8::113:197]"}, // <-- should not affect anything
 				},
+				RemoteAddr: "[2001:db8::113:1]:8080",
 			},
-			"127.0.0.1",
-		},
-		{
-			&Context{
-				request: &http.Request{
-					Header: http.Header{HeaderXForwardedFor: []string{"[2001:db8:85a3:8d3:1319:8a2e:370:7348], 2001:db8::1, "}},
-				},
-			},
-			"2001:db8:85a3:8d3:1319:8a2e:370:7348",
-		},
-		{
-			&Context{
-				request: &http.Request{
-					Header: http.Header{HeaderXForwardedFor: []string{"[2001:db8:85a3:8d3:1319:8a2e:370:7348],[2001:db8::1]"}},
-				},
-			},
-			"2001:db8:85a3:8d3:1319:8a2e:370:7348",
-		},
-		{
-			&Context{
-				request: &http.Request{
-					Header: http.Header{HeaderXForwardedFor: []string{"2001:db8:85a3:8d3:1319:8a2e:370:7348"}},
-				},
-			},
-			"2001:db8:85a3:8d3:1319:8a2e:370:7348",
-		},
-		{
-			&Context{
-				request: &http.Request{
-					Header: http.Header{
-						"X-Real-Ip": []string{"192.168.0.1"},
-					},
-				},
-			},
-			"192.168.0.1",
-		},
-		{
-			&Context{
-				request: &http.Request{
-					Header: http.Header{
-						"X-Real-Ip": []string{"[2001:db8::1]"},
-					},
-				},
-			},
-			"2001:db8::1",
-		},
-
-		{
-			&Context{
-				request: &http.Request{
-					RemoteAddr: "89.89.89.89:1654",
-				},
-			},
-			"89.89.89.89",
+			expect: "2001:db8::113:199",
 		},
 	}
-
-	for _, tt := range tests {
-		assert.Equal(t, tt.s, tt.c.RealIP())
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := New()
+			c := e.NewContext(tc.whenReq, nil)
+			if tc.givenIPExtrator != nil {
+				e.IPExtractor = tc.givenIPExtrator
+			}
+			assert.Equal(t, tc.expect, c.RealIP())
+		})
 	}
 }
 

ip.go

@@ -202,8 +202,8 @@ func (c *ipChecker) trust(ip net.IP) bool {
 // See https://echo.labstack.com/guide/ip-address for more details.
 type IPExtractor func(*http.Request) string
 
-// ExtractIPDirect extracts IP address using actual IP address.
-// Use this if your server faces to internet directory (i.e.: uses no proxy).
+// ExtractIPDirect extracts an IP address using an actual IP address.
+// Use this if your server faces to internet directly (i.e.: uses no proxy).
 func ExtractIPDirect() IPExtractor {
 	return extractIP
 }
@@ -219,7 +219,7 @@ func extractIP(req *http.Request) string {
 	return host
 }
 
-// ExtractIPFromRealIPHeader extracts IP address using x-real-ip header.
+// ExtractIPFromRealIPHeader extracts IP address using `x-real-ip` header.
 // Use this if you put proxy which uses this header.
 func ExtractIPFromRealIPHeader(options ...TrustOption) IPExtractor {
 	checker := newIPChecker(options)
@@ -236,7 +236,7 @@ func ExtractIPFromRealIPHeader(options ...TrustOption) IPExtractor {
 	}
 }
 
-// ExtractIPFromXFFHeader extracts IP address using x-forwarded-for header.
+// ExtractIPFromXFFHeader extracts IP address using `x-forwarded-for` header.
 // Use this if you put proxy which uses this header.
 // This returns nearest untrustable IP. If all IPs are trustable, returns furthest one (i.e.: XFF[0]).
 func ExtractIPFromXFFHeader(options ...TrustOption) IPExtractor {
@@ -265,3 +265,45 @@ func ExtractIPFromXFFHeader(options ...TrustOption) IPExtractor {
 		return strings.TrimSpace(ips[0])
 	}
 }
+
+// LegacyIPExtractor returns an IPExtractor that derives the client IP address
+// from common proxy headers, falling back to the request's remote address.
+//
+// Resolution order:
+//  1. X-Forwarded-For: returns the first IP in the comma-separated list.
+//     If multiple values are present, only the left-most (original client)
+//     is used. Surrounding brackets (for IPv6) are stripped.
+//  2. X-Real-IP: used if X-Forwarded-For is absent. Surrounding brackets
+//     (for IPv6) are stripped.
+//  3. req.RemoteAddr: used as a fallback; the host portion is extracted
+//     via net.SplitHostPort.
+//
+// Notes:
+//   - No validation is performed on header values.
+//   - This function trusts headers as-is and is therefore not safe against
+//     spoofing unless the application is behind a trusted proxy that is
+//     configured to strip/replace/modify headers correctly.
+//
+// Use ExtractIPFromXFFHeader or ExtractIPFromRealIPHeader instead of LegacyIPExtractor.
+func LegacyIPExtractor() IPExtractor {
+	return legacyIPExtractor
+}
+
+func legacyIPExtractor(req *http.Request) string {
+	if ip := req.Header.Get(HeaderXForwardedFor); ip != "" {
+		i := strings.IndexAny(ip, ",")
+		if i > 0 {
+			ip = strings.TrimSpace(ip[:i])
+		}
+		ip = strings.TrimPrefix(ip, "[")
+		ip = strings.TrimSuffix(ip, "]")
+		return ip
+	}
+	if ip := req.Header.Get(HeaderXRealIP); ip != "" {
+		ip = strings.TrimPrefix(ip, "[")
+		ip = strings.TrimSuffix(ip, "]")
+		return ip
+	}
+	ra, _, _ := net.SplitHostPort(req.RemoteAddr)
+	return ra
+}

ip_test.go

@@ -714,3 +714,75 @@ func TestExtractIPFromXFFHeader(t *testing.T) {
 		})
 	}
 }
+
+func TestLegacyIPExtractor(t *testing.T) {
+	var testCases = []struct {
+		name          string
+		whenReq       *http.Request
+		expect        string
+		expectedError string
+	}{
+		{
+			name:    "extract first ip from X-Forwarded-For",
+			whenReq: &http.Request{Header: http.Header{"X-Forwarded-For": []string{"203.0.113.10, 198.51.100.7"}}},
+			expect:  "203.0.113.10",
+		},
+		{
+			name:    "extract single ip from X-Forwarded-For",
+			whenReq: &http.Request{Header: http.Header{"X-Forwarded-For": []string{"203.0.113.10"}}},
+			expect:  "203.0.113.10",
+		},
+		{
+			name:    "trim brackets from ipv6 in X-Forwarded-For when multiple values",
+			whenReq: &http.Request{Header: http.Header{"X-Forwarded-For": []string{"[2001:db8::1], 198.51.100.7"}}},
+			expect:  "2001:db8::1",
+		},
+		{
+			name: "prefer X-Forwarded-For over X-Real-Ip",
+			whenReq: &http.Request{
+				Header: http.Header{
+					"X-Forwarded-For": []string{"203.0.113.10"},
+					"X-Real-Ip":       []string{"198.51.100.7"},
+				},
+			},
+			expect: "203.0.113.10",
+		},
+		{
+			name:    "extract from X-Real-Ip",
+			whenReq: &http.Request{Header: http.Header{"X-Real-Ip": []string{"[2001:db8::1]"}}},
+			expect:  "2001:db8::1",
+		},
+		{
+			name:    "extract plain ipv4 from X-Real-Ip",
+			whenReq: &http.Request{Header: http.Header{"X-Real-Ip": []string{"203.0.113.10"}}},
+			expect:  "203.0.113.10",
+		},
+		{
+			name:    "fallback to RemoteAddr host",
+			whenReq: &http.Request{RemoteAddr: "203.0.113.10:12345"},
+			expect:  "203.0.113.10",
+		},
+		{
+			name:    "fallback to RemoteAddr ipv6 host",
+			whenReq: &http.Request{RemoteAddr: "[2001:db8::1]:12345"},
+			expect:  "2001:db8::1",
+		},
+		{
+			name:    "returns empty string when RemoteAddr is invalid and no headers exist",
+			whenReq: &http.Request{RemoteAddr: "not-a-host-port"},
+			expect:  "",
+		},
+		{
+			name:    "trim brackets from single ipv6 in X-Forwarded-For",
+			whenReq: &http.Request{Header: http.Header{"X-Forwarded-For": []string{"[2001:db8::1]"}}},
+			expect:  "2001:db8::1",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ip := LegacyIPExtractor()(tc.whenReq)
+			assert.Equal(t, tc.expect, ip)
+		})
+	}
+}

middleware/request_logger_test.go

@@ -27,6 +27,7 @@ func TestRequestLoggerOK(t *testing.T) {
 	})
 
 	e := echo.New()
+	e.IPExtractor = echo.LegacyIPExtractor()
 	buf := new(bytes.Buffer)
 	e.Logger = slog.New(slog.NewJSONHandler(buf, nil))
 	e.Use(RequestLogger())
@@ -439,7 +440,7 @@ func TestRequestLogger_allFields(t *testing.T) {
 	assert.Equal(t, time.Unix(1631045377, 0), expect.StartTime)
 	assert.Equal(t, 10*time.Second, expect.Latency)
 	assert.Equal(t, "HTTP/1.1", expect.Protocol)
-	assert.Equal(t, "8.8.8.8", expect.RemoteIP)
+	assert.Equal(t, "192.0.2.1", expect.RemoteIP)
 	assert.Equal(t, "example.com", expect.Host)
 	assert.Equal(t, http.MethodPost, expect.Method)
 	assert.Equal(t, "/test?lang=en&checked=1&checked=2", expect.URI)
@@ -530,7 +531,7 @@ func TestTestRequestLogger(t *testing.T) {
 			assert.Contains(t, string(rawlog), `"uri":"/test?lang=en&checked=1&checked=2"`)
 			assert.Contains(t, string(rawlog), `"latency":`) // this value varies
 			assert.Contains(t, string(rawlog), `"request_id":"MY_ID"`)
-			assert.Contains(t, string(rawlog), `"remote_ip":"8.8.8.8"`)
+			assert.Contains(t, string(rawlog), `"remote_ip":"192.0.2.1"`)
 			assert.Contains(t, string(rawlog), `"host":"example.com"`)
 			assert.Contains(t, string(rawlog), `"user_agent":"curl/7.68.0"`)
 			assert.Contains(t, string(rawlog), `"bytes_in":"32"`)