fix: data race with trustedCIDRs (#2674) (#2675)

Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
This commit is contained in:
Xudong Cai 2021-04-09 00:27:34 +08:00 committed by GitHub
parent d496f64540
commit 03e5e05ae0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 17 additions and 3 deletions

View File

@ -767,8 +767,6 @@ func (c *Context) RemoteIP() (net.IP, bool) {
return nil, false return nil, false
} }
trustedCIDRs, _ := c.engine.prepareTrustedCIDRs()
c.engine.trustedCIDRs = trustedCIDRs
if c.engine.trustedCIDRs != nil { if c.engine.trustedCIDRs != nil {
for _, cidr := range c.engine.trustedCIDRs { for _, cidr := range c.engine.trustedCIDRs {
if cidr.Contains(remoteIP) { if cidr.Contains(remoteIP) {

View File

@ -1388,10 +1388,14 @@ func TestContextAbortWithError(t *testing.T) {
assert.True(t, c.IsAborted()) assert.True(t, c.IsAborted())
} }
func resetTrustedCIDRs(c *Context) {
c.engine.trustedCIDRs, _ = c.engine.prepareTrustedCIDRs()
}
func TestContextClientIP(t *testing.T) { func TestContextClientIP(t *testing.T) {
c, _ := CreateTestContext(httptest.NewRecorder()) c, _ := CreateTestContext(httptest.NewRecorder())
c.Request, _ = http.NewRequest("POST", "/", nil) c.Request, _ = http.NewRequest("POST", "/", nil)
resetTrustedCIDRs(c)
resetContextForClientIPTests(c) resetContextForClientIPTests(c)
// Legacy tests (validating that the defaults don't break the // Legacy tests (validating that the defaults don't break the
@ -1421,35 +1425,43 @@ func TestContextClientIP(t *testing.T) {
// No trusted proxies // No trusted proxies
c.engine.TrustedProxies = []string{} c.engine.TrustedProxies = []string{}
resetTrustedCIDRs(c)
c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"} c.engine.RemoteIPHeaders = []string{"X-Forwarded-For"}
assert.Equal(t, "40.40.40.40", c.ClientIP()) assert.Equal(t, "40.40.40.40", c.ClientIP())
// Last proxy is trusted, but the RemoteAddr is not // Last proxy is trusted, but the RemoteAddr is not
c.engine.TrustedProxies = []string{"30.30.30.30"} c.engine.TrustedProxies = []string{"30.30.30.30"}
resetTrustedCIDRs(c)
assert.Equal(t, "40.40.40.40", c.ClientIP()) assert.Equal(t, "40.40.40.40", c.ClientIP())
// Only trust RemoteAddr // Only trust RemoteAddr
c.engine.TrustedProxies = []string{"40.40.40.40"} c.engine.TrustedProxies = []string{"40.40.40.40"}
resetTrustedCIDRs(c)
assert.Equal(t, "20.20.20.20", c.ClientIP()) assert.Equal(t, "20.20.20.20", c.ClientIP())
// All steps are trusted // All steps are trusted
c.engine.TrustedProxies = []string{"40.40.40.40", "30.30.30.30", "20.20.20.20"} c.engine.TrustedProxies = []string{"40.40.40.40", "30.30.30.30", "20.20.20.20"}
resetTrustedCIDRs(c)
assert.Equal(t, "20.20.20.20", c.ClientIP()) assert.Equal(t, "20.20.20.20", c.ClientIP())
// Use CIDR // Use CIDR
c.engine.TrustedProxies = []string{"40.40.25.25/16", "30.30.30.30"} c.engine.TrustedProxies = []string{"40.40.25.25/16", "30.30.30.30"}
resetTrustedCIDRs(c)
assert.Equal(t, "20.20.20.20", c.ClientIP()) assert.Equal(t, "20.20.20.20", c.ClientIP())
// Use hostname that resolves to all the proxies // Use hostname that resolves to all the proxies
c.engine.TrustedProxies = []string{"foo"} c.engine.TrustedProxies = []string{"foo"}
resetTrustedCIDRs(c)
assert.Equal(t, "40.40.40.40", c.ClientIP()) assert.Equal(t, "40.40.40.40", c.ClientIP())
// Use hostname that returns an error // Use hostname that returns an error
c.engine.TrustedProxies = []string{"bar"} c.engine.TrustedProxies = []string{"bar"}
resetTrustedCIDRs(c)
assert.Equal(t, "40.40.40.40", c.ClientIP()) assert.Equal(t, "40.40.40.40", c.ClientIP())
// X-Forwarded-For has a non-IP element // X-Forwarded-For has a non-IP element
c.engine.TrustedProxies = []string{"40.40.40.40"} c.engine.TrustedProxies = []string{"40.40.40.40"}
resetTrustedCIDRs(c)
c.Request.Header.Set("X-Forwarded-For", " blah ") c.Request.Header.Set("X-Forwarded-For", " blah ")
assert.Equal(t, "40.40.40.40", c.ClientIP()) assert.Equal(t, "40.40.40.40", c.ClientIP())
@ -1457,10 +1469,12 @@ func TestContextClientIP(t *testing.T) {
// happen, but we should test it to make sure we handle it // happen, but we should test it to make sure we handle it
// gracefully. // gracefully.
c.engine.TrustedProxies = []string{"baz"} c.engine.TrustedProxies = []string{"baz"}
resetTrustedCIDRs(c)
c.Request.Header.Set("X-Forwarded-For", " 30.30.30.30 ") c.Request.Header.Set("X-Forwarded-For", " 30.30.30.30 ")
assert.Equal(t, "40.40.40.40", c.ClientIP()) assert.Equal(t, "40.40.40.40", c.ClientIP())
c.engine.TrustedProxies = []string{"40.40.40.40"} c.engine.TrustedProxies = []string{"40.40.40.40"}
resetTrustedCIDRs(c)
c.Request.Header.Del("X-Forwarded-For") c.Request.Header.Del("X-Forwarded-For")
c.engine.RemoteIPHeaders = []string{"X-Forwarded-For", "X-Real-IP"} c.engine.RemoteIPHeaders = []string{"X-Forwarded-For", "X-Real-IP"}
assert.Equal(t, "10.10.10.10", c.ClientIP()) assert.Equal(t, "10.10.10.10", c.ClientIP())

View File

@ -185,6 +185,8 @@ func TestLoggerWithConfigFormatting(t *testing.T) {
buffer := new(bytes.Buffer) buffer := new(bytes.Buffer)
router := New() router := New()
router.engine.trustedCIDRs, _ = router.engine.prepareTrustedCIDRs()
router.Use(LoggerWithConfig(LoggerConfig{ router.Use(LoggerWithConfig(LoggerConfig{
Output: buffer, Output: buffer,
Formatter: func(param LogFormatterParams) string { Formatter: func(param LogFormatterParams) string {