diff --git a/gin_integration_test.go b/gin_integration_test.go index 094c46e871..067795f54b 100644 --- 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 index fb0a5935c2..32865eccdd 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,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