Skip to content

Commit

Permalink
Fix #1526 trailing slash to any route (#1563)
Browse files Browse the repository at this point in the history
* refs #1526: Add tests for trailing slash requests with nested any routes

* refs #1526: Handle specual router case with trailing slash for non-root any route

* refs #1526: Fix accidential lookup for any route without trailing slash in request
  • Loading branch information
lammel authored May 6, 2020
1 parent c08f303 commit 43e32ba
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 1 deletion.
4 changes: 4 additions & 0 deletions router.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,10 @@ func (r *Router) Find(method, path string, c Context) {

// Attempt to go back up the tree on no matching prefix or no remaining search
if l != pl || search == "" {
// Handle special case of trailing slash route with existing any route (see #1526)
if path[len(path)-1] == '/' && cn.findChildByKind(akind) != nil {
goto Any
}
if nn == nil { // Issue #1348
return // Not found
}
Expand Down
73 changes: 72 additions & 1 deletion router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,6 @@ func TestRouterMatchAny(t *testing.T) {
return nil
})
c := e.NewContext(nil, nil).(*context)

r.Find(http.MethodGet, "/", c)
assert.Equal(t, "", c.Param("*"))

Expand All @@ -619,6 +618,78 @@ func TestRouterMatchAny(t *testing.T) {
assert.Equal(t, "joe", c.Param("*"))
}

// TestRouterMatchAnySlash shall verify finding the best route
// for any routes with trailing slash requests
func TestRouterMatchAnySlash(t *testing.T) {
e := New()
r := e.router

handler := func(c Context) error {
c.Set("path", c.Path())
return nil
}

// Routes
r.Add(http.MethodGet, "/users", handler)
r.Add(http.MethodGet, "/users/*", handler)
r.Add(http.MethodGet, "/img/*", handler)
r.Add(http.MethodGet, "/img/load", handler)
r.Add(http.MethodGet, "/img/load/*", handler)
r.Add(http.MethodGet, "/assets/*", handler)

c := e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/", c)
assert.Equal(t, "", c.Param("*"))

// Test trailing slash request for simple any route (see #1526)
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/users/", c)
c.handler(c)
assert.Equal(t, "/users/*", c.Get("path"))
assert.Equal(t, "", c.Param("*"))

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

// Test trailing slash request for nested any route (see #1526)
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/img/load", c)
c.handler(c)
assert.Equal(t, "/img/load", c.Get("path"))
assert.Equal(t, "", c.Param("*"))

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

c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/img/load/ben", c)
c.handler(c)
assert.Equal(t, "/img/load/*", c.Get("path"))
assert.Equal(t, "ben", c.Param("*"))

// Test /assets/* any route
// ... without trailing slash must not match
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/assets", c)
c.handler(c)
assert.Equal(t, nil, c.Get("path"))
assert.Equal(t, "", c.Param("*"))

// ... with trailing slash must match
c = e.NewContext(nil, nil).(*context)
r.Find(http.MethodGet, "/assets/", c)
c.handler(c)
assert.Equal(t, "/assets/*", c.Get("path"))
assert.Equal(t, "", c.Param("*"))

}

func TestRouterMatchAnyMultiLevel(t *testing.T) {
e := New()
r := e.router
Expand Down

0 comments on commit 43e32ba

Please sign in to comment.