Skip to content

Commit 2dc3c02

Browse files
fix(engine): accept bracketed IPv6 and port-suffixed entries in X-Forwarded-For
`validateHeader` called `net.ParseIP` directly on each comma-split item, so anything with brackets or a `:port` suffix got rejected silently and `ClientIP()` fell through to `RemoteAddr` — which means a client coming in through IIS/ARR or certain cloud LBs would show up as the reverse proxy instead of the real caller. The four forms called out in #4572 are all normal real-world outputs: - "192.168.8.39" - "240e:318:2f4a:de56::240" - "[240e:318:2f4a:de56::240]" - "192.168.8.39:38792" - "[240e:318:2f4a:de56::240]:38792" Extract a small `parseForwardedForItem` helper that tries `net.SplitHostPort` first (handles the two `:port` variants and strips brackets in the process) and falls back to bracket-stripping + `net.ParseIP` for bare `[ipv6]`. The returned `clientIP` is now always the bare IP regardless of which proxy produced the header, which keeps the shape of `ClientIP()` stable. Table tests cover all four reporter-listed forms, plus a chain with a port on the last entry and a couple of garbage inputs. Closes #4572
1 parent d3ffc99 commit 2dc3c02

2 files changed

Lines changed: 63 additions & 2 deletions

File tree

gin.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -485,8 +485,8 @@ func (engine *Engine) validateHeader(header string) (clientIP string, valid bool
485485
}
486486
items := strings.Split(header, ",")
487487
for i := len(items) - 1; i >= 0; i-- {
488-
ipStr := strings.TrimSpace(items[i])
489-
ip := net.ParseIP(ipStr)
488+
item := strings.TrimSpace(items[i])
489+
ipStr, ip := parseForwardedForItem(item)
490490
if ip == nil {
491491
break
492492
}
@@ -500,6 +500,35 @@ func (engine *Engine) validateHeader(header string) (clientIP string, valid bool
500500
return "", false
501501
}
502502

503+
// parseForwardedForItem normalizes a single X-Forwarded-For entry and parses it.
504+
// It accepts the four common forms emitted by reverse proxies:
505+
//
506+
// - "1.2.3.4"
507+
// - "2001:db8::1"
508+
// - "[2001:db8::1]" (IIS/ARR style)
509+
// - "1.2.3.4:12345" (with port, some LBs)
510+
// - "[2001:db8::1]:12345" (IIS/ARR + port)
511+
//
512+
// The returned string is the IP without brackets or port, so callers see a
513+
// consistent form regardless of which proxy produced the header.
514+
func parseForwardedForItem(item string) (string, net.IP) {
515+
// Try host:port form first (handles "ip:port" and "[ipv6]:port").
516+
if host, _, err := net.SplitHostPort(item); err == nil {
517+
if ip := net.ParseIP(host); ip != nil {
518+
return host, ip
519+
}
520+
}
521+
// Strip optional surrounding brackets for bare "[ipv6]" with no port.
522+
unbracketed := item
523+
if strings.HasPrefix(unbracketed, "[") && strings.HasSuffix(unbracketed, "]") {
524+
unbracketed = unbracketed[1 : len(unbracketed)-1]
525+
}
526+
if ip := net.ParseIP(unbracketed); ip != nil {
527+
return unbracketed, ip
528+
}
529+
return "", nil
530+
}
531+
503532
// updateRouteTree do update to the route tree recursively
504533
func updateRouteTree(n *node) {
505534
n.path = strings.ReplaceAll(n.path, escapedColon, colon)

gin_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,3 +1156,35 @@ func TestUpdateRouteTreesCalledOnce(t *testing.T) {
11561156
assert.Equal(t, "ok", w.Body.String())
11571157
}
11581158
}
1159+
1160+
func TestValidateHeaderForwardedForForms(t *testing.T) {
1161+
engine := New()
1162+
// Disable trusted proxies so the rightmost parseable entry is returned.
1163+
require.NoError(t, engine.SetTrustedProxies(nil))
1164+
1165+
tests := []struct {
1166+
name string
1167+
header string
1168+
wantIP string
1169+
wantOK bool
1170+
}{
1171+
{"plain IPv4", "192.168.8.39", "192.168.8.39", true},
1172+
{"plain IPv6", "240e:318:2f4a:de56::240", "240e:318:2f4a:de56::240", true},
1173+
{"bracketed IPv6 (IIS/ARR)", "[240e:318:2f4a:de56::240]", "240e:318:2f4a:de56::240", true},
1174+
{"IPv4 with port", "192.168.8.39:38792", "192.168.8.39", true},
1175+
{"bracketed IPv6 with port", "[240e:318:2f4a:de56::240]:38792", "240e:318:2f4a:de56::240", true},
1176+
{"IPv6 loopback bracketed", "[::1]", "::1", true},
1177+
{"chain with port on last entry", "1.2.3.4, 5.6.7.8:9000", "5.6.7.8", true},
1178+
{"empty", "", "", false},
1179+
{"garbage", "not-an-ip", "", false},
1180+
{"bracketed garbage", "[not-an-ip]", "", false},
1181+
}
1182+
1183+
for _, tt := range tests {
1184+
t.Run(tt.name, func(t *testing.T) {
1185+
gotIP, gotOK := engine.validateHeader(tt.header)
1186+
assert.Equal(t, tt.wantOK, gotOK)
1187+
assert.Equal(t, tt.wantIP, gotIP)
1188+
})
1189+
}
1190+
}

0 commit comments

Comments
 (0)