Skip to content

Route conflict #875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3 tasks done
mmindenhall opened this issue Mar 10, 2017 · 4 comments
Closed
3 tasks done

Route conflict #875

mmindenhall opened this issue Mar 10, 2017 · 4 comments
Labels

Comments

@mmindenhall
Copy link

Description

If I have two routes defined, one with a static value and one with a path param, the static value cannot be used as the path param even if the two routes do not have the same number of path segments.

For example, if I define these two routes:

GET /users/:id/comments   // 3 path segments
GET /:resourceName/:id    // 2 path segments

The second route fails when :resourceId is users (but works for any other value). I'm confused by this behavior. I know the matching order is static -> param -> any, but I would have thought that if no match was found down the static /users path, the router would then attempt to match down the /:resourceId path.

Checklist

  • Dependencies installed
  • No typos
  • Searched existing issues and docs

Expected behaviour

All of these requests should work:

GET /users/1/comments
GET /boozers/1
GET /users/1

Actual behaviour

These two work:

GET /users/1/comments
GET /boozers/1

But this fails:

GET /users/1

Steps to reproduce

Working code to debug

package main

import (
	"testing"

	"github.com/labstack/echo"
	"github.com/stretchr/testify/assert"
)

func TestRouteConflict(t *testing.T) {
	e := echo.New()
	r := e.Router()

	e.GET("/users/:id/comments", func(c echo.Context) error {
		c.Set("r", 1)
		return nil
	})

	e.GET("/:resourceName/:id", func(c echo.Context) error {
		c.Set("r", 2)
		return nil
	})

	c := e.NewContext(nil, nil)
	r.Find(echo.GET, "/users/1/comments", c)
	c.Handler()(c)
	assert.Equal(t, 1, c.Get("r"))

	c = e.NewContext(nil, nil)
	r.Find(echo.GET, "/boozers/1", c)
	c.Handler()(c)
	assert.Equal(t, 2, c.Get("r"))

	c = e.NewContext(nil, nil)
	r.Find(echo.GET, "/users/1", c)
	c.Handler()(c)
	assert.Equal(t, 2, c.Get("r")) // this assertion fails
}

Version/commit

3.1.0-rc1

@adred
Copy link

adred commented Mar 10, 2017

httprouter has this problem which is a radix tree based router also.

@mmindenhall
Copy link
Author

Echo already does better than httprouter in this regard. From httprouter README:

For example you can not register the patterns /user/new and /user/:user for the same request method at the same time.

Echo uses the same patterns as a working example on the Routing page of the guide:

e.GET("/users/:id", func(c echo.Context) error {
	return c.String(http.StatusOK, "/users/:id")
})

e.GET("/users/new", func(c echo.Context) error {
	return c.String(http.StatusOK, "/users/new")
})

e.GET("/users/1/files/*", func(c echo.Context) error {
	return c.String(http.StatusOK, "/users/1/files/*")
})

@abaehre
Copy link

abaehre commented Apr 4, 2018

The reason for this is the way the radix tree is created.
Running your test file the radix tree looks like such -

└── /, pnames=[] ppath= handler=&{<nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil>}
 ├── users/, pnames=[] ppath= handler=&{<nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil>}
 │ └── :, pnames=[id] ppath=/users/:id/comments handler=&{<nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil>}
 │  └── /comments, pnames=[id] ppath=/users/:id/comments handler=&{<nil> <nil> 0x135ff50 <nil> <nil> <nil> <nil> <nil> <nil> <nil>}
 └── :, pnames=[resourceName] ppath=/:resourceName/:id handler=&{<nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil>}
  └── /, pnames=[] ppath= handler=&{<nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil> <nil>}
   └── :, pnames=[resourceName id] ppath=/:resourceName/:id handler=&{<nil> <nil> 0x135ff50 <nil> <nil> <nil> <nil> <nil> <nil> <nil>}

your 3rd test hits on /users/: which has no handlers associated with it since you didn't actually create a handler for that path. It just happened to be an intermediate step in the creation of the tree.

I'm not sure if this is a limitation of the radix tree, or something specific to echo, but I figured I'd at least comment for those that see this in the future.

@stale
Copy link

stale bot commented Nov 30, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 30, 2018
@stale stale bot closed this as completed Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants