From 08dd235dc4905a4d56d8ca21b849bf3893d5af0b Mon Sep 17 00:00:00 2001 From: yiranzai Date: Thu, 15 Apr 2021 14:01:49 +0800 Subject: [PATCH 1/5] set engine.TrustedProxies For items that don't use gin.RUN --- gin.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/gin.go b/gin.go index 03a0e127e6..7d6b25cd54 100644 --- a/gin.go +++ b/gin.go @@ -326,11 +326,11 @@ func iterate(path, method string, routes RoutesInfo, root *node) RoutesInfo { func (engine *Engine) Run(addr ...string) (err error) { defer func() { debugPrintError(err) }() - trustedCIDRs, err := engine.prepareTrustedCIDRs() + err = engine.parseTrustedProxies() if err != nil { return err } - engine.trustedCIDRs = trustedCIDRs + address := resolveAddress(addr) debugPrint("Listening and serving HTTP on %s\n", address) err = http.ListenAndServe(address, engine) @@ -366,6 +366,23 @@ func (engine *Engine) prepareTrustedCIDRs() ([]*net.IPNet, error) { return cidr, nil } +// SetTrustedProxies set Engine.TrustedProxies +func (engine *Engine) SetTrustedProxies(trustedProxies []string) error { + engine.ForwardedByClientIP = true + engine.TrustedProxies = trustedProxies + return engine.parseTrustedProxies() +} + +// parseTrustedProxies parse Engine.TrustedProxies to Engine.trustedCIDRs +func (engine *Engine) parseTrustedProxies() error { + trustedCIDRs, err := engine.prepareTrustedCIDRs() + if err != nil { + return err + } + engine.trustedCIDRs = trustedCIDRs + return nil +} + // parseIP parse a string representation of an IP and returns a net.IP with the // minimum byte representation or nil if input is invalid. func parseIP(ip string) net.IP { From 936c45de44c39afc695b064416fd24cc45432401 Mon Sep 17 00:00:00 2001 From: yiranzai Date: Wed, 21 Apr 2021 10:05:59 +0800 Subject: [PATCH 2/5] context_test TestContextClientIP --- context_test.go | 36 +++++++++++------------------------- gin.go | 7 ++----- 2 files changed, 13 insertions(+), 30 deletions(-) diff --git a/context_test.go b/context_test.go index 993c632f74..aaa358e9d0 100644 --- a/context_test.go +++ b/context_test.go @@ -1392,14 +1392,10 @@ func TestContextAbortWithError(t *testing.T) { assert.True(t, c.IsAborted()) } -func resetTrustedCIDRs(c *Context) { - c.engine.trustedCIDRs, _ = c.engine.prepareTrustedCIDRs() -} - func TestContextClientIP(t *testing.T) { c, _ := CreateTestContext(httptest.NewRecorder()) c.Request, _ = http.NewRequest("POST", "/", nil) - resetTrustedCIDRs(c) + c.engine.trustedCIDRs, _ = c.engine.prepareTrustedCIDRs() resetContextForClientIPTests(c) // Legacy tests (validating that the defaults don't break the @@ -1428,57 +1424,47 @@ func TestContextClientIP(t *testing.T) { resetContextForClientIPTests(c) // No trusted proxies - c.engine.TrustedProxies = []string{} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{}) c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"} assert.Equal(t, "40.40.40.40", c.ClientIP()) // Last proxy is trusted, but the RemoteAddr is not - c.engine.TrustedProxies = []string{"30.30.30.30"} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{"30.30.30.30"}) assert.Equal(t, "40.40.40.40", c.ClientIP()) // Only trust RemoteAddr - c.engine.TrustedProxies = []string{"40.40.40.40"} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{"40.40.40.40"}) assert.Equal(t, "20.20.20.20", c.ClientIP()) // All steps are trusted - c.engine.TrustedProxies = []string{"40.40.40.40", "30.30.30.30", "20.20.20.20"} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{"40.40.40.40", "30.30.30.30", "20.20.20.20"}) assert.Equal(t, "20.20.20.20", c.ClientIP()) // Use CIDR - c.engine.TrustedProxies = []string{"40.40.25.25/16", "30.30.30.30"} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{"40.40.25.25/16", "30.30.30.30"}) assert.Equal(t, "20.20.20.20", c.ClientIP()) // Use hostname that resolves to all the proxies - c.engine.TrustedProxies = []string{"foo"} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{"foo"}) assert.Equal(t, "40.40.40.40", c.ClientIP()) // Use hostname that returns an error - c.engine.TrustedProxies = []string{"bar"} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{"bar"}) assert.Equal(t, "40.40.40.40", c.ClientIP()) // X-Forwarded-For has a non-IP element - c.engine.TrustedProxies = []string{"40.40.40.40"} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{"40.40.40.40"}) c.Request.Header.Set("X-Forwarded-For", " blah ") assert.Equal(t, "40.40.40.40", c.ClientIP()) // Result from LookupHost has non-IP element. This should never // happen, but we should test it to make sure we handle it // gracefully. - c.engine.TrustedProxies = []string{"baz"} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{"baz"}) c.Request.Header.Set("X-Forwarded-For", " 30.30.30.30 ") assert.Equal(t, "40.40.40.40", c.ClientIP()) - c.engine.TrustedProxies = []string{"40.40.40.40"} - resetTrustedCIDRs(c) + _ = c.engine.SetTrustedProxies([]string{"40.40.40.40"}) c.Request.Header.Del("X-Forwarded-For") c.engine.RemoteIPHeaders = []string{"X-Forwarded-For", "X-Real-IP"} assert.Equal(t, "10.10.10.10", c.ClientIP()) diff --git a/gin.go b/gin.go index 7d6b25cd54..77e52a6359 100644 --- a/gin.go +++ b/gin.go @@ -368,7 +368,7 @@ func (engine *Engine) prepareTrustedCIDRs() ([]*net.IPNet, error) { // SetTrustedProxies set Engine.TrustedProxies func (engine *Engine) SetTrustedProxies(trustedProxies []string) error { - engine.ForwardedByClientIP = true + //engine.ForwardedByClientIP = true engine.TrustedProxies = trustedProxies return engine.parseTrustedProxies() } @@ -376,11 +376,8 @@ func (engine *Engine) SetTrustedProxies(trustedProxies []string) error { // parseTrustedProxies parse Engine.TrustedProxies to Engine.trustedCIDRs func (engine *Engine) parseTrustedProxies() error { trustedCIDRs, err := engine.prepareTrustedCIDRs() - if err != nil { - return err - } engine.trustedCIDRs = trustedCIDRs - return nil + return err } // parseIP parse a string representation of an IP and returns a net.IP with the From 824f8ddc319d7f788dac4626743ce85a2c2f396c Mon Sep 17 00:00:00 2001 From: yiranzai Date: Fri, 7 May 2021 19:54:23 +0800 Subject: [PATCH 3/5] add engine.parseTrustedProxies to all Run* methods --- gin.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/gin.go b/gin.go index 77e52a6359..56e5c76848 100644 --- a/gin.go +++ b/gin.go @@ -368,7 +368,6 @@ func (engine *Engine) prepareTrustedCIDRs() ([]*net.IPNet, error) { // SetTrustedProxies set Engine.TrustedProxies func (engine *Engine) SetTrustedProxies(trustedProxies []string) error { - //engine.ForwardedByClientIP = true engine.TrustedProxies = trustedProxies return engine.parseTrustedProxies() } @@ -401,6 +400,11 @@ func (engine *Engine) RunTLS(addr, certFile, keyFile string) (err error) { debugPrint("Listening and serving HTTPS on %s\n", addr) defer func() { debugPrintError(err) }() + err = engine.parseTrustedProxies() + if err != nil { + return err + } + err = http.ListenAndServeTLS(addr, certFile, keyFile, engine) return } @@ -412,6 +416,11 @@ func (engine *Engine) RunUnix(file string) (err error) { debugPrint("Listening and serving HTTP on unix:/%s", file) defer func() { debugPrintError(err) }() + err = engine.parseTrustedProxies() + if err != nil { + return err + } + listener, err := net.Listen("unix", file) if err != nil { return @@ -430,6 +439,11 @@ func (engine *Engine) RunFd(fd int) (err error) { debugPrint("Listening and serving HTTP on fd@%d", fd) defer func() { debugPrintError(err) }() + err = engine.parseTrustedProxies() + if err != nil { + return err + } + f := os.NewFile(uintptr(fd), fmt.Sprintf("fd@%d", fd)) listener, err := net.FileListener(f) if err != nil { @@ -445,6 +459,12 @@ func (engine *Engine) RunFd(fd int) (err error) { func (engine *Engine) RunListener(listener net.Listener) (err error) { debugPrint("Listening and serving HTTP on listener what's bind with address@%s", listener.Addr()) defer func() { debugPrintError(err) }() + + err = engine.parseTrustedProxies() + if err != nil { + return err + } + err = http.Serve(listener, engine) return } From 3496c8aaf9ed2bb08fe90a09adc6526d687942e9 Mon Sep 17 00:00:00 2001 From: yiranzai Date: Fri, 7 May 2021 20:23:54 +0800 Subject: [PATCH 4/5] unit test --- gin_integration_test.go | 63 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/gin_integration_test.go b/gin_integration_test.go index fd972657cc..943a771101 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -55,13 +55,74 @@ func TestRunEmpty(t *testing.T) { testRequest(t, "http://localhost:8080/example") } -func TestTrustedCIDRsForRun(t *testing.T) { +func TestBadTrustedCIDRsForRun(t *testing.T) { os.Setenv("PORT", "") router := New() router.TrustedProxies = []string{"hello/world"} assert.Error(t, router.Run(":8080")) } +func TestBadTrustedCIDRsForRunUnix(t *testing.T){ + router := New() + router.TrustedProxies = []string{"hello/world"} + + unixTestSocket := filepath.Join(os.TempDir(), "unix_unit_test") + + defer os.Remove(unixTestSocket) + + go func() { + router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") }) + assert.Error(t, router.RunUnix(unixTestSocket)) + }() + // have to wait for the goroutine to start and run the server + // otherwise the main thread will complete + time.Sleep(5 * time.Millisecond) +} + +func TestBadTrustedCIDRsForRunFd(t *testing.T){ + router := New() + router.TrustedProxies = []string{"hello/world"} + + addr, err := net.ResolveTCPAddr("tcp", "localhost:0") + assert.NoError(t, err) + listener, err := net.ListenTCP("tcp", addr) + assert.NoError(t, err) + socketFile, err := listener.File() + assert.NoError(t, err) + + go func() { + router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") }) + assert.Error(t, router.RunFd(int(socketFile.Fd()))) + }() + // have to wait for the goroutine to start and run the server + // otherwise the main thread will complete + time.Sleep(5 * time.Millisecond) +} + +func TestBadTrustedCIDRsForRunListener(t *testing.T){ + router := New() + router.TrustedProxies = []string{"hello/world"} + + addr, err := net.ResolveTCPAddr("tcp", "localhost:0") + assert.NoError(t, err) + listener, err := net.ListenTCP("tcp", addr) + assert.NoError(t, err) + go func() { + router.GET("/example", func(c *Context) { c.String(http.StatusOK, "it worked") }) + assert.Error(t, router.RunListener(listener)) + }() + // have to wait for the goroutine to start and run the server + // otherwise the main thread will complete + time.Sleep(5 * time.Millisecond) +} + +func TestBadTrustedCIDRsForRunTLS(t *testing.T) { + os.Setenv("PORT", "") + router := New() + router.TrustedProxies = []string{"hello/world"} + assert.Error(t, router.RunTLS(":8080", "./testdata/certificate/cert.pem", "./testdata/certificate/key.pem")) +} + func TestRunTLS(t *testing.T) { router := New() go func() { From 126f8aa6af3b8fbd50d8a3ebe9ac725014a1eeaa Mon Sep 17 00:00:00 2001 From: yiranzai Date: Fri, 7 May 2021 20:33:54 +0800 Subject: [PATCH 5/5] code style fixed --- gin_integration_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gin_integration_test.go b/gin_integration_test.go index 943a771101..2eb2d52bf8 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -62,7 +62,7 @@ func TestBadTrustedCIDRsForRun(t *testing.T) { assert.Error(t, router.Run(":8080")) } -func TestBadTrustedCIDRsForRunUnix(t *testing.T){ +func TestBadTrustedCIDRsForRunUnix(t *testing.T) { router := New() router.TrustedProxies = []string{"hello/world"} @@ -79,7 +79,7 @@ func TestBadTrustedCIDRsForRunUnix(t *testing.T){ time.Sleep(5 * time.Millisecond) } -func TestBadTrustedCIDRsForRunFd(t *testing.T){ +func TestBadTrustedCIDRsForRunFd(t *testing.T) { router := New() router.TrustedProxies = []string{"hello/world"} @@ -99,7 +99,7 @@ func TestBadTrustedCIDRsForRunFd(t *testing.T){ time.Sleep(5 * time.Millisecond) } -func TestBadTrustedCIDRsForRunListener(t *testing.T){ +func TestBadTrustedCIDRsForRunListener(t *testing.T) { router := New() router.TrustedProxies = []string{"hello/world"}