fix(context): ClientIP handling for multiple X-Forwarded-For header values

[Nurysso]

Description:

This PR addresses the behavior described in issue #4468 by updating how ClientIP processes headers listed in RemoteIPHeaders.

When multiple headers with the same name (e.g. X-Forwarded-For) are present, Gin previously only considered the first header value via Header.Get. Per the HTTP specification, such headers must be treated as a single, ordered, comma-separated list.

This change joins all header values using Request.Header.Values(headerName) before validation, ensuring correct client IP resolution in multi-proxy scenarios.

Changes:

• Combine all values of headers in RemoteIPHeaders before passing them to validateHeader.
• Add tests covering multiple header values to ensure correct behavior.

Checklist:

• PR opened against master
• All CI tests pass
• Tests added to cover the updated behavior
• Documentation update not required (bug fix, not a new feature)

References:

• Fixes ClientIP from RemoteIPHeaders calculation should concatenate all header values #4468

[fxedel]

Looks good to me!

[anfaas1618]

please rebase or merge with master. this pr is behind

[anfaas1618]

Suggested change

	engine.SetTrustedProxies([]string{"127.0.0.1"})
	engine.SetTrustedProxies([]string{localhostIP})

[anfaas1618]

Suggested change

	engine.SetTrustedProxies([]string{"127.0.0.1"})
	engine.SetTrustedProxies([]string{localhostIP})

[Nurysso]

please rebase or merge with master. this pr is behind

done, also regarding the changes you requested with use of localhostIP instead of hardcoded 127.0.0.1 fails the test I wrote should I leave it or do something like this

func TestContextClientIPWithSingleHeader(t *testing.T) {
	c, _ := CreateTestContext(httptest.NewRecorder())
	c.Request, _ = http.NewRequest(http.MethodGet, "/test", nil)
	c.Request.Header.Set("X-Forwarded-For", fmt.Sprintf("1.2.3.4, %s", localhostIP))
	c.Request.RemoteAddr = fmt.Sprintf("%s:1234", localhostIP)
	
	c.engine.ForwardedByClientIP = true
	c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"}
	_ = c.engine.SetTrustedProxies([]string{localhostIP})
	
	// Should return 1.2.3.4
	assert.Equal(t, "1.2.3.4", c.ClientIP())
}

I used AI for this and not sure if this is the best way.

[appleboy]

@Nurysso, please take a look at the CI testing failure.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.99%. Comparing base (3dc1cd6) to head (dffc2e3).
⚠️ Report is 230 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4472      +/-   ##
==========================================
- Coverage   99.21%   98.99%   -0.22%     
==========================================
  Files          42       44       +2     
  Lines        3182     2988     -194     
==========================================
- Hits         3157     2958     -199     
- Misses         17       21       +4     
- Partials        8        9       +1     

Flag	Coverage Δ
	?
--ldflags="-checklinkname=0" -tags sonic	98.98% <100.00%> (?)
-tags go_json	98.92% <100.00%> (?)
-tags nomsgpack	98.98% <100.00%> (?)
go-1.18	?
go-1.19	?
go-1.20	?
go-1.21	?
go-1.24	98.99% <100.00%> (?)
go-1.25	98.99% <100.00%> (?)
macos-latest	98.99% <100.00%> (-0.22%)	⬇️
ubuntu-latest	98.99% <100.00%> (-0.22%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[appleboy]

@anfaas1618 Please help review again.

[anfaas1618]

please rebase or merge with master. this pr is behind

done, also regarding the changes you requested with use of localhostIP instead of hardcoded 127.0.0.1 fails the test I wrote should I leave it or do something like this

func TestContextClientIPWithSingleHeader(t *testing.T) {
	c, _ := CreateTestContext(httptest.NewRecorder())
	c.Request, _ = http.NewRequest(http.MethodGet, "/test", nil)
	c.Request.Header.Set("X-Forwarded-For", fmt.Sprintf("1.2.3.4, %s", localhostIP))
	c.Request.RemoteAddr = fmt.Sprintf("%s:1234", localhostIP)
	
	c.engine.ForwardedByClientIP = true
	c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"}
	_ = c.engine.SetTrustedProxies([]string{localhostIP})
	
	// Should return 1.2.3.4
	assert.Equal(t, "1.2.3.4", c.ClientIP())
}

I used AI for this and not sure if this is the best way.

this is good , you can use this, fmt.Sprintf is discouraged but since its used only for testing its ok to use this way, it will make sure we are correctly and consistently using the consts defined which is easier when writing or updating test cases.

[Nurysso]

this is good , you can use this, fmt.Sprintf is discouraged but since its used only for testing its ok to use this way, it will make sure we are correctly and consistently using the consts defined which is easier when writing or updating test cases.

Okay understood, but in this test golangci-lint fails cause Sprintf. Should i update it to use string concatenation?

[anfaas1618]

this is good , you can use this, fmt.Sprintf is discouraged but since its used only for testing its ok to use this way, it will make sure we are correctly and consistently using the consts defined which is easier when writing or updating test cases.

Okay understood, but in this test golangci-lint fails cause Sprintf. Should i update it to use string concatenation?

c.Request.Header.Set("X-Forwarded-For", "1.2.3.4, " + localhostIP)
c.Request.RemoteAddr = localhostIP + ":1234"

this should be fine here , why
no need for extra allocations or parsing

[appleboy]

@Nurysso @anfaas1618 Thanks All.