Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions context.go
Expand Up @@ -767,8 +767,6 @@ func (c *Context) RemoteIP() (net.IP, bool) {
return nil, false
}

trustedCIDRs, _ := c.engine.prepareTrustedCIDRs()
c.engine.trustedCIDRs = trustedCIDRs
if c.engine.trustedCIDRs != nil {
for _, cidr := range c.engine.trustedCIDRs {
if cidr.Contains(remoteIP) {
Expand Down
16 changes: 15 additions & 1 deletion context_test.go
Expand Up @@ -1388,10 +1388,14 @@ 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)
resetContextForClientIPTests(c)

// Legacy tests (validating that the defaults don't break the
Expand Down Expand Up @@ -1421,46 +1425,56 @@ func TestContextClientIP(t *testing.T) {

// No trusted proxies
c.engine.TrustedProxies = []string{}
resetTrustedCIDRs(c)
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)
assert.Equal(t, "40.40.40.40", c.ClientIP())

// Only trust RemoteAddr
c.engine.TrustedProxies = []string{"40.40.40.40"}
resetTrustedCIDRs(c)
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)
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)
assert.Equal(t, "20.20.20.20", c.ClientIP())

// Use hostname that resolves to all the proxies
c.engine.TrustedProxies = []string{"foo"}
resetTrustedCIDRs(c)
assert.Equal(t, "40.40.40.40", c.ClientIP())

// Use hostname that returns an error
c.engine.TrustedProxies = []string{"bar"}
resetTrustedCIDRs(c)
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.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.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.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())
Expand Down
2 changes: 2 additions & 0 deletions logger_test.go
Expand Up @@ -185,6 +185,8 @@ func TestLoggerWithConfigFormatting(t *testing.T) {
buffer := new(bytes.Buffer)

router := New()
router.engine.trustedCIDRs, _ = router.engine.prepareTrustedCIDRs()

router.Use(LoggerWithConfig(LoggerConfig{
Output: buffer,
Formatter: func(param LogFormatterParams) string {
Expand Down