Skip to content

Commit

Permalink
refs labstack#1412: Fix multi level match any routes
Browse files Browse the repository at this point in the history
  • Loading branch information
lammel committed Oct 16, 2019
1 parent d286e28 commit 8d47b34
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 13 deletions.
40 changes: 27 additions & 13 deletions router.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package echo

import "net/http"
import (
"net/http"
"strings"
)

type (
// Router is the registry of all registered routes for an `Echo` instance for
Expand All @@ -20,8 +23,8 @@ type (
pnames []string
methodHandler *methodHandler
}
kind uint8
children []*node
kind uint8
children []*node
methodHandler struct {
connect HandlerFunc
delete HandlerFunc
Expand Down Expand Up @@ -336,7 +339,6 @@ func (r *Router) Find(method, path string, c Context) {
}
}


if l == pl {
// Continue search
search = search[l:]
Expand Down Expand Up @@ -398,16 +400,28 @@ func (r *Router) Find(method, path string, c Context) {
Any:
if cn = cn.findChildByKind(akind); cn == nil {
if nn != nil {
cn = nn
nn = cn.parent // Next (Issue #954)
if nn != nil {
nk = nn.kind
}
// No next node to go down in routing (issue #954)
// Find nearest "any" route going up the routing tree
search = ns
if nk == pkind {
goto Param
} else if nk == akind {
goto Any
np := nn.parent
// Consider param route one level up only
// if no slash is remaining in search string
if cn = nn.findChildByKind(pkind); cn != nil && strings.IndexByte(ns, '/') == -1 {
break
}
for {
np = nn.parent
if cn = nn.findChildByKind(akind); cn != nil {
break
}
if np == nil {
break // no further parent nodes in tree, abort
}
nn = np
}
if cn != nil { // use the found "any" route and update path
pvalues[len(cn.pnames)-1] = search
break
}
}
return // Not found
Expand Down
65 changes: 65 additions & 0 deletions router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,20 +595,41 @@ func TestRouterMatchAnyMultiLevel(t *testing.T) {
// Routes
r.Add(http.MethodGet, "/api/users/jack", handler)
r.Add(http.MethodGet, "/api/users/jill", handler)
r.Add(http.MethodGet, "/api/users/*", handler)
r.Add(http.MethodGet, "/api/*", handler)
r.Add(http.MethodGet, "/other/*", handler)
r.Add(http.MethodGet, "/*", handler)

c := e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/api/users/jack", c)
c.handler(c)
assert.Equal(t, "/api/users/jack", c.Get("path"))
assert.Equal(t, "", c.Param("*"))

r.Find(http.MethodGet, "/api/users/jill", c)
c.handler(c)
assert.Equal(t, "/api/users/jill", c.Get("path"))
assert.Equal(t, "", c.Param("*"))

r.Find(http.MethodGet, "/api/users/joe", c)
c.handler(c)
assert.Equal(t, "/api/users/*", c.Get("path"))
assert.Equal(t, "joe", c.Param("*"))

r.Find(http.MethodGet, "/api/nousers/joe", c)
c.handler(c)
assert.Equal(t, "/api/*", c.Get("path"))
assert.Equal(t, "nousers/joe", c.Param("*"))

r.Find(http.MethodGet, "/api/none", c)
c.handler(c)
assert.Equal(t, "/api/*", c.Get("path"))
assert.Equal(t, "none", c.Param("*"))

r.Find(http.MethodGet, "/noapi/users/jim", c)
c.handler(c)
assert.Equal(t, "/*", c.Get("path"))
assert.Equal(t, "noapi/users/jim", c.Param("*"))
}

func TestRouterMicroParam(t *testing.T) {
Expand Down Expand Up @@ -702,6 +723,15 @@ func TestRouterPriority(t *testing.T) {
c.Set("g", 7)
return nil
})
r.Add(http.MethodGet, "/users/new/*", func(c Context) error {
c.Set("h", 8)
return nil
})
r.Add(http.MethodGet, "/*", func(c Context) error {
c.Set("i", 9)
return nil
})

c := e.NewContext(nil, nil).(*context)

// Route > /users
Expand Down Expand Up @@ -734,11 +764,46 @@ func TestRouterPriority(t *testing.T) {
c.handler(c)
assert.Equal(t, 3, c.Get("c"))

// Route > /users/newsee
r.Find(http.MethodGet, "/users/newsee", c)
c.handler(c)
assert.Equal(t, 6, c.Get("f"))

// Route > /users/*
r.Find(http.MethodGet, "/users/joe/books", c)
c.handler(c)
assert.Equal(t, 7, c.Get("g"))
assert.Equal(t, "joe/books", c.Param("*"))

// Route > /users/new/* should be matched
r.Find(http.MethodGet, "/users/new/someone", c)
c.handler(c)
assert.Equal(t, 8, c.Get("h"))
assert.Equal(t, "someone", c.Param("*"))

// Route > /users/* should be matched although /users/dew exists
r.Find(http.MethodGet, "/users/dew/someone", c)
c.handler(c)
assert.Equal(t, 7, c.Get("g"))
assert.Equal(t, "dew/someone", c.Param("*"))

// Route > /users/* should be matched although /users/dew exists
r.Find(http.MethodGet, "/users/notexists/someone/else", c)
c.handler(c)
assert.Equal(t, 7, c.Get("g"))
assert.Equal(t, "notexists/someone/else", c.Param("*"))

// Route > *
r.Find(http.MethodGet, "/nousers", c)
c.handler(c)
assert.Equal(t, 9, c.Get("i"))
assert.Equal(t, "nousers", c.Param("*"))

// Route > *
r.Find(http.MethodGet, "/nousers/new", c)
c.handler(c)
assert.Equal(t, 9, c.Get("i"))
assert.Equal(t, "nousers/new", c.Param("*"))
}

func TestRouterIssue1348(t *testing.T) {
Expand Down

0 comments on commit 8d47b34

Please sign in to comment.