Skip to content

Commit

Permalink
Fix conflict between param and exact path (#2706)
Browse files Browse the repository at this point in the history
* Fix conflict between param and exact path

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Add test

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Fix prefix conflict in exact paths

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Use backtracking

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>

* Fix panic

Signed-off-by: Yue Yang <g1enyy0ung@gmail.com>
  • Loading branch information
g1eny0ung authored and thinkerou committed Nov 23, 2021
1 parent 77649bc commit 3f28f99
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
29 changes: 29 additions & 0 deletions tree.go
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -411,6 +418,21 @@ walk: // Outer loop for walking the tree
idxc := path[0]
for i, c := range []byte(n.indices) {
if c == idxc {
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,
},
}
}

n = n.children[i]
continue walk
}
Expand Down Expand Up @@ -542,6 +564,13 @@ walk: // Outer loop for walking the tree
return
}

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 == "/") ||
Expand Down
18 changes: 15 additions & 3 deletions tree_test.go
Expand Up @@ -135,13 +135,16 @@ 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/",
"/search/:query",
"/search/gin-gonic",
"/search/google",
"/user_:name",
"/user_:name/about",
"/files/:dir/*filepath",
Expand All @@ -150,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))
Expand All @@ -159,21 +163,29 @@ 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},
{"/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"}}},
{"/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)
Expand Down

0 comments on commit 3f28f99

Please sign in to comment.