Skip to content

Commit

Permalink
Add support for wildcard paths with other children
Browse files Browse the repository at this point in the history
  • Loading branch information
rw-access committed Mar 26, 2021
1 parent fe77dd0 commit 50dd04c
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 65 deletions.
110 changes: 56 additions & 54 deletions tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func longestCommonPrefix(a, b string) int {

// Search for a wildcard segment and check the name for invalid characters.
// Returns -1 as index, if no wildcard was found.
func findWildcard(path string) (wilcard string, i int, valid bool) {
func findWildcard(path string) (wildcard string, i int, valid bool) {
// Find start
for start, c := range []byte(path) {
// A wildcard starts with ':' (param) or '*' (catch-all)
Expand Down Expand Up @@ -81,6 +81,16 @@ type node struct {
handle Handle
}

// addChild will add a child node, keeping wildcards at the end
func (n *node) addChild(child *node) {
if n.wildChild && len(n.children) > 0 {
wildcardChild := n.children[len(n.children)-1]
n.children = append(n.children[:len(n.children)-1], child, wildcardChild)
} else {
n.children = append(n.children, child)
}
}

// Increments priority of the given child and reorders if necessary
func (n *node) incrementChildPrio(pos int) int {
cs := n.children
Expand Down Expand Up @@ -137,20 +147,46 @@ walk:
}

n.children = []*node{&child}
n.wildChild = false
// []byte for proper unicode char conversion, see #65
n.indices = string([]byte{n.path[i]})
n.path = path[:i]
n.handle = nil
n.wildChild = false
}

// Make new node a child of this node
if i < len(path) {
path = path[i:]
idxc := path[0]

if n.wildChild {
// '/' after param
if n.nType == param && idxc == '/' && len(n.children) == 1 {
n = n.children[0]
n.priority++
continue walk
}

// Check if a child with the next path byte exists
for i, c := range []byte(n.indices) {
if c == idxc {
i = n.incrementChildPrio(i)
n = n.children[i]
continue walk
}
}

// Otherwise insert it
if idxc != ':' && idxc != '*' && n.nType != catchAll {
// []byte for proper unicode char conversion, see #65
n.indices += string([]byte{idxc})
child := &node{}
n.addChild(child)
n.incrementChildPrio(len(n.indices) - 1)
n = child
} else if n.wildChild {
// inserting a wildcard node, need to check if it conflicts with the existing wildcard
n = n.children[len(n.children)-1]
n.priority++

// Check if the wildcard matches
if len(path) >= len(n.path) && n.path == path[:len(n.path)] &&
Expand All @@ -174,33 +210,6 @@ walk:
}
}

idxc := path[0]

// '/' after param
if n.nType == param && idxc == '/' && len(n.children) == 1 {
n = n.children[0]
n.priority++
continue walk
}

// Check if a child with the next path byte exists
for i, c := range []byte(n.indices) {
if c == idxc {
i = n.incrementChildPrio(i)
n = n.children[i]
continue walk
}
}

// Otherwise insert it
if idxc != ':' && idxc != '*' {
// []byte for proper unicode char conversion, see #65
n.indices += string([]byte{idxc})
child := &node{}
n.children = append(n.children, child)
n.incrementChildPrio(len(n.indices) - 1)
n = child
}
n.insertChild(path, fullPath, handle)
return
}
Expand All @@ -218,7 +227,7 @@ func (n *node) insertChild(path, fullPath string, handle Handle) {
for {
// Find prefix until first wildcard
wildcard, i, valid := findWildcard(path)
if i < 0 { // No wilcard found
if i < 0 { // No wildcard found
break
}

Expand All @@ -233,13 +242,6 @@ func (n *node) insertChild(path, fullPath string, handle Handle) {
panic("wildcards must be named with a non-empty name in path '" + fullPath + "'")
}

// Check if this node has existing children which would be
// unreachable if we insert the wildcard here
if len(n.children) > 0 {
panic("wildcard segment '" + wildcard +
"' conflicts with existing children in path '" + fullPath + "'")
}

// param
if wildcard[0] == ':' {
if i > 0 {
Expand All @@ -248,12 +250,12 @@ func (n *node) insertChild(path, fullPath string, handle Handle) {
path = path[i:]
}

n.wildChild = true
child := &node{
nType: param,
path: wildcard,
}
n.children = []*node{child}
n.addChild(child)
n.wildChild = true
n = child
n.priority++

Expand All @@ -264,7 +266,7 @@ func (n *node) insertChild(path, fullPath string, handle Handle) {
child := &node{
priority: 1,
}
n.children = []*node{child}
n.addChild(child)
n = child
continue
}
Expand Down Expand Up @@ -296,7 +298,7 @@ func (n *node) insertChild(path, fullPath string, handle Handle) {
wildChild: true,
nType: catchAll,
}
n.children = []*node{child}
n.addChild(child)
n.indices = string('/')
n = child
n.priority++
Expand Down Expand Up @@ -331,27 +333,27 @@ walk: // Outer loop for walking the tree
if path[:len(prefix)] == prefix {
path = path[len(prefix):]

// If this node does not have a wildcard (param or catchAll)
// child, we can just look up the next child node and continue
// to walk down the tree
if !n.wildChild {
idxc := path[0]
for i, c := range []byte(n.indices) {
if c == idxc {
n = n.children[i]
continue walk
}
// Try all the non-wildcard children first by matching the indices
idxc := path[0]
for i, c := range []byte(n.indices) {
if c == idxc {
n = n.children[i]
continue walk
}
}

// 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.
tsr = (path == "/" && n.handle != nil)
return
}

// Handle wildcard child
n = n.children[0]
// Handle wildcard child, which is always after the other children
n = n.children[len(n.children)-1]

switch n.nType {
case param:
// Find param end (either '/' or path end)
Expand Down
49 changes: 38 additions & 11 deletions tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ func TestTreeWildcard(t *testing.T) {
"/",
"/cmd/:tool/:sub",
"/cmd/:tool/",
"/cmd/whoami",
"/cmd/whoami/root/",
"/src/*filepath",
"/search/",
"/search/:query",
Expand All @@ -168,7 +170,12 @@ func TestTreeWildcard(t *testing.T) {

checkRequests(t, tree, testRequests{
{"/", false, "/", nil},
{"/cmd/test", true, "/cmd/:tool/", Params{Param{"tool", "test"}}},
{"/cmd/test/", false, "/cmd/:tool/", Params{Param{"tool", "test"}}},
{"/cmd/whoami", false, "/cmd/whoami", nil},
{"/cmd/whoami/", true, "/cmd/whoami", nil},
{"/cmd/whoami/root/", false, "/cmd/whoami/root/", nil},
{"/cmd/whoami/root", true, "/cmd/whoami/root/", nil},
{"/cmd/test", true, "", Params{Param{"tool", "test"}}},
{"/cmd/test/3", false, "/cmd/:tool/:sub", Params{Param{"tool", "test"}, Param{"sub", "3"}}},
{"/src/", false, "/src/*filepath", Params{Param{"filepath", "/"}}},
Expand Down Expand Up @@ -224,41 +231,62 @@ func testRoutes(t *testing.T, routes []testRoute) {
func TestTreeWildcardConflict(t *testing.T) {
routes := []testRoute{
{"/cmd/:tool/:sub", false},
{"/cmd/vet", true},
{"/cmd/vet", false},
{"/foo/bar", false},
{"/foo/:name", false},
{"/foo/:names", true},
{"/cmd/*path", true},
{"/cmd/:badvar", true},
{"/cmd/:tool/names", false},
{"/cmd/:tool/:badsub/details", true},
{"/src/*filepath", false},
{"/src/:file", true},
{"/src/static.json", true},
{"/src/*filepathx", true},
{"/src/", true},
{"/src/foo/bar", true},
{"/src1/", false},
{"/src1/*filepath", true},
{"/src2*filepath", true},
{"/src2/*filepath", false},
{"/search/:query", false},
{"/search/invalid", true},
{"/search/valid", false},
{"/user_:name", false},
{"/user_x", true},
{"/user_x", false},
{"/user_:name", false},
{"/id:id", false},
{"/id/:id", true},
{"/id/:id", false},
}
testRoutes(t, routes)
}

func TestCatchAllAfterSlash(t *testing.T) {
routes := []testRoute{
{"/non-leading-*catchall", true},
}
testRoutes(t, routes)
}

func TestTreeChildConflict(t *testing.T) {
routes := []testRoute{
{"/cmd/vet", false},
{"/cmd/:tool/:sub", true},
{"/cmd/:tool", false},
{"/cmd/:tool/:sub", false},
{"/cmd/:tool/misc", false},
{"/cmd/:tool/:othersub", true},
{"/src/AUTHORS", false},
{"/src/*filepath", true},
{"/user_x", false},
{"/user_:name", true},
{"/user_:name", false},
{"/id/:id", false},
{"/id:id", true},
{"/:id", true},
{"/id:id", false},
{"/:id", false},
{"/*filepath", true},
}
testRoutes(t, routes)
}

func TestTreeDupliatePath(t *testing.T) {
func TestTreeDuplicatePath(t *testing.T) {
tree := &node{}

routes := [...]string{
Expand Down Expand Up @@ -668,8 +696,7 @@ func TestTreeWildcardConflictEx(t *testing.T) {
{"/who/are/foo", "/foo", `/who/are/\*you`, `/\*you`},
{"/who/are/foo/", "/foo/", `/who/are/\*you`, `/\*you`},
{"/who/are/foo/bar", "/foo/bar", `/who/are/\*you`, `/\*you`},
{"/conxxx", "xxx", `/con:tact`, `:tact`},
{"/conooo/xxx", "ooo", `/con:tact`, `:tact`},
{"/con:nection", ":nection", `/con:tact`, `:tact`},
}

for i := range conflicts {
Expand Down

0 comments on commit 50dd04c

Please sign in to comment.