From dd8a27c0b6fbbfd192c5586ba3165700f535ac7f Mon Sep 17 00:00:00 2001 From: "Alessandro (Ale) Segala" <43508+ItalyPaleAle@users.noreply.github.com> Date: Wed, 23 Jun 2021 17:58:10 -0700 Subject: [PATCH 1/4] Setting trusted platform using an enum-like (#2739) --- context.go | 16 +++++++++++++--- context_appengine.go | 2 +- context_test.go | 15 ++++++++++++--- gin.go | 23 +++++++++++++++++------ 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/context.go b/context.go index 0c1fb07f..5c1aba5e 100644 --- a/context.go +++ b/context.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "io/ioutil" + "log" "math" "mime/multipart" "net" @@ -731,17 +732,26 @@ func (c *Context) ShouldBindBodyWith(obj interface{}, bb binding.BindingBody) (e // If the headers are nots syntactically valid OR the remote IP does not correspong to a trusted proxy, // the remote IP (coming form Request.RemoteAddr) is returned. func (c *Context) ClientIP() string { - switch { - case c.engine.AppEngine: + // Check if we're running on a tursted platform + switch c.engine.TrustedPlatform { + case PlatformGoogleAppEngine: if addr := c.requestHeader("X-Appengine-Remote-Addr"); addr != "" { return addr } - case c.engine.CloudflareProxy: + case PlatformCloudflare: if addr := c.requestHeader("CF-Connecting-IP"); addr != "" { return addr } } + // Legacy "AppEngine" flag + if c.engine.AppEngine { + log.Println(`The AppEngine flag is going to be deprecated. Please check issues #2723 and #2739 and use 'TrustedPlatform: gin.PlatformGoogleAppEngine' instead.`) + if addr := c.requestHeader("X-Appengine-Remote-Addr"); addr != "" { + return addr + } + } + remoteIP, trusted := c.RemoteIP() if remoteIP == nil { return "" diff --git a/context_appengine.go b/context_appengine.go index d5658434..8bf93896 100644 --- a/context_appengine.go +++ b/context_appengine.go @@ -8,5 +8,5 @@ package gin func init() { - defaultAppEngine = true + defaultPlatform = PlatformGoogleAppEngine } diff --git a/context_test.go b/context_test.go index e0de717e..93b58ee8 100644 --- a/context_test.go +++ b/context_test.go @@ -1410,7 +1410,7 @@ func TestContextClientIP(t *testing.T) { c.Request.Header.Del("X-Forwarded-For") c.Request.Header.Del("X-Real-IP") - c.engine.AppEngine = true + c.engine.TrustedPlatform = PlatformGoogleAppEngine assert.Equal(t, "50.50.50.50", c.ClientIP()) c.Request.Header.Del("X-Appengine-Remote-Addr") @@ -1470,19 +1470,27 @@ func TestContextClientIP(t *testing.T) { assert.Equal(t, "10.10.10.10", c.ClientIP()) c.engine.RemoteIPHeaders = []string{} + c.engine.TrustedPlatform = PlatformGoogleAppEngine + assert.Equal(t, "50.50.50.50", c.ClientIP()) + + // Test the legacy flag + c.engine.TrustedPlatform = "" c.engine.AppEngine = true assert.Equal(t, "50.50.50.50", c.ClientIP()) + c.engine.AppEngine = false + c.engine.TrustedPlatform = PlatformGoogleAppEngine c.Request.Header.Del("X-Appengine-Remote-Addr") assert.Equal(t, "40.40.40.40", c.ClientIP()) - c.engine.AppEngine = false - c.engine.CloudflareProxy = true + c.engine.TrustedPlatform = PlatformCloudflare assert.Equal(t, "60.60.60.60", c.ClientIP()) c.Request.Header.Del("CF-Connecting-IP") assert.Equal(t, "40.40.40.40", c.ClientIP()) + c.engine.TrustedPlatform = "" + // no port c.Request.RemoteAddr = "50.50.50.50" assert.Empty(t, c.ClientIP()) @@ -1494,6 +1502,7 @@ func resetContextForClientIPTests(c *Context) { c.Request.Header.Set("X-Appengine-Remote-Addr", "50.50.50.50") c.Request.Header.Set("CF-Connecting-IP", "60.60.60.60") c.Request.RemoteAddr = " 40.40.40.40:42123 " + c.engine.TrustedPlatform = "" c.engine.AppEngine = false } diff --git a/gin.go b/gin.go index 00686e77..fd4fa512 100644 --- a/gin.go +++ b/gin.go @@ -25,7 +25,7 @@ var ( default405Body = []byte("405 method not allowed") ) -var defaultAppEngine bool +var defaultPlatform string // HandlerFunc defines the handler used by gin middleware as return value. type HandlerFunc func(*Context) @@ -52,6 +52,16 @@ type RouteInfo struct { // RoutesInfo defines a RouteInfo array. type RoutesInfo []RouteInfo +// Trusted platforms +const ( + // When running on Google App Engine. Trust X-Appengine-Remote-Addr + // for determining the client's IP + PlatformGoogleAppEngine = "google-app-engine" + // When using Cloudflare's CDN. Trust CF-Connecting-IP for determining + // the client's IP + PlatformCloudflare = "cloudflare" +) + // Engine is the framework's instance, it contains the muxer, middleware and configuration settings. // Create an instance of Engine, by using New() or Default() type Engine struct { @@ -101,14 +111,15 @@ type Engine struct { // `true`. TrustedProxies []string + // If set to a constant of value gin.Platform*, trusts the headers set by + // that platform, for example to determine the client IP + TrustedPlatform string + + // DEPRECATED: USE `TrustedPlatform` WITH VALUE `gin.GoogleAppEngine` INSTEAD // #726 #755 If enabled, it will trust some headers starting with // 'X-AppEngine...' for better integration with that PaaS. AppEngine bool - // If enabled, it will trust the CF-Connecting-IP header to determine the - // IP of the client. - CloudflareProxy bool - // If enabled, the url.RawPath will be used to find parameters. UseRawPath bool @@ -164,7 +175,7 @@ func New() *Engine { ForwardedByClientIP: true, RemoteIPHeaders: []string{"X-Forwarded-For", "X-Real-IP"}, TrustedProxies: []string{"0.0.0.0/0"}, - AppEngine: defaultAppEngine, + TrustedPlatform: defaultPlatform, UseRawPath: false, RemoveExtraSlash: false, UnescapePathValues: true, From be860ec1578f0c9d1e220934455e9507d3a4c1e6 Mon Sep 17 00:00:00 2001 From: ziheng Date: Thu, 24 Jun 2021 13:07:49 +0800 Subject: [PATCH 2/4] fix typo and add comments (#2760) --- recovery.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/recovery.go b/recovery.go index 563f5aaa..85199591 100644 --- a/recovery.go +++ b/recovery.go @@ -34,7 +34,7 @@ func Recovery() HandlerFunc { return RecoveryWithWriter(DefaultErrorWriter) } -//CustomRecovery returns a middleware that recovers from any panics and calls the provided handle func to handle it. +// CustomRecovery returns a middleware that recovers from any panics and calls the provided handle func to handle it. func CustomRecovery(handle RecoveryFunc) HandlerFunc { return RecoveryWithWriter(DefaultErrorWriter, handle) } @@ -165,6 +165,7 @@ func function(pc uintptr) []byte { return name } +// timeFormat returns a customized time string for logger. func timeFormat(t time.Time) string { timeString := t.Format("2006/01/02 - 15:04:05") return timeString From 09f6cff92a319b080789c8d20aa4cc6efbe876fe Mon Sep 17 00:00:00 2001 From: ziheng Date: Thu, 24 Jun 2021 15:31:38 +0800 Subject: [PATCH 3/4] skip unnecessary variable assignment in timeFormat (#2761) --- recovery.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/recovery.go b/recovery.go index 85199591..3101fe28 100644 --- a/recovery.go +++ b/recovery.go @@ -167,6 +167,5 @@ func function(pc uintptr) []byte { // timeFormat returns a customized time string for logger. func timeFormat(t time.Time) string { - timeString := t.Format("2006/01/02 - 15:04:05") - return timeString + return t.Format("2006/01/02 - 15:04:05") } From 7834a03e8467c1b58eda0efd58374896f240f69c Mon Sep 17 00:00:00 2001 From: wei Date: Thu, 24 Jun 2021 16:33:14 +0800 Subject: [PATCH 4/4] gin.Context with fallback value from gin.Context.Request.Context() (#2751) * Update tree.go (#2659) delete more "()" * updated comments for Get function for params (#2756) * ci: add github action workflows (#2596) * ci: add github action workflows * test: fixed the TestUnixSocket test on windows (#20) * ci: add github action workflows (#18) * Remove .travis.yml * ci: replace GITTER_ROOM_ID and upload coverage every time you go test * ci: update coverage using codecov/codecov-action@v1 * Merge branch 'master' into github-actions * repo: replace travis ci to github actions * ci: add go version 1.16 * fix: go install requires a specific version * chore(ci): remove go 1.12 support * chore(ci): remove os windows-latest Co-authored-by: thinkerou Co-authored-by: Bo-Yi Wu * Setting trusted platform using an enum-like (#2739) * gin.Context with fallback value from c.Request.Context() * add test case Co-authored-by: youzeliang Co-authored-by: Ashwani Co-authored-by: Jeff Co-authored-by: thinkerou Co-authored-by: Bo-Yi Wu Co-authored-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com> --- context.go | 10 +++++++--- context_test.go | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/context.go b/context.go index 5c1aba5e..ede16fd4 100644 --- a/context.go +++ b/context.go @@ -1186,8 +1186,12 @@ func (c *Context) Value(key interface{}) interface{} { return c.Request } if keyAsString, ok := key.(string); ok { - val, _ := c.Get(keyAsString) - return val + if val, exists := c.Get(keyAsString); exists { + return val + } } - return nil + if c.Request == nil || c.Request.Context() == nil { + return nil + } + return c.Request.Context().Value(key) } diff --git a/context_test.go b/context_test.go index 93b58ee8..2a4d2185 100644 --- a/context_test.go +++ b/context_test.go @@ -2057,3 +2057,56 @@ func TestRemoteIPFail(t *testing.T) { assert.Nil(t, ip) assert.False(t, trust) } + +func TestContextWithFallbackValueFromRequestContext(t *testing.T) { + tests := []struct { + name string + getContextAndKey func() (*Context, interface{}) + value interface{} + }{ + { + name: "c with struct context key", + getContextAndKey: func() (*Context, interface{}) { + var key struct{} + c := &Context{} + c.Request, _ = http.NewRequest("POST", "/", nil) + c.Request = c.Request.WithContext(context.WithValue(context.TODO(), key, "value")) + return c, key + }, + value: "value", + }, + { + name: "c with string context key", + getContextAndKey: func() (*Context, interface{}) { + c := &Context{} + c.Request, _ = http.NewRequest("POST", "/", nil) + c.Request = c.Request.WithContext(context.WithValue(context.TODO(), "key", "value")) + return c, "key" + }, + value: "value", + }, + { + name: "c with nil http.Request", + getContextAndKey: func() (*Context, interface{}) { + c := &Context{} + return c, "key" + }, + value: nil, + }, + { + name: "c with nil http.Request.Context()", + getContextAndKey: func() (*Context, interface{}) { + c := &Context{} + c.Request, _ = http.NewRequest("POST", "/", nil) + return c, "key" + }, + value: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, key := tt.getContextAndKey() + assert.Equal(t, tt.value, c.Value(key)) + }) + } +}