julienschmidt / httprouter

A high performance HTTP request router that scales well
https://pkg.go.dev/github.com/julienschmidt/httprouter
BSD 3-Clause "New" or "Revised" License
16.56k stars 1.47k forks source link

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

Open prencher opened 9 years ago

prencher commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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.

mholt commented 9 years ago

:+1: Looking forward to this feature.

mholt commented 9 years ago

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:

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 commented 9 years ago

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

0x7061 commented 9 years ago

+1

olanod commented 9 years ago

:+1: a must.