1
0
mirror of https://github.com/labstack/echo.git synced 2025-04-23 12:18:53 +02:00

Fix #1493 router loop for param routes (#1501)

* Add test to reproduce router loop for #1493

* Simplify and correct  router param tests

* Fix #1493 to avoid router loop for param nodes
This commit is contained in:
Roland Lammel 2020-02-19 16:10:57 +01:00 committed by GitHub
parent 504f39abaf
commit f4b5a90ad3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 61 additions and 33 deletions

View File

@ -376,8 +376,8 @@ func (r *Router) Find(method, path string, c Context) {
continue continue
} }
// Param node
Param: Param:
// Param node
if child = cn.findChildByKind(pkind); child != nil { if child = cn.findChildByKind(pkind); child != nil {
// Issue #378 // Issue #378
if len(pvalues) == n { if len(pvalues) == n {
@ -401,24 +401,37 @@ func (r *Router) Find(method, path string, c Context) {
continue continue
} }
// Any node
Any: Any:
if cn = cn.findChildByKind(akind); cn == nil { // Any node
if cn = cn.findChildByKind(akind); cn != nil {
// If any node is found, use remaining path for pvalues
pvalues[len(cn.pnames)-1] = search
break
}
// No node found, continue at stored next node
// or find nearest "any" route
if nn != nil { if nn != nil {
// No next node to go down in routing (issue #954) // No next node to go down in routing (issue #954)
// Find nearest "any" route going up the routing tree // Find nearest "any" route going up the routing tree
search = ns search = ns
np := nn.parent np := nn.parent
// Consider param route one level up only // Consider param route one level up only
// if no slash is remaining in search string if cn = nn.findChildByKind(pkind); cn != nil {
if cn = nn.findChildByKind(pkind); cn != nil && strings.IndexByte(ns, '/') == -1 { pos := strings.IndexByte(ns, '/')
if pos == -1 {
// If no slash is remaining in search string set param value
pvalues[len(cn.pnames)-1] = search pvalues[len(cn.pnames)-1] = search
break break
} else if cn != nil && strings.IndexByte(ns, '/') != 1 { } else if pos > 0 {
// If slash is present, it means that this is a parameterized route. // Otherwise continue route processing with restored next node
cn = cn.parent cn = nn
nn = nil
ns = ""
goto Param goto Param
} }
}
// No param route found, try to resolve nearest any route
for { for {
np = nn.parent np = nn.parent
if cn = nn.findChildByKind(akind); cn != nil { if cn = nn.findChildByKind(akind); cn != nil {
@ -439,9 +452,7 @@ func (r *Router) Find(method, path string, c Context) {
} }
} }
return // Not found return // Not found
}
pvalues[len(cn.pnames)-1] = search
break
} }
ctx.handler = cn.findHandler(method) ctx.handler = cn.findHandler(method)

View File

@ -1031,12 +1031,15 @@ func TestRouterParamBacktraceNotFound(t *testing.T) {
r.Find(http.MethodGet, "/a", c) r.Find(http.MethodGet, "/a", c)
assert.Equal(t, "a", c.Param("param1")) assert.Equal(t, "a", c.Param("param1"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/a/foo", c) r.Find(http.MethodGet, "/a/foo", c)
assert.Equal(t, "a", c.Param("param1")) assert.Equal(t, "a", c.Param("param1"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/a/bar", c) r.Find(http.MethodGet, "/a/bar", c)
assert.Equal(t, "a", c.Param("param1")) assert.Equal(t, "a", c.Param("param1"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/a/bar/b", c) r.Find(http.MethodGet, "/a/bar/b", c)
assert.Equal(t, "a", c.Param("param1")) assert.Equal(t, "a", c.Param("param1"))
assert.Equal(t, "b", c.Param("param2")) assert.Equal(t, "b", c.Param("param2"))
@ -1157,31 +1160,45 @@ func TestRouterParam1466(t *testing.T) {
r.Find(http.MethodGet, "/users/ajitem", c) r.Find(http.MethodGet, "/users/ajitem", c)
assert.Equal(t, "ajitem", c.Param("username")) assert.Equal(t, "ajitem", c.Param("username"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/sharewithme", c) r.Find(http.MethodGet, "/users/sharewithme", c)
assert.Equal(t, "sharewithme", c.Param("username")) assert.Equal(t, "sharewithme", c.Param("username"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/signup", c) r.Find(http.MethodGet, "/users/signup", c)
assert.Equal(t, "", c.Param("username")) assert.Equal(t, "", c.Param("username"))
// Additional assertions for #1479 // Additional assertions for #1479
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/sharewithme/likes/projects/ids", c) r.Find(http.MethodGet, "/users/sharewithme/likes/projects/ids", c)
assert.Equal(t, "sharewithme", c.Param("username")) assert.Equal(t, "sharewithme", c.Param("username"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/ajitem/likes/projects/ids", c) r.Find(http.MethodGet, "/users/ajitem/likes/projects/ids", c)
assert.Equal(t, "ajitem", c.Param("username")) assert.Equal(t, "ajitem", c.Param("username"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/sharewithme/profile", c) r.Find(http.MethodGet, "/users/sharewithme/profile", c)
assert.Equal(t, "sharewithme", c.Param("username")) assert.Equal(t, "sharewithme", c.Param("username"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/ajitem/profile", c) r.Find(http.MethodGet, "/users/ajitem/profile", c)
assert.Equal(t, "ajitem", c.Param("username")) assert.Equal(t, "ajitem", c.Param("username"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/sharewithme/uploads/self", c) r.Find(http.MethodGet, "/users/sharewithme/uploads/self", c)
assert.Equal(t, "sharewithme", c.Param("username")) assert.Equal(t, "sharewithme", c.Param("username"))
assert.Equal(t, "self", c.Param("type")) assert.Equal(t, "self", c.Param("type"))
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/ajitem/uploads/self", c) r.Find(http.MethodGet, "/users/ajitem/uploads/self", c)
assert.Equal(t, "ajitem", c.Param("username")) assert.Equal(t, "ajitem", c.Param("username"))
assert.Equal(t, "self", c.Param("type")) assert.Equal(t, "self", c.Param("type"))
// Issue #1493 - check for routing loop
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/tree/free", c)
assert.Equal(t, "", c.Param("id"))
assert.Equal(t, 0, c.response.Status)
} }
func benchmarkRouterRoutes(b *testing.B, routes []*Route) { func benchmarkRouterRoutes(b *testing.B, routes []*Route) {