Skip to content

Commit

Permalink
Fix adding route with host overwrites default host route with same me…
Browse files Browse the repository at this point in the history
…thod+path in list of routes.
  • Loading branch information
aldas committed Dec 25, 2022
1 parent 895121d commit f1cf1ec
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 82 deletions.
44 changes: 7 additions & 37 deletions echo.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ Learn more at https://echo.labstack.com
package echo

import (
"bytes"
stdContext "context"
"crypto/tls"
"errors"
Expand Down Expand Up @@ -528,20 +527,13 @@ func (e *Echo) File(path, file string, m ...MiddlewareFunc) *Route {
}

func (e *Echo) add(host, method, path string, handler HandlerFunc, middleware ...MiddlewareFunc) *Route {
name := handlerName(handler)
router := e.findRouter(host)
// FIXME: when handler+middleware are both nil ... make it behave like handler removal
router.Add(method, path, func(c Context) error {
//FIXME: when handler+middleware are both nil ... make it behave like handler removal
name := handlerName(handler)
return router.add(method, path, name, func(c Context) error {
h := applyMiddleware(handler, middleware...)
return h(c)
})
r := &Route{
Method: method,
Path: path,
Name: name,
}
e.router.routes[method+path] = r
return r
}

// Add registers a new route for an HTTP method and path with matching handler
Expand Down Expand Up @@ -578,35 +570,13 @@ func (e *Echo) URL(h HandlerFunc, params ...interface{}) string {

// Reverse generates an URL from route name and provided parameters.
func (e *Echo) Reverse(name string, params ...interface{}) string {
uri := new(bytes.Buffer)
ln := len(params)
n := 0
for _, r := range e.router.routes {
if r.Name == name {
for i, l := 0, len(r.Path); i < l; i++ {
if (r.Path[i] == ':' || r.Path[i] == '*') && n < ln {
for ; i < l && r.Path[i] != '/'; i++ {
}
uri.WriteString(fmt.Sprintf("%v", params[n]))
n++
}
if i < l {
uri.WriteByte(r.Path[i])
}
}
break
}
}
return uri.String()
return e.router.Reverse(name, params...)
}

// Routes returns the registered routes.
// Routes returns the registered routes for default router.
// In case when Echo serves multiple hosts/domains use `e.Routers()["domain2.site"].Routes()` to get specific host routes.
func (e *Echo) Routes() []*Route {
routes := make([]*Route, 0, len(e.router.routes))
for _, v := range e.router.routes {
routes = append(routes, v)
}
return routes
return e.router.Routes()
}

// AcquireContext returns an empty `Context` instance from the pool.
Expand Down
88 changes: 69 additions & 19 deletions echo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,34 +530,71 @@ func TestEchoRoutes(t *testing.T) {
}
}

func TestEchoRoutesHandleHostsProperly(t *testing.T) {
func TestEchoRoutesHandleAdditionalHosts(t *testing.T) {
e := New()
h := e.Host("route.com")
domain2Router := e.Host("domain2.router.com")
routes := []*Route{
{http.MethodGet, "/users/:user/events", ""},
{http.MethodGet, "/users/:user/events/public", ""},
{http.MethodPost, "/repos/:owner/:repo/git/refs", ""},
{http.MethodPost, "/repos/:owner/:repo/git/tags", ""},
}
for _, r := range routes {
h.Add(r.Method, r.Path, func(c Context) error {
domain2Router.Add(r.Method, r.Path, func(c Context) error {
return c.String(http.StatusOK, "OK")
})
}
e.Add(http.MethodGet, "/api", func(c Context) error {
return c.String(http.StatusOK, "OK")
})

if assert.Equal(t, len(routes), len(e.Routes())) {
for _, r := range e.Routes() {
found := false
for _, rr := range routes {
if r.Method == rr.Method && r.Path == rr.Path {
found = true
break
}
domain2Routes := e.Routers()["domain2.router.com"].Routes()

assert.Len(t, domain2Routes, len(routes))
for _, r := range domain2Routes {
found := false
for _, rr := range routes {
if r.Method == rr.Method && r.Path == rr.Path {
found = true
break
}
if !found {
t.Errorf("Route %s %s not found", r.Method, r.Path)
}
if !found {
t.Errorf("Route %s %s not found", r.Method, r.Path)
}
}
}

func TestEchoRoutesHandleDefaultHost(t *testing.T) {
e := New()
routes := []*Route{
{http.MethodGet, "/users/:user/events", ""},
{http.MethodGet, "/users/:user/events/public", ""},
{http.MethodPost, "/repos/:owner/:repo/git/refs", ""},
{http.MethodPost, "/repos/:owner/:repo/git/tags", ""},
}
for _, r := range routes {
e.Add(r.Method, r.Path, func(c Context) error {
return c.String(http.StatusOK, "OK")
})
}
e.Host("subdomain.mysite.site").Add(http.MethodGet, "/api", func(c Context) error {
return c.String(http.StatusOK, "OK")
})

defaultRouterRoutes := e.Routes()
assert.Len(t, defaultRouterRoutes, len(routes))
for _, r := range defaultRouterRoutes {
found := false
for _, rr := range routes {
if r.Method == rr.Method && r.Path == rr.Path {
found = true
break
}
}
if !found {
t.Errorf("Route %s %s not found", r.Method, r.Path)
}
}
}

Expand Down Expand Up @@ -1468,14 +1505,27 @@ func TestEchoReverseHandleHostProperly(t *testing.T) {
dummyHandler := func(Context) error { return nil }

e := New()

// routes added to the default router are different form different hosts
e.GET("/static", dummyHandler).Name = "default-host /static"
e.GET("/static/*", dummyHandler).Name = "xxx"

// different host
h := e.Host("the_host")
h.GET("/static", dummyHandler).Name = "/static"
h.GET("/static/*", dummyHandler).Name = "/static/*"
h.GET("/static", dummyHandler).Name = "host2 /static"
h.GET("/static/v2/*", dummyHandler).Name = "xxx"

assert.Equal(t, "/static", e.Reverse("default-host /static"))
// when actual route does not have params and we provide some to Reverse we should get that route url back
assert.Equal(t, "/static", e.Reverse("default-host /static", "missing param"))

host2Router := e.Routers()["the_host"]
assert.Equal(t, "/static", host2Router.Reverse("host2 /static"))
assert.Equal(t, "/static", host2Router.Reverse("host2 /static", "missing param"))

assert.Equal(t, "/static/v2/*", host2Router.Reverse("xxx"))
assert.Equal(t, "/static/v2/foo.txt", host2Router.Reverse("xxx", "foo.txt"))

assert.Equal(t, "/static", e.Reverse("/static"))
assert.Equal(t, "/static", e.Reverse("/static", "missing param"))
assert.Equal(t, "/static/*", e.Reverse("/static/*"))
assert.Equal(t, "/static/foo.txt", e.Reverse("/static/*", "foo.txt"))
}

func TestEcho_ListenerAddr(t *testing.T) {
Expand Down
46 changes: 46 additions & 0 deletions router.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package echo

import (
"bytes"
"fmt"
"net/http"
)

Expand Down Expand Up @@ -141,6 +142,51 @@ func NewRouter(e *Echo) *Router {
}
}

// Routes returns the registered routes.
func (r *Router) Routes() []*Route {
routes := make([]*Route, 0, len(r.routes))
for _, v := range r.routes {
routes = append(routes, v)
}
return routes
}

// Reverse generates an URL from route name and provided parameters.
func (r *Router) Reverse(name string, params ...interface{}) string {
uri := new(bytes.Buffer)
ln := len(params)
n := 0
for _, route := range r.routes {
if route.Name == name {
for i, l := 0, len(route.Path); i < l; i++ {
if (route.Path[i] == ':' || route.Path[i] == '*') && n < ln {
for ; i < l && route.Path[i] != '/'; i++ {
}
uri.WriteString(fmt.Sprintf("%v", params[n]))
n++
}
if i < l {
uri.WriteByte(route.Path[i])
}
}
break
}
}
return uri.String()
}

func (r *Router) add(method, path, name string, h HandlerFunc) *Route {
r.Add(method, path, h)

route := &Route{
Method: method,
Path: path,
Name: name,
}
r.routes[method+path] = route
return route
}

// Add registers a new route for method and path with matching handler.
func (r *Router) Add(method, path string, h HandlerFunc) {
// Validate path
Expand Down

0 comments on commit f1cf1ec

Please sign in to comment.