From 801d7b3bd62dcf0e2f2aa6144e6930db08bf6c98 Mon Sep 17 00:00:00 2001 From: Yue Yang Date: Sun, 25 Apr 2021 17:29:42 +0800 Subject: [PATCH 1/5] Fix conflict between param and exact path Signed-off-by: Yue Yang --- tree.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tree.go b/tree.go index ca753e6d27..a0df321ff0 100644 --- a/tree.go +++ b/tree.go @@ -411,8 +411,14 @@ walk: // Outer loop for walking the tree idxc := path[0] for i, c := range []byte(n.indices) { if c == idxc { - n = n.children[i] - continue walk + childWillWalkTo := n.children[i] + + if strings.Split(childWillWalkTo.path, "/")[0] != strings.Split(path, "/")[0] && strings.HasPrefix(n.children[len(n.children)-1].path, ":") { + continue + } else { + n = childWillWalkTo + continue walk + } } } From 39fa5a345f146ca7580350c4e78e4ff14818ebab Mon Sep 17 00:00:00 2001 From: Yue Yang Date: Sun, 25 Apr 2021 18:01:51 +0800 Subject: [PATCH 2/5] Add test Signed-off-by: Yue Yang --- tree_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tree_test.go b/tree_test.go index d7c4fb0b9c..2f372bc029 100644 --- a/tree_test.go +++ b/tree_test.go @@ -142,6 +142,7 @@ func TestTreeWildcard(t *testing.T) { "/src/*filepath", "/search/", "/search/:query", + "/search/gin-gonic", "/user_:name", "/user_:name/about", "/files/:dir/*filepath", @@ -169,6 +170,7 @@ func TestTreeWildcard(t *testing.T) { {"/search/", false, "/search/", nil}, {"/search/someth!ng+in+ünìcodé", false, "/search/:query", Params{Param{Key: "query", Value: "someth!ng+in+ünìcodé"}}}, {"/search/someth!ng+in+ünìcodé/", true, "", Params{Param{Key: "query", Value: "someth!ng+in+ünìcodé"}}}, + {"/search/gin", false, "/search/:query", Params{Param{"query", "gin"}}}, {"/user_gopher", false, "/user_:name", Params{Param{Key: "name", Value: "gopher"}}}, {"/user_gopher/about", false, "/user_:name/about", Params{Param{Key: "name", Value: "gopher"}}}, {"/files/js/inc/framework.js", false, "/files/:dir/*filepath", Params{Param{Key: "dir", Value: "js"}, Param{Key: "filepath", Value: "/inc/framework.js"}}}, From 7c9e3b0e29efcced26c3e38ec25eb2d48f56247c Mon Sep 17 00:00:00 2001 From: Yue Yang Date: Wed, 28 Apr 2021 14:56:07 +0800 Subject: [PATCH 3/5] Fix prefix conflict in exact paths Signed-off-by: Yue Yang --- tree.go | 16 +++++++++++++--- tree_test.go | 3 +++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/tree.go b/tree.go index a0df321ff0..7cda7e7f41 100644 --- a/tree.go +++ b/tree.go @@ -414,11 +414,21 @@ walk: // Outer loop for walking the tree childWillWalkTo := n.children[i] if strings.Split(childWillWalkTo.path, "/")[0] != strings.Split(path, "/")[0] && strings.HasPrefix(n.children[len(n.children)-1].path, ":") { + for _, child := range childWillWalkTo.children { + cPath := string(c) + child.path + + if cPath == path { + child.path = cPath + n = child + continue walk + } + } + continue - } else { - n = childWillWalkTo - continue walk } + + n = childWillWalkTo + continue walk } } diff --git a/tree_test.go b/tree_test.go index 2f372bc029..77340bd37a 100644 --- a/tree_test.go +++ b/tree_test.go @@ -143,6 +143,7 @@ func TestTreeWildcard(t *testing.T) { "/search/", "/search/:query", "/search/gin-gonic", + "/search/google", "/user_:name", "/user_:name/about", "/files/:dir/*filepath", @@ -171,6 +172,8 @@ func TestTreeWildcard(t *testing.T) { {"/search/someth!ng+in+ünìcodé", false, "/search/:query", Params{Param{Key: "query", Value: "someth!ng+in+ünìcodé"}}}, {"/search/someth!ng+in+ünìcodé/", true, "", Params{Param{Key: "query", Value: "someth!ng+in+ünìcodé"}}}, {"/search/gin", false, "/search/:query", Params{Param{"query", "gin"}}}, + {"/search/gin-gonic", false, "/search/gin-gonic", nil}, + {"/search/google", false, "/search/google", nil}, {"/user_gopher", false, "/user_:name", Params{Param{Key: "name", Value: "gopher"}}}, {"/user_gopher/about", false, "/user_:name/about", Params{Param{Key: "name", Value: "gopher"}}}, {"/files/js/inc/framework.js", false, "/files/:dir/*filepath", Params{Param{Key: "dir", Value: "js"}, Param{Key: "filepath", Value: "/inc/framework.js"}}}, From 3bb530800481d505e4a2fa8d3b1ba80443a81aaa Mon Sep 17 00:00:00 2001 From: Yue Yang Date: Thu, 29 Apr 2021 09:21:29 +0800 Subject: [PATCH 4/5] Use backtracking Signed-off-by: Yue Yang --- tree.go | 41 +++++++++++++++++++++++++++-------------- tree_test.go | 13 ++++++++++--- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/tree.go b/tree.go index 7cda7e7f41..2ab92a52dd 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,6 +405,8 @@ 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 skipped *skip + walk: // Outer loop for walking the tree for { prefix := n.path @@ -411,23 +418,22 @@ walk: // Outer loop for walking the tree idxc := path[0] for i, c := range []byte(n.indices) { if c == idxc { - childWillWalkTo := n.children[i] - - if strings.Split(childWillWalkTo.path, "/")[0] != strings.Split(path, "/")[0] && strings.HasPrefix(n.children[len(n.children)-1].path, ":") { - for _, child := range childWillWalkTo.children { - cPath := string(c) + child.path - - if cPath == path { - child.path = cPath - n = child - continue walk - } + 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, + }, } - - continue } - n = childWillWalkTo + n = n.children[i] continue walk } } @@ -558,6 +564,13 @@ walk: // Outer loop for walking the tree return } + if path != "/" && 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 == "/") || diff --git a/tree_test.go b/tree_test.go index 77340bd37a..298c5ed0c5 100644 --- a/tree_test.go +++ b/tree_test.go @@ -135,9 +135,10 @@ func TestTreeWildcard(t *testing.T) { routes := [...]string{ "/", - "/cmd/:tool/:sub", "/cmd/:tool/", + "/cmd/:tool/:sub", "/cmd/whoami", + "/cmd/whoami/root", "/cmd/whoami/root/", "/src/*filepath", "/search/", @@ -152,6 +153,7 @@ func TestTreeWildcard(t *testing.T) { "/doc/go1.html", "/info/:user/public", "/info/:user/project/:project", + "/info/:user/project/golang", } for _, route := range routes { tree.addRoute(route, fakeHandler(route)) @@ -161,11 +163,15 @@ func TestTreeWildcard(t *testing.T) { {"/", false, "/", nil}, {"/cmd/test", true, "/cmd/:tool/", Params{Param{"tool", "test"}}}, {"/cmd/test/", false, "/cmd/:tool/", Params{Param{"tool", "test"}}}, + {"/cmd/test/3", false, "/cmd/:tool/:sub", Params{Param{Key: "tool", Value: "test"}, Param{Key: "sub", Value: "3"}}}, + {"/cmd/who", true, "/cmd/:tool/", Params{Param{"tool", "who"}}}, + {"/cmd/who/", false, "/cmd/:tool/", Params{Param{"tool", "who"}}}, {"/cmd/whoami", false, "/cmd/whoami", nil}, {"/cmd/whoami/", true, "/cmd/whoami", nil}, + {"/cmd/whoami/r", false, "/cmd/:tool/:sub", Params{Param{Key: "tool", Value: "whoami"}, Param{Key: "sub", Value: "r"}}}, + {"/cmd/whoami/r/", true, "/cmd/:tool/:sub", Params{Param{Key: "tool", Value: "whoami"}, Param{Key: "sub", Value: "r"}}}, + {"/cmd/whoami/root", false, "/cmd/whoami/root", nil}, {"/cmd/whoami/root/", false, "/cmd/whoami/root/", nil}, - {"/cmd/whoami/root", true, "/cmd/whoami/root/", nil}, - {"/cmd/test/3", false, "/cmd/:tool/:sub", Params{Param{Key: "tool", Value: "test"}, Param{Key: "sub", Value: "3"}}}, {"/src/", false, "/src/*filepath", Params{Param{Key: "filepath", Value: "/"}}}, {"/src/some/file.png", false, "/src/*filepath", Params{Param{Key: "filepath", Value: "/some/file.png"}}}, {"/search/", false, "/search/", nil}, @@ -179,6 +185,7 @@ func TestTreeWildcard(t *testing.T) { {"/files/js/inc/framework.js", false, "/files/:dir/*filepath", Params{Param{Key: "dir", Value: "js"}, Param{Key: "filepath", Value: "/inc/framework.js"}}}, {"/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"}}}, }) checkPriorities(t, tree) From e75fe4a44257cab4ca16f5283beef9c95c042832 Mon Sep 17 00:00:00 2001 From: Yue Yang Date: Thu, 29 Apr 2021 10:24:38 +0800 Subject: [PATCH 5/5] Fix panic Signed-off-by: Yue Yang --- tree.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tree.go b/tree.go index 2ab92a52dd..0d082d0571 100644 --- a/tree.go +++ b/tree.go @@ -564,7 +564,7 @@ walk: // Outer loop for walking the tree return } - if path != "/" && strings.HasSuffix(skipped.path, path) { + if path != "/" && skipped != nil && strings.HasSuffix(skipped.path, path) { path = skipped.path n = skipped.paramNode skipped = nil