Skip to content
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

Non-wildcard routes that share a prefix with a wildcard route should take priority over the wildcard route #73

Open
prencher opened this issue Apr 2, 2015 · 9 comments · Fixed by gin-gonic/gin#2663
Labels
Milestone

Comments

@prencher
Copy link

prencher commented Apr 2, 2015

Currently it is impossible to do the following with httprouter:

router := httprouter.New()
router.Handle("GET", "/*path", proxy)
router.Handle("GET", "/api", api)

This makes it impossible to do "enhanced" proxies and CMS'es that have completely dynamic URL's other than a control panel, just to name two practical examples.

To address this, I propose that URL's that share prefixes with wildcards would take priority over the wildcard. Effectively, they would have a higher weight. This is how I solved a similar problem in the Werkzeug WSGI library for Python.

In the above example, /api would take priority because it has a non-wildcard component in the same place as another route with a wildcard.

Combined with #72, this would also dramatically improve the ability to compose packages with httprouter.

@julienschmidt
Copy link
Owner

No, let me cite you from the README:

Only explicit matches: With other routers, like http.ServeMux, a requested URL path could match multiple patterns. Therefore they have some awkward pattern priority rules, like longest match or first registered, first matched. By design of this router, a request can only match exactly one or no route. As a result, there are also no unintended matches, which makes it great for SEO and improves the user experience.

The special treatment of /api would have to be done within the registered handler (can be a middleware). It is not impossible to implement the described routing behavior, but it has to be done manually, e.g. by the framework.

@prencher
Copy link
Author

prencher commented Apr 9, 2015

I'm sorry but "makes it great for SEO and improves the user experience"?

First off, It has absolutely nothing to do with SEO, I don't know where you even got that idea.

Second, a user now has to apply middleware or use the net/http router to allow wildcards at the same level as normal routes, which entirely defeats the purpose of having a custom router in the first place.

Defining wildcards as being fallbacks when there isn't an exact match gives a much more flexible API, much like the change I made in #72 for HTTP methods. It does not clutter or confuse the API.

@rogpeppe
Copy link
Contributor

I just came across this problem and found this issue, so am replying here.

The special treatment of /api would have to be done within the registered handler (can be a middleware).

Can you suggest a way that it might be possible to continue to use httprouter for
the static part of the route?

For example, if we wanted to serve something like this, with the
more specific paths taking precedence over the wildcard
path:

router := httprouter.New()
router.Handle("GET", "/a/b/c/*path", proxy)
router.Handle("GET", "/a/b/c/api", api)
router.Handle("GET", "/a/b/c/api/path1", apiPath1)
router.Handle("GET", "/a/b/c/api/path2", apiPath2)

Your suggestion, I think, would be to make a custom handler
for /a/b/c and manually route all of those other paths. Do you see
a way to use httprouter to route them instead of doing it manually
or using some other router package?

@julienschmidt julienschmidt reopened this May 27, 2015
@julienschmidt
Copy link
Owner

I'm currently working on httprouter v2, for which I plan an (optional) less strict routing mode.
Currently the given routing structure is simple not possible with httprouter.

@julienschmidt julienschmidt added this to the v2 milestone May 27, 2015
@mholt
Copy link

mholt commented Jun 9, 2015

👍 Looking forward to this feature.

@mholt
Copy link

mholt commented Jun 9, 2015

Looking at this and my code again, my needs might be a bit different. I need these two routes to play nicely with each other:

  • /:addr
  • /cmd/reload

I was surprised to see this panic:

panic: wildcard route ':addr' conflicts with existing children in path '/:addr'

because, from the docs:

Named parameters only match a single path segment

I would have expected /:addr to match any request with a single path segment, but /cmd/reload has two, so I'm not sure why it conflicts.

Still kind of new to this router... maybe this is a separate issue?

@julienschmidt
Copy link
Owner

/:addr and /cmd are conflicting here.
But this will (probably) also work in v2.

@0x7061

This comment has been minimized.

@olanod

This comment has been minimized.

Repository owner locked and limited conversation to collaborators Aug 20, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
6 participants