diff --git a/tree.go b/tree.go index 5eb093489e..0d082d0571 100644 --- a/tree.go +++ b/tree.go @@ -118,6 +118,11 @@ type node struct { fullPath string } +type skip struct { + path string + paramNode *node +} + // Increments priority of the given child and reorders if necessary func (n *node) incrementChildPrio(pos int) int { cs := n.children @@ -400,10 +405,7 @@ type nodeValue struct { // 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 - ) + var skipped *skip walk: // Outer loop for walking the tree for { @@ -416,17 +418,18 @@ walk: // Outer loop for walking the tree idxc := path[0] for i, c := range []byte(n.indices) { 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, + if strings.HasPrefix(n.children[len(n.children)-1].path, ":") { + skipped = &skip{ + path: prefix + path, + paramNode: &node{ + path: n.path, + wildChild: n.wildChild, + nType: n.nType, + priority: n.priority, + children: n.children, + handlers: n.handlers, + fullPath: n.fullPath, + }, } } @@ -434,19 +437,13 @@ walk: // Outer loop for walking the tree continue walk } } - // 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 - } // If there is no wildcard pattern, recommend a redirection if !n.wildChild { // Nothing found. // We can recommend to redirect to the same URL without a // trailing slash if a leaf exists for that path. - value.tsr = path == "/" && n.handlers != nil + value.tsr = (path == "/" && n.handlers != nil) return } @@ -455,16 +452,6 @@ walk: // Outer loop for walking the tree 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 for end < len(path) && path[end] != '/' { @@ -548,11 +535,6 @@ 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] - } // We should have reached the node containing the handle. // Check if this node has a handle registered. if value.handlers = n.handlers; value.handlers != nil { @@ -582,25 +564,18 @@ 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 = "" + if path != "/" && skipped != nil && strings.HasSuffix(skipped.path, path) { + path = skipped.path + n = skipped.paramNode + skipped = nil 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 - value.tsr = path == "/" || - (len(prefix) == len(path)+1 && n.handlers != nil) + value.tsr = (path == "/") || + (len(prefix) == len(path)+1 && prefix[len(path)] == '/' && + path == prefix[:len(prefix)-1] && n.handlers != nil) return } } diff --git a/tree_test.go b/tree_test.go index d799e45a5a..298c5ed0c5 100644 --- a/tree_test.go +++ b/tree_test.go @@ -154,37 +154,6 @@ func TestTreeWildcard(t *testing.T) { "/info/:user/public", "/info/:user/project/:project", "/info/:user/project/golang", - "/aa/*xx", - "/ab/*xx", - "/:cc", - "/:cc/cc", - "/:cc/:dd/ee", - "/:cc/:dd/:ee/ff", - "/:cc/:dd/:ee/:ff/gg", - "/:cc/:dd/:ee/:ff/:gg/hh", - "/get/test/abc/", - "/get/:param/abc/", - "/something/:paramname/thirdthing", - "/something/secondthing/test", - "/get/abc", - "/get/:param", - "/get/abc/123abc", - "/get/abc/:param", - "/get/abc/123abc/xxx8", - "/get/abc/123abc/:param", - "/get/abc/123abc/xxx8/1234", - "/get/abc/123abc/xxx8/:param", - "/get/abc/123abc/xxx8/1234/ffas", - "/get/abc/123abc/xxx8/1234/:param", - "/get/abc/123abc/xxx8/1234/kkdd/12c", - "/get/abc/123abc/xxx8/1234/kkdd/:param", - "/get/abc/:param/test", - "/get/abc/123abd/:param", - "/get/abc/123abddd/:param", - "/get/abc/123/:param", - "/get/abc/123abg/:param", - "/get/abc/123abf/:param", - "/get/abc/123abfff/:param", } for _, route := range routes { tree.addRoute(route, fakeHandler(route)) @@ -217,94 +186,6 @@ func TestTreeWildcard(t *testing.T) { {"/info/gordon/public", false, "/info/:user/public", Params{Param{Key: "user", Value: "gordon"}}}, {"/info/gordon/project/go", false, "/info/:user/project/:project", Params{Param{Key: "user", Value: "gordon"}, Param{Key: "project", Value: "go"}}}, {"/info/gordon/project/golang", false, "/info/:user/project/golang", Params{Param{Key: "user", Value: "gordon"}}}, - {"/aa/aa", false, "/aa/*xx", Params{Param{Key: "xx", Value: "/aa"}}}, - {"/ab/ab", false, "/ab/*xx", Params{Param{Key: "xx", Value: "/ab"}}}, - {"/a", false, "/:cc", Params{Param{Key: "cc", Value: "a"}}}, - // * Error with argument being intercepted - // new PR handle (/all /all/cc /a/cc) - // fix PR: https://github.com/gin-gonic/gin/pull/2796 - {"/all", false, "/:cc", Params{Param{Key: "cc", Value: "all"}}}, - {"/d", false, "/:cc", Params{Param{Key: "cc", Value: "d"}}}, - {"/ad", false, "/:cc", Params{Param{Key: "cc", Value: "ad"}}}, - {"/dd", false, "/:cc", Params{Param{Key: "cc", Value: "dd"}}}, - {"/dddaa", false, "/:cc", Params{Param{Key: "cc", Value: "dddaa"}}}, - {"/aa", false, "/:cc", Params{Param{Key: "cc", Value: "aa"}}}, - {"/aaa", false, "/:cc", Params{Param{Key: "cc", Value: "aaa"}}}, - {"/aaa/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "aaa"}}}, - {"/ab", false, "/:cc", Params{Param{Key: "cc", Value: "ab"}}}, - {"/abb", false, "/:cc", Params{Param{Key: "cc", Value: "abb"}}}, - {"/abb/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "abb"}}}, - {"/allxxxx", false, "/:cc", Params{Param{Key: "cc", Value: "allxxxx"}}}, - {"/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"}}}, - {"/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"}}}, - {"/acllcc/cc", false, "/:cc/cc", Params{Param{Key: "cc", Value: "acllcc"}}}, - {"/get/test/abc/", false, "/get/test/abc/", nil}, - {"/get/te/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "te"}}}, - {"/get/testaa/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "testaa"}}}, - {"/get/xx/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "xx"}}}, - {"/get/tt/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "tt"}}}, - {"/get/a/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "a"}}}, - {"/get/t/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "t"}}}, - {"/get/aa/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "aa"}}}, - {"/get/abas/abc/", false, "/get/:param/abc/", Params{Param{Key: "param", Value: "abas"}}}, - {"/something/secondthing/test", false, "/something/secondthing/test", nil}, - {"/something/abcdad/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "abcdad"}}}, - {"/something/secondthingaaaa/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "secondthingaaaa"}}}, - {"/something/se/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "se"}}}, - {"/something/s/thirdthing", false, "/something/:paramname/thirdthing", Params{Param{Key: "paramname", Value: "s"}}}, - {"/c/d/ee", false, "/:cc/:dd/ee", Params{Param{Key: "cc", Value: "c"}, Param{Key: "dd", Value: "d"}}}, - {"/c/d/e/ff", false, "/:cc/:dd/:ee/ff", Params{Param{Key: "cc", Value: "c"}, Param{Key: "dd", Value: "d"}, Param{Key: "ee", Value: "e"}}}, - {"/c/d/e/f/gg", false, "/:cc/:dd/:ee/:ff/gg", Params{Param{Key: "cc", Value: "c"}, Param{Key: "dd", Value: "d"}, Param{Key: "ee", Value: "e"}, Param{Key: "ff", Value: "f"}}}, - {"/c/d/e/f/g/hh", false, "/:cc/:dd/:ee/:ff/:gg/hh", Params{Param{Key: "cc", Value: "c"}, Param{Key: "dd", Value: "d"}, Param{Key: "ee", Value: "e"}, Param{Key: "ff", Value: "f"}, Param{Key: "gg", Value: "g"}}}, - {"/cc/dd/ee/ff/gg/hh", false, "/:cc/:dd/:ee/:ff/:gg/hh", Params{Param{Key: "cc", Value: "cc"}, Param{Key: "dd", Value: "dd"}, Param{Key: "ee", Value: "ee"}, Param{Key: "ff", Value: "ff"}, Param{Key: "gg", Value: "gg"}}}, - {"/get/abc", false, "/get/abc", nil}, - {"/get/a", false, "/get/:param", Params{Param{Key: "param", Value: "a"}}}, - {"/get/abz", false, "/get/:param", Params{Param{Key: "param", Value: "abz"}}}, - {"/get/12a", false, "/get/:param", Params{Param{Key: "param", Value: "12a"}}}, - {"/get/abcd", false, "/get/:param", Params{Param{Key: "param", Value: "abcd"}}}, - {"/get/abc/123abc", false, "/get/abc/123abc", nil}, - {"/get/abc/12", false, "/get/abc/:param", Params{Param{Key: "param", Value: "12"}}}, - {"/get/abc/123ab", false, "/get/abc/:param", Params{Param{Key: "param", Value: "123ab"}}}, - {"/get/abc/xyz", false, "/get/abc/:param", Params{Param{Key: "param", Value: "xyz"}}}, - {"/get/abc/123abcddxx", false, "/get/abc/:param", Params{Param{Key: "param", Value: "123abcddxx"}}}, - {"/get/abc/123abc/xxx8", false, "/get/abc/123abc/xxx8", nil}, - {"/get/abc/123abc/x", false, "/get/abc/123abc/:param", Params{Param{Key: "param", Value: "x"}}}, - {"/get/abc/123abc/xxx", false, "/get/abc/123abc/:param", Params{Param{Key: "param", Value: "xxx"}}}, - {"/get/abc/123abc/abc", false, "/get/abc/123abc/:param", Params{Param{Key: "param", Value: "abc"}}}, - {"/get/abc/123abc/xxx8xxas", false, "/get/abc/123abc/:param", Params{Param{Key: "param", Value: "xxx8xxas"}}}, - {"/get/abc/123abc/xxx8/1234", false, "/get/abc/123abc/xxx8/1234", nil}, - {"/get/abc/123abc/xxx8/1", false, "/get/abc/123abc/xxx8/:param", Params{Param{Key: "param", Value: "1"}}}, - {"/get/abc/123abc/xxx8/123", false, "/get/abc/123abc/xxx8/:param", Params{Param{Key: "param", Value: "123"}}}, - {"/get/abc/123abc/xxx8/78k", false, "/get/abc/123abc/xxx8/:param", Params{Param{Key: "param", Value: "78k"}}}, - {"/get/abc/123abc/xxx8/1234xxxd", false, "/get/abc/123abc/xxx8/:param", Params{Param{Key: "param", Value: "1234xxxd"}}}, - {"/get/abc/123abc/xxx8/1234/ffas", false, "/get/abc/123abc/xxx8/1234/ffas", nil}, - {"/get/abc/123abc/xxx8/1234/f", false, "/get/abc/123abc/xxx8/1234/:param", Params{Param{Key: "param", Value: "f"}}}, - {"/get/abc/123abc/xxx8/1234/ffa", false, "/get/abc/123abc/xxx8/1234/:param", Params{Param{Key: "param", Value: "ffa"}}}, - {"/get/abc/123abc/xxx8/1234/kka", false, "/get/abc/123abc/xxx8/1234/:param", Params{Param{Key: "param", Value: "kka"}}}, - {"/get/abc/123abc/xxx8/1234/ffas321", false, "/get/abc/123abc/xxx8/1234/:param", Params{Param{Key: "param", Value: "ffas321"}}}, - {"/get/abc/123abc/xxx8/1234/kkdd/12c", false, "/get/abc/123abc/xxx8/1234/kkdd/12c", nil}, - {"/get/abc/123abc/xxx8/1234/kkdd/1", false, "/get/abc/123abc/xxx8/1234/kkdd/:param", Params{Param{Key: "param", Value: "1"}}}, - {"/get/abc/123abc/xxx8/1234/kkdd/12", false, "/get/abc/123abc/xxx8/1234/kkdd/:param", Params{Param{Key: "param", Value: "12"}}}, - {"/get/abc/123abc/xxx8/1234/kkdd/12b", false, "/get/abc/123abc/xxx8/1234/kkdd/:param", Params{Param{Key: "param", Value: "12b"}}}, - {"/get/abc/123abc/xxx8/1234/kkdd/34", false, "/get/abc/123abc/xxx8/1234/kkdd/:param", Params{Param{Key: "param", Value: "34"}}}, - {"/get/abc/123abc/xxx8/1234/kkdd/12c2e3", false, "/get/abc/123abc/xxx8/1234/kkdd/:param", Params{Param{Key: "param", Value: "12c2e3"}}}, - {"/get/abc/12/test", false, "/get/abc/:param/test", Params{Param{Key: "param", Value: "12"}}}, - {"/get/abc/123abdd/test", false, "/get/abc/:param/test", Params{Param{Key: "param", Value: "123abdd"}}}, - {"/get/abc/123abdddf/test", false, "/get/abc/:param/test", Params{Param{Key: "param", Value: "123abdddf"}}}, - {"/get/abc/123ab/test", false, "/get/abc/:param/test", Params{Param{Key: "param", Value: "123ab"}}}, - {"/get/abc/123abgg/test", false, "/get/abc/:param/test", Params{Param{Key: "param", Value: "123abgg"}}}, - {"/get/abc/123abff/test", false, "/get/abc/:param/test", Params{Param{Key: "param", Value: "123abff"}}}, - {"/get/abc/123abffff/test", false, "/get/abc/:param/test", Params{Param{Key: "param", Value: "123abffff"}}}, - {"/get/abc/123abd/test", false, "/get/abc/123abd/:param", Params{Param{Key: "param", Value: "test"}}}, - {"/get/abc/123abddd/test", false, "/get/abc/123abddd/:param", Params{Param{Key: "param", Value: "test"}}}, - {"/get/abc/123/test22", false, "/get/abc/123/:param", Params{Param{Key: "param", Value: "test22"}}}, - {"/get/abc/123abg/test", false, "/get/abc/123abg/:param", Params{Param{Key: "param", Value: "test"}}}, - {"/get/abc/123abf/testss", false, "/get/abc/123abf/:param", Params{Param{Key: "param", Value: "testss"}}}, - {"/get/abc/123abfff/te", false, "/get/abc/123abfff/:param", Params{Param{Key: "param", Value: "te"}}}, }) checkPriorities(t, tree)