Merge remote-tracking branch 'upstream/master'

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)
 
@@ -2000,7 +2001,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 {
@@ -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.

context.go

@@ -57,6 +57,7 @@ type Context struct {
 
 	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.
@@ -733,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
 		}
 	}

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

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.
@@ -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

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")

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()
 }
 

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,8 +426,11 @@ 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{
+							index := len(*skippedNodes)
+							*skippedNodes = (*skippedNodes)[:index+1]
+							(*skippedNodes)[index] = skippedNode{
+								path: prefix + path,
+								node: &node{
 									path:      n.path,
 									wildChild: n.wildChild,
 									nType:     n.nType,
@@ -426,6 +438,8 @@ walk: // Outer loop for walking the tree
 									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,20 +614,22 @@ 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 = ""
+		// 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
 		// extra trailing slash if a leaf exists for that path

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", &params, false)
+	tree.getValue("/test", &params, getSkippedNodes(), false)
 }
 
 func TestTreeWildcardConflictEx(t *testing.T) {