Merge 6e891029bf1128e0845ce64a111bd35f0682dcce into 9914178584e42458ff7d23891463a880f58c9d86

* Fix ClientIP calculation by concatenating all RemoteIPHeaders values

* test: used http.MethodGet instead constants and fix lints

* lint error fixed

* Refactor ClientIP X-Forwarded-For tests

---------

Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>

context.go

@@ -989,7 +989,8 @@ func (c *Context) ClientIP() string {
 
 	if trusted && c.engine.ForwardedByClientIP && c.engine.RemoteIPHeaders != nil {
 		for _, headerName := range c.engine.RemoteIPHeaders {
-			ip, valid := c.engine.validateHeader(c.requestHeader(headerName))
+			headerValue := strings.Join(c.Request.Header.Values(headerName), ",")
+			ip, valid := c.engine.validateHeader(headerValue)
 			if valid {
 				return ip
 			}
@@ -1394,16 +1395,19 @@ func (c *Context) SetAccepted(formats ...string) {
 /***** GOLANG.ORG/X/NET/CONTEXT *****/
 /************************************/
 
-// hasRequestContext returns whether c.Request has Context and fallback.
+// hasFallback returns whether c.engine has ContextWithFallback.
+func (c *Context) hasFallback() bool {
+	return c.engine != nil && c.engine.ContextWithFallback
+}
+
+// hasRequestContext returns whether c.Request has Context
 func (c *Context) hasRequestContext() bool {
-	hasFallback := c.engine != nil && c.engine.ContextWithFallback
-	hasRequestContext := c.Request != nil && c.Request.Context() != nil
-	return hasFallback && hasRequestContext
+	return c.Request != nil && c.Request.Context() != nil
 }
 
 // Deadline returns that there is no deadline (ok==false) when c.Request has no Context.
 func (c *Context) Deadline() (deadline time.Time, ok bool) {
-	if !c.hasRequestContext() {
+	if !c.hasFallback() || !c.hasRequestContext() {
 		return
 	}
 	return c.Request.Context().Deadline()
@@ -1411,7 +1415,7 @@ func (c *Context) Deadline() (deadline time.Time, ok bool) {
 
 // Done returns nil (chan which will wait forever) when c.Request has no Context.
 func (c *Context) Done() <-chan struct{} {
-	if !c.hasRequestContext() {
+	if !c.hasFallback() || !c.hasRequestContext() {
 		return nil
 	}
 	return c.Request.Context().Done()
@@ -1419,7 +1423,7 @@ func (c *Context) Done() <-chan struct{} {
 
 // Err returns nil when c.Request has no Context.
 func (c *Context) Err() error {
-	if !c.hasRequestContext() {
+	if !c.hasFallback() || !c.hasRequestContext() {
 		return nil
 	}
 	return c.Request.Context().Err()
@@ -1440,7 +1444,7 @@ func (c *Context) Value(key any) any {
 			return val
 		}
 	}
-	if !c.hasRequestContext() {
+	if !c.hasFallback() || !c.hasRequestContext() {
 		return nil
 	}
 	return c.Request.Context().Value(key)

context_test.go

@@ -1143,6 +1143,37 @@ func TestContextRenderNoContentIndentedJSON(t *testing.T) {
 	assert.Equal(t, "application/json; charset=utf-8", w.Header().Get("Content-Type"))
 }
 
+func TestContextClientIPWithMultipleHeaders(t *testing.T) {
+	c, _ := CreateTestContext(httptest.NewRecorder())
+	c.Request, _ = http.NewRequest(http.MethodGet, "/test", nil)
+
+	// Multiple X-Forwarded-For headers
+	c.Request.Header.Add("X-Forwarded-For", "1.2.3.4, "+localhostIP)
+	c.Request.Header.Add("X-Forwarded-For", "5.6.7.8")
+	c.Request.RemoteAddr = localhostIP + ":1234"
+
+	c.engine.ForwardedByClientIP = true
+	c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"}
+	_ = c.engine.SetTrustedProxies([]string{localhostIP})
+
+	// Should return 5.6.7.8 (last non-trusted IP)
+	assert.Equal(t, "5.6.7.8", c.ClientIP())
+}
+
+func TestContextClientIPWithSingleHeader(t *testing.T) {
+	c, _ := CreateTestContext(httptest.NewRecorder())
+	c.Request, _ = http.NewRequest(http.MethodGet, "/test", nil)
+	c.Request.Header.Set("X-Forwarded-For", "1.2.3.4, "+localhostIP)
+	c.Request.RemoteAddr = localhostIP + ":1234"
+
+	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())
+}
+
 // Tests that the response is serialized as Secure JSON
 // and Content-Type is set to application/json
 func TestContextRenderSecureJSON(t *testing.T) {
@@ -3023,20 +3054,38 @@ func TestRemoteIPFail(t *testing.T) {
 
 func TestHasRequestContext(t *testing.T) {
 	c, _ := CreateTestContext(httptest.NewRecorder())
-	assert.False(t, c.hasRequestContext(), "no request, no fallback")
+	assert.False(t, c.hasRequestContext())
 	c.engine.ContextWithFallback = true
-	assert.False(t, c.hasRequestContext(), "no request, has fallback")
+	assert.False(t, c.hasRequestContext())
 	c.Request, _ = http.NewRequest(http.MethodGet, "/", nil)
-	assert.True(t, c.hasRequestContext(), "has request, has fallback")
+	assert.True(t, c.hasRequestContext())
 	c.Request, _ = http.NewRequestWithContext(nil, "", "", nil) //nolint:staticcheck
-	assert.False(t, c.hasRequestContext(), "has request with nil ctx, has fallback")
+	assert.False(t, c.hasRequestContext())
 	c.engine.ContextWithFallback = false
-	assert.False(t, c.hasRequestContext(), "has request, no fallback")
+	assert.False(t, c.hasRequestContext())
 
 	c = &Context{}
-	assert.False(t, c.hasRequestContext(), "no request, no engine")
+	assert.False(t, c.hasRequestContext())
 	c.Request, _ = http.NewRequest(http.MethodGet, "/", nil)
-	assert.False(t, c.hasRequestContext(), "has request, no engine")
+	assert.True(t, c.hasRequestContext())
+}
+
+func TestHasFallback(t *testing.T) {
+	c, _ := CreateTestContext(httptest.NewRecorder())
+	assert.False(t, c.hasFallback())
+	c.engine.ContextWithFallback = true
+	assert.True(t, c.hasFallback())
+	c.Request, _ = http.NewRequest(http.MethodGet, "/", nil)
+	assert.True(t, c.hasFallback())
+	c.Request, _ = http.NewRequestWithContext(nil, "", "", nil) //nolint:staticcheck
+	assert.True(t, c.hasFallback())
+	c.engine.ContextWithFallback = false
+	assert.False(t, c.hasFallback())
+
+	c = &Context{}
+	assert.False(t, c.hasFallback())
+	c.Request, _ = http.NewRequest(http.MethodGet, "/", nil)
+	assert.False(t, c.hasFallback())
 }
 
 func TestContextWithFallbackDeadlineFromRequestContext(t *testing.T) {