From 34ae7777cada2ab0f0ca650363fef7134d7ad74f Mon Sep 17 00:00:00 2001 From: Allen Ren Date: Fri, 22 Oct 2021 21:26:14 +0800 Subject: [PATCH 1/4] README.md: fix a typo (#2913) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 62094ff1..0fa78eca 100644 --- a/README.md +++ b/README.md @@ -2000,7 +2000,7 @@ func SomeHandler(c *gin.Context) { objA := formA{} objB := formB{} // This reads c.Request.Body and stores the result into the context. - if errA := c.ShouldBindBodyWith(&objA, binding.JSON); errA == nil { + if errA := c.ShouldBindBodyWith(&objA, binding.Form); errA == nil { c.String(http.StatusOK, `the body should be formA`) // At this time, it reuses body stored in the context. } else if errB := c.ShouldBindBodyWith(&objB, binding.JSON); errB == nil { From 3fe928994b7ba7ed16b0c5114ca82a9dafa7a974 Mon Sep 17 00:00:00 2001 From: Zhu Xi Date: Sat, 23 Oct 2021 11:58:57 +0800 Subject: [PATCH 2/4] Update the code logic for latestNode in tree.go (#2897) --- context.go | 6 ++- gin.go | 12 +++-- gin_integration_test.go | 15 ++++++ tree.go | 117 ++++++++++++++++++++++++++-------------- tree_test.go | 22 +++++--- 5 files changed, 120 insertions(+), 52 deletions(-) diff --git a/context.go b/context.go index bea95cca..bc2c38e1 100644 --- a/context.go +++ b/context.go @@ -55,8 +55,9 @@ type Context struct { index int8 fullPath string - engine *Engine - params *Params + engine *Engine + params *Params + skippedNodes *[]skippedNode // This mutex protect Keys map mu sync.RWMutex @@ -99,6 +100,7 @@ func (c *Context) reset() { c.queryCache = nil c.formCache = nil *c.params = (*c.params)[:0] + *c.skippedNodes = (*c.skippedNodes)[:0] } // Copy returns a copy of the current context that can be safely used outside the request's scope. diff --git a/gin.go b/gin.go index af83161b..1774717f 100644 --- a/gin.go +++ b/gin.go @@ -144,6 +144,7 @@ type Engine struct { pool sync.Pool trees methodTrees maxParams uint16 + maxSections uint16 trustedProxies []string trustedCIDRs []*net.IPNet } @@ -200,7 +201,8 @@ func Default() *Engine { func (engine *Engine) allocateContext() *Context { v := make(Params, 0, engine.maxParams) - return &Context{engine: engine, params: &v} + skippedNodes := make([]skippedNode, 0, engine.maxSections) + return &Context{engine: engine, params: &v, skippedNodes: &skippedNodes} } // Delims sets template left and right delims and returns a Engine instance. @@ -306,6 +308,10 @@ func (engine *Engine) addRoute(method, path string, handlers HandlersChain) { if paramsCount := countParams(path); paramsCount > engine.maxParams { engine.maxParams = paramsCount } + + if sectionsCount := countSections(path); sectionsCount > engine.maxSections { + engine.maxSections = sectionsCount + } } // Routes returns a slice of registered routes, including some useful information, such as: @@ -539,7 +545,7 @@ func (engine *Engine) handleHTTPRequest(c *Context) { } root := t[i].root // Find route in tree - value := root.getValue(rPath, c.params, unescape) + value := root.getValue(rPath, c.params, c.skippedNodes, unescape) if value.params != nil { c.Params = *value.params } @@ -567,7 +573,7 @@ func (engine *Engine) handleHTTPRequest(c *Context) { if tree.method == httpMethod { continue } - if value := tree.root.getValue(rPath, nil, unescape); value.handlers != nil { + if value := tree.root.getValue(rPath, nil, c.skippedNodes, unescape); value.handlers != nil { c.handlers = engine.allNoMethod serveError(c, http.StatusMethodNotAllowed, default405Body) return diff --git a/gin_integration_test.go b/gin_integration_test.go index 0b67b542..8c22e7bd 100644 --- a/gin_integration_test.go +++ b/gin_integration_test.go @@ -408,8 +408,13 @@ func TestTreeRunDynamicRouting(t *testing.T) { router.GET("/ab/*xx", func(c *Context) { c.String(http.StatusOK, "/ab/*xx") }) router.GET("/", func(c *Context) { c.String(http.StatusOK, "home") }) router.GET("/:cc", func(c *Context) { c.String(http.StatusOK, "/:cc") }) + router.GET("/c1/:dd/e", func(c *Context) { c.String(http.StatusOK, "/c1/:dd/e") }) + router.GET("/c1/:dd/e1", func(c *Context) { c.String(http.StatusOK, "/c1/:dd/e1") }) + router.GET("/c1/:dd/f1", func(c *Context) { c.String(http.StatusOK, "/c1/:dd/f1") }) + router.GET("/c1/:dd/f2", func(c *Context) { c.String(http.StatusOK, "/c1/:dd/f2") }) router.GET("/:cc/cc", func(c *Context) { c.String(http.StatusOK, "/:cc/cc") }) router.GET("/:cc/:dd/ee", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/ee") }) + router.GET("/:cc/:dd/f", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/f") }) router.GET("/:cc/:dd/:ee/ff", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/ff") }) router.GET("/:cc/:dd/:ee/:ff/gg", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/gg") }) router.GET("/:cc/:dd/:ee/:ff/:gg/hh", func(c *Context) { c.String(http.StatusOK, "/:cc/:dd/:ee/:ff/:gg/hh") }) @@ -446,6 +451,10 @@ func TestTreeRunDynamicRouting(t *testing.T) { testRequest(t, ts.URL+"/all", "", "/:cc") testRequest(t, ts.URL+"/all/cc", "", "/:cc/cc") testRequest(t, ts.URL+"/a/cc", "", "/:cc/cc") + testRequest(t, ts.URL+"/c1/d/e", "", "/c1/:dd/e") + testRequest(t, ts.URL+"/c1/d/e1", "", "/c1/:dd/e1") + testRequest(t, ts.URL+"/c1/d/ee", "", "/:cc/:dd/ee") + testRequest(t, ts.URL+"/c1/d/f", "", "/:cc/:dd/f") testRequest(t, ts.URL+"/c/d/ee", "", "/:cc/:dd/ee") testRequest(t, ts.URL+"/c/d/e/ff", "", "/:cc/:dd/:ee/ff") testRequest(t, ts.URL+"/c/d/e/f/gg", "", "/:cc/:dd/:ee/:ff/gg") @@ -528,6 +537,12 @@ func TestTreeRunDynamicRouting(t *testing.T) { testRequest(t, ts.URL+"/get/abc/123abf/testss", "", "/get/abc/123abf/:param") testRequest(t, ts.URL+"/get/abc/123abfff/te", "", "/get/abc/123abfff/:param") // 404 not found + testRequest(t, ts.URL+"/c/d/e", "404 Not Found") + testRequest(t, ts.URL+"/c/d/e1", "404 Not Found") + testRequest(t, ts.URL+"/c/d/eee", "404 Not Found") + testRequest(t, ts.URL+"/c1/d/eee", "404 Not Found") + testRequest(t, ts.URL+"/c1/d/e2", "404 Not Found") + testRequest(t, ts.URL+"/cc/dd/ee/ff/gg/hh1", "404 Not Found") testRequest(t, ts.URL+"/a/dd", "404 Not Found") testRequest(t, ts.URL+"/addr/dd/aa", "404 Not Found") testRequest(t, ts.URL+"/something/secondthing/121", "404 Not Found") diff --git a/tree.go b/tree.go index fb0a5935..c8a7548e 100644 --- a/tree.go +++ b/tree.go @@ -17,6 +17,7 @@ import ( var ( strColon = []byte(":") strStar = []byte("*") + strSlash = []byte("/") ) // Param is a single URL parameter, consisting of a key and a value. @@ -98,6 +99,11 @@ func countParams(path string) uint16 { return n } +func countSections(path string) uint16 { + s := bytesconv.StringToBytes(path) + return uint16(bytes.Count(s, strSlash)) +} + type nodeType uint8 const ( @@ -393,16 +399,19 @@ type nodeValue struct { fullPath string } +type skippedNode struct { + path string + node *node + paramsCount int16 +} + // Returns the handle registered with the given path (key). The values of // wildcards are saved to a map. // If no handle can be found, a TSR (trailing slash redirect) recommendation is // made if a handle exists with an extra (without the) trailing slash for the // given path. -func (n *node) getValue(path string, params *Params, unescape bool) (value nodeValue) { - var ( - skippedPath string - latestNode = n // Caching the latest node - ) +func (n *node) getValue(path string, params *Params, skippedNodes *[]skippedNode, unescape bool) (value nodeValue) { + var globalParamsCount int16 walk: // Outer loop for walking the tree for { @@ -417,15 +426,20 @@ walk: // Outer loop for walking the tree if c == idxc { // strings.HasPrefix(n.children[len(n.children)-1].path, ":") == n.wildChild if n.wildChild { - skippedPath = prefix + path - latestNode = &node{ - path: n.path, - wildChild: n.wildChild, - nType: n.nType, - priority: n.priority, - children: n.children, - handlers: n.handlers, - fullPath: n.fullPath, + index := len(*skippedNodes) + *skippedNodes = (*skippedNodes)[:index+1] + (*skippedNodes)[index] = skippedNode{ + path: prefix + path, + node: &node{ + path: n.path, + wildChild: n.wildChild, + nType: n.nType, + priority: n.priority, + children: n.children, + handlers: n.handlers, + fullPath: n.fullPath, + }, + paramsCount: globalParamsCount, } } @@ -434,10 +448,22 @@ walk: // Outer loop for walking the tree } } // If the path at the end of the loop is not equal to '/' and the current node has no child nodes - // the current node needs to be equal to the latest matching node - matched := path != "/" && !n.wildChild - if matched { - n = latestNode + // the current node needs to roll back to last vaild skippedNode + + if path != "/" && !n.wildChild { + for l := len(*skippedNodes); l > 0; { + skippedNode := (*skippedNodes)[l-1] + *skippedNodes = (*skippedNodes)[:l-1] + if strings.HasSuffix(skippedNode.path, path) { + path = skippedNode.path + n = skippedNode.node + if value.params != nil { + *value.params = (*value.params)[:skippedNode.paramsCount] + } + globalParamsCount = skippedNode.paramsCount + continue walk + } + } } // If there is no wildcard pattern, recommend a redirection @@ -451,18 +477,12 @@ walk: // Outer loop for walking the tree // Handle wildcard child, which is always at the end of the array n = n.children[len(n.children)-1] + globalParamsCount++ switch n.nType { case param: // fix truncate the parameter // tree_test.go line: 204 - if matched { - path = prefix + path - // The saved path is used after the prefix route is intercepted by matching - if n.indices == "/" { - path = skippedPath[1:] - } - } // Find param end (either '/' or path end) end := 0 @@ -548,9 +568,22 @@ walk: // Outer loop for walking the tree if path == prefix { // If the current path does not equal '/' and the node does not have a registered handle and the most recently matched node has a child node - // the current node needs to be equal to the latest matching node - if latestNode.wildChild && n.handlers == nil && path != "/" { - n = latestNode.children[len(latestNode.children)-1] + // the current node needs to roll back to last vaild skippedNode + if n.handlers == nil && path != "/" { + for l := len(*skippedNodes); l > 0; { + skippedNode := (*skippedNodes)[l-1] + *skippedNodes = (*skippedNodes)[:l-1] + if strings.HasSuffix(skippedNode.path, path) { + path = skippedNode.path + n = skippedNode.node + if value.params != nil { + *value.params = (*value.params)[:skippedNode.paramsCount] + } + globalParamsCount = skippedNode.paramsCount + continue walk + } + } + // n = latestNode.children[len(latestNode.children)-1] } // We should have reached the node containing the handle. // Check if this node has a handle registered. @@ -581,19 +614,21 @@ walk: // Outer loop for walking the tree return } - if path != "/" && len(skippedPath) > 0 && strings.HasSuffix(skippedPath, path) { - path = skippedPath - // Reduce the number of cycles - n, latestNode = latestNode, n - // skippedPath cannot execute - // example: - // * /:cc/cc - // call /a/cc expectations:match/200 Actual:match/200 - // call /a/dd expectations:unmatch/404 Actual: panic - // call /addr/dd/aa expectations:unmatch/404 Actual: panic - // skippedPath: It can only be executed if the secondary route is not found - skippedPath = "" - continue walk + // roll back to last vaild skippedNode + if path != "/" { + for l := len(*skippedNodes); l > 0; { + skippedNode := (*skippedNodes)[l-1] + *skippedNodes = (*skippedNodes)[:l-1] + if strings.HasSuffix(skippedNode.path, path) { + path = skippedNode.path + n = skippedNode.node + if value.params != nil { + *value.params = (*value.params)[:skippedNode.paramsCount] + } + globalParamsCount = skippedNode.paramsCount + continue walk + } + } } // Nothing found. We can recommend to redirect to the same URL with an diff --git a/tree_test.go b/tree_test.go index 8ae5b7db..49b3b57e 100644 --- a/tree_test.go +++ b/tree_test.go @@ -33,6 +33,11 @@ func getParams() *Params { return &ps } +func getSkippedNodes() *[]skippedNode { + ps := make([]skippedNode, 0, 20) + return &ps +} + func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes ...bool) { unescape := false if len(unescapes) >= 1 { @@ -40,7 +45,7 @@ func checkRequests(t *testing.T, tree *node, requests testRequests, unescapes .. } for _, request := range requests { - value := tree.getValue(request.path, getParams(), unescape) + value := tree.getValue(request.path, getParams(), getSkippedNodes(), unescape) if value.handlers == nil { if !request.nilHandler { @@ -157,6 +162,8 @@ func TestTreeWildcard(t *testing.T) { "/aa/*xx", "/ab/*xx", "/:cc", + "/c1/:dd/e", + "/c1/:dd/e1", "/:cc/cc", "/:cc/:dd/ee", "/:cc/:dd/:ee/ff", @@ -238,6 +245,9 @@ func TestTreeWildcard(t *testing.T) { {"/alldd", false, "/:cc", Params{Param{Key: "cc", Value: "alldd"}}}, {"/all/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "all"}}}, {"/a/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "a"}}}, + {"/c1/d/e", false, "/c1/:dd/e", Params{Param{Key: "dd", Value: "d"}}}, + {"/c1/d/e1", false, "/c1/:dd/e1", Params{Param{Key: "dd", Value: "d"}}}, + {"/c1/d/ee", false, "/:cc/:dd/ee", Params{Param{Key: "cc", Value: "c1"}, Param{Key: "dd", Value: "d"}}}, {"/cc/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "cc"}}}, {"/ccc/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "ccc"}}}, {"/deedwjfs/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "deedwjfs"}}}, @@ -605,7 +615,7 @@ func TestTreeTrailingSlashRedirect(t *testing.T) { "/doc/", } for _, route := range tsrRoutes { - value := tree.getValue(route, nil, false) + value := tree.getValue(route, nil, getSkippedNodes(), false) if value.handlers != nil { t.Fatalf("non-nil handler for TSR route '%s", route) } else if !value.tsr { @@ -622,7 +632,7 @@ func TestTreeTrailingSlashRedirect(t *testing.T) { "/api/world/abc", } for _, route := range noTsrRoutes { - value := tree.getValue(route, nil, false) + value := tree.getValue(route, nil, getSkippedNodes(), false) if value.handlers != nil { t.Fatalf("non-nil handler for No-TSR route '%s", route) } else if value.tsr { @@ -641,7 +651,7 @@ func TestTreeRootTrailingSlashRedirect(t *testing.T) { t.Fatalf("panic inserting test route: %v", recv) } - value := tree.getValue("/", nil, false) + value := tree.getValue("/", nil, getSkippedNodes(), false) if value.handlers != nil { t.Fatalf("non-nil handler") } else if value.tsr { @@ -821,7 +831,7 @@ func TestTreeInvalidNodeType(t *testing.T) { // normal lookup recv := catchPanic(func() { - tree.getValue("/test", nil, false) + tree.getValue("/test", nil, getSkippedNodes(), false) }) if rs, ok := recv.(string); !ok || rs != panicMsg { t.Fatalf("Expected panic '"+panicMsg+"', got '%v'", recv) @@ -846,7 +856,7 @@ func TestTreeInvalidParamsType(t *testing.T) { params := make(Params, 0) // try to trigger slice bounds out of range with capacity 0 - tree.getValue("/test", ¶ms, false) + tree.getValue("/test", ¶ms, getSkippedNodes(), false) } func TestTreeWildcardConflictEx(t *testing.T) { From 2d3d6d2f1355f21a77f8ef6cf45a5bc531c8c935 Mon Sep 17 00:00:00 2001 From: Notealot <714804968@qq.com> Date: Sun, 24 Oct 2021 08:34:03 +0800 Subject: [PATCH 3/4] Provide custom options of TrustedPlatform for another CDN services (#2906) * refine TrustedPlatform and docs * refactor for switch * refactor switch to if statement --- README.md | 29 +++++++++++++++++++++++++++++ context.go | 14 +++++--------- context_test.go | 14 +++++++++++++- gin.go | 4 ++-- 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 0fa78eca..cad746d6 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,7 @@ Gin is a web framework written in Go (Golang). It features a martini-like API wi - [http2 server push](#http2-server-push) - [Define format for the log of routes](#define-format-for-the-log-of-routes) - [Set and get a cookie](#set-and-get-a-cookie) + - [Don't trust all proxies](#don't-trust-all-proxies) - [Testing](#testing) - [Users](#users) @@ -2236,6 +2237,34 @@ func main() { } ``` +**Notice:** If you are using a CDN service, you can set the `Engine.TrustedPlatform` +to skip TrustedProxies check, it has a higher priority than TrustedProxies. +Look at the example below: +```go +import ( + "fmt" + + "github.com/gin-gonic/gin" +) + +func main() { + + router := gin.Default() + // Use predefined header gin.PlatformXXX + router.TrustedPlatform = gin.PlatformGoogleAppEngine + // Or set your own trusted request header for another trusted proxy service + // Don't set it to any suspect request header, it's unsafe + router.TrustedPlatform = "X-CDN-IP" + + router.GET("/", func(c *gin.Context) { + // If you set TrustedPlatform, ClientIP() will resolve the + // corresponding header and return IP directly + fmt.Printf("ClientIP: %s\n", c.ClientIP()) + }) + router.Run() +} +``` + ## Testing The `net/http/httptest` package is preferable way for HTTP testing. diff --git a/context.go b/context.go index bc2c38e1..58f38c88 100644 --- a/context.go +++ b/context.go @@ -735,20 +735,16 @@ func (c *Context) ShouldBindBodyWith(obj interface{}, bb binding.BindingBody) (e return bb.BindBody(body, obj) } -// ClientIP implements a best effort algorithm to return the real client IP. +// ClientIP implements one best effort algorithm to return the real client IP. // It called c.RemoteIP() under the hood, to check if the remote IP is a trusted proxy or not. // If it is it will then try to parse the headers defined in Engine.RemoteIPHeaders (defaulting to [X-Forwarded-For, X-Real-Ip]). // If the headers are not syntactically valid OR the remote IP does not correspond to a trusted proxy, // the remote IP (coming form Request.RemoteAddr) is returned. func (c *Context) ClientIP() string { - // Check if we're running on a trusted platform - switch c.engine.TrustedPlatform { - case PlatformGoogleAppEngine: - if addr := c.requestHeader("X-Appengine-Remote-Addr"); addr != "" { - return addr - } - case PlatformCloudflare: - if addr := c.requestHeader("CF-Connecting-IP"); addr != "" { + // Check if we're running on a trusted platform, continue running backwards if error + if c.engine.TrustedPlatform != "" { + // Developers can define their own header of Trusted Platform or use predefined constants + if addr := c.requestHeader(c.engine.TrustedPlatform); addr != "" { return addr } } diff --git a/context_test.go b/context_test.go index e9fe88f9..c286c0f4 100644 --- a/context_test.go +++ b/context_test.go @@ -1458,8 +1458,20 @@ func TestContextClientIP(t *testing.T) { c.engine.TrustedPlatform = PlatformGoogleAppEngine assert.Equal(t, "50.50.50.50", c.ClientIP()) - // Test the legacy flag + // Use custom TrustedPlatform header + c.engine.TrustedPlatform = "X-CDN-IP" + c.Request.Header.Set("X-CDN-IP", "80.80.80.80") + assert.Equal(t, "80.80.80.80", c.ClientIP()) + // wrong header + c.engine.TrustedPlatform = "X-Wrong-Header" + assert.Equal(t, "40.40.40.40", c.ClientIP()) + + c.Request.Header.Del("X-CDN-IP") + // TrustedPlatform is empty c.engine.TrustedPlatform = "" + assert.Equal(t, "40.40.40.40", c.ClientIP()) + + // Test the legacy flag c.engine.AppEngine = true assert.Equal(t, "50.50.50.50", c.ClientIP()) c.engine.AppEngine = false diff --git a/gin.go b/gin.go index 1774717f..1d9c9fee 100644 --- a/gin.go +++ b/gin.go @@ -59,10 +59,10 @@ type RoutesInfo []RouteInfo const ( // When running on Google App Engine. Trust X-Appengine-Remote-Addr // for determining the client's IP - PlatformGoogleAppEngine = "google-app-engine" + PlatformGoogleAppEngine = "X-Appengine-Remote-Addr" // When using Cloudflare's CDN. Trust CF-Connecting-IP for determining // the client's IP - PlatformCloudflare = "cloudflare" + PlatformCloudflare = "CF-Connecting-IP" ) // Engine is the framework's instance, it contains the muxer, middleware and configuration settings. From eb75ce0ff5d64b269f2a25e83952e374e762c4b6 Mon Sep 17 00:00:00 2001 From: heige Date: Sun, 24 Oct 2021 09:31:13 +0800 Subject: [PATCH 4/4] adjust the routergroup Any method (#2701) --- routergroup.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/routergroup.go b/routergroup.go index bb24bd52..27d7aad6 100644 --- a/routergroup.go +++ b/routergroup.go @@ -14,6 +14,13 @@ import ( var ( // reg match english letters for http method name regEnLetter = regexp.MustCompile("^[A-Z]+$") + + // anyMethods for RouterGroup Any method + anyMethods = []string{ + http.MethodGet, http.MethodPost, http.MethodPut, http.MethodPatch, + http.MethodHead, http.MethodOptions, http.MethodDelete, http.MethodConnect, + http.MethodTrace, + } ) // IRouter defines all router handle interface includes single and group router. @@ -136,15 +143,10 @@ func (group *RouterGroup) HEAD(relativePath string, handlers ...HandlerFunc) IRo // Any registers a route that matches all the HTTP methods. // GET, POST, PUT, PATCH, HEAD, OPTIONS, DELETE, CONNECT, TRACE. func (group *RouterGroup) Any(relativePath string, handlers ...HandlerFunc) IRoutes { - group.handle(http.MethodGet, relativePath, handlers) - group.handle(http.MethodPost, relativePath, handlers) - group.handle(http.MethodPut, relativePath, handlers) - group.handle(http.MethodPatch, relativePath, handlers) - group.handle(http.MethodHead, relativePath, handlers) - group.handle(http.MethodOptions, relativePath, handlers) - group.handle(http.MethodDelete, relativePath, handlers) - group.handle(http.MethodConnect, relativePath, handlers) - group.handle(http.MethodTrace, relativePath, handlers) + for _, method := range anyMethods { + group.handle(method, relativePath, handlers) + } + return group.returnObj() }