a new fix for #2820

gin-gonic · appleboy · Oct 23, 2021 · Oct 8, 2021 · Oct 8, 2021 · Oct 9, 2021
commit c639b3ef26640da9ba29cc06488be745041c53ed
diff --git a/gin_integration_test.go b/gin_integration_test.go
@@ -521,6 +521,7 @@ 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+"/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
@@ -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,20 @@ 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
-	)
+	skippedNodes := make([]skippedNode, 0, countSections(path)) // Caching the latest nodes
+	var globalParamsCount int16 = 0
 
 walk: // Outer loop for walking the tree
 	for {
@@ -417,15 +427,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,
 							}
 						}
 
@@ -435,9 +450,34 @@ 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
+				/*
+					matched := path != "/" && !n.wildChild
+					if matched {
+						if len(skippedNodes) > 0 {
+							l := len(skippedNodes)
+							n = skippedNodes[l-1].node
+							path = skippedNodes[l-1].path
+							skippedNodes = skippedNodes[:l-1]
+						}
+						//n = latestNode
+					}
+				*/
+
+				if path != "/" && !n.wildChild {
+					for len(skippedNodes) > 0 {
+						l := len(skippedNodes)
+						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 +491,21 @@ 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 += 1
 
 				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:]
+					/*
+						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
@@ -549,8 +592,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]
+			if n.handlers == nil && path != "/" {
+				for len(skippedNodes) > 0 {
+					l := len(skippedNodes)
+					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 +638,38 @@ 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
+		if path != "/" {
+			for len(skippedNodes) > 0 {
+				l := len(skippedNodes)
+				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
+				}
+			}
 		}
+		/*
+			len(skippedNodes) > 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
+			}
+		*/
 
 		// Nothing found. We can recommend to redirect to the same URL with an
 		// extra trailing slash if a leaf exists for that path