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

static segment conflicts with wildcard segment #183

Open
jameshfisher opened this issue Feb 7, 2017 · 11 comments · Fixed by gin-gonic/gin#2663
Open

static segment conflicts with wildcard segment #183

jameshfisher opened this issue Feb 7, 2017 · 11 comments · Fixed by gin-gonic/gin#2663
Milestone

Comments

@jameshfisher
Copy link

Steps to reproduce

  • Use version 4563b0b of httprouter
  • Compile the following program
package main

import (
    "github.com/julienschmidt/httprouter"
    "net/http"
)

func main() {
    router := httprouter.New()
    router.GET("/user/:user", nil)
    router.GET("/user/gordon/:profile", nil)
    http.ListenAndServe(":8080", router)
}
  • Run it

Expected behavior

Process starts web server. Requests with a two-segment path, if beginning with /user/, go to the first handler. Requests with a three-segment path, if beginning with /user/gordon/, go to the second handler. All other requests error.

Actual behavior

Process panics:

panic: 'gordon' in new path '/user/gordon/:profile' conflicts with existing wildcard ':user' in existing prefix '/user/:user'

goroutine 1 [running]:
panic(0x20c240, 0xc42000d200)
	/usr/local/Cellar/go/1.7.3/libexec/src/runtime/panic.go:500 +0x1a1
github.com/pusher/httproutertest/vendor/github.com/julienschmidt/httprouter.(*node).addRoute(0xc420012730, 0x2676c4, 0x15, 0x0)
	/Users/jhf/go/src/github.com/pusher/httproutertest/vendor/github.com/julienschmidt/httprouter/tree.go:159 +0x73e
github.com/pusher/httproutertest/vendor/github.com/julienschmidt/httprouter.(*Router).Handle(0xc420010500, 0x263045, 0x3, 0x2676c4, 0x15, 0x0)
	/Users/jhf/go/src/github.com/pusher/httproutertest/vendor/github.com/julienschmidt/httprouter/router.go:236 +0xcc
github.com/pusher/httproutertest/vendor/github.com/julienschmidt/httprouter.(*Router).GET(0xc420010500, 0x2676c4, 0x15, 0x0)
	/Users/jhf/go/src/github.com/pusher/httproutertest/vendor/github.com/julienschmidt/httprouter/router.go:180 +0x5e
main.main()
	/Users/jhf/go/src/github.com/pusher/httproutertest/main.go:11 +0xba
@Thomasdezeeuw
Copy link

This is because :user in the first route is a wild card, which match gordon/:profile of the second route as well. See #175.

@jameshfisher
Copy link
Author

@Thomasdezeeuw @julienschmidt thanks. :user certainly shouldn't match gordon/:profile, because "Named parameters only match a single path segment". So I think this should be considered a bug (or a documentation bug).

From #175, I see this is because httprouter uses a prefix tree. I think there's a simple algorithmic fix: first branch based on the length in number of path segments, before branching on the individual segments. So the prefix tree in my example should look like this:

O --+--2--> O --"user"--> O --any--> O
    |
    +--3--> O --"user"--> O --"gordon"--> O --any--> O

@liminalitythree
Copy link

liminalitythree commented May 5, 2018

Has any progress been made on this issue? This is a very common pattern in websites, even the page you're probably on right now.

github.com/julienschmidt/
and
github.com/julienschmidt/httprouter/

would something simple like this not be possible with httprouter?

if this is about the "Only explicit matches" thing, I think this is still explicit.

one route is /* and the other is /*/*. I don't think the wildcard from the first route should match the second route, because there's another path/subdirectory there.

Repository owner deleted a comment from zigo101 Jul 15, 2018
@julienschmidt julienschmidt modified the milestones: v1.2, v1.3 Oct 13, 2018
@yaron2
Copy link

yaron2 commented Mar 21, 2019

Hi, interested in knowing whether this is going to be addressed soon?

@julienschmidt julienschmidt modified the milestones: v1.3, v2 Sep 25, 2019
@julienschmidt julienschmidt changed the title Spurious panic: three-segment route "conflicts with existing" two-segment route static segment conflicts with wildcard segment Sep 25, 2019
@julienschmidt
Copy link
Owner

julienschmidt commented Sep 25, 2019

For now the following should work. If necessary, one would have to check that :user is gordon. Relaxed routing, where static segments have preference over dynamic segements (wildcards), is planned for v2.

package main

import (
    "net/http"

    "github.com/julienschmidt/httprouter"
)

func main() {
    router := httprouter.New()
    router.GET("/user/:user", handleUser)
    router.GET("/user/:user/:profile", handleUserProfile)
    http.ListenAndServe(":8080", router)
}

See also #73

@arianitu
Copy link

arianitu commented Mar 20, 2020

PLEASE fix this so users of frameworks like Gin do not have to rip out the entire framework. That's what we are about to do. We are about to create our own package called http that has a sane router and some basic other things we need like middleware.go and binding.go and be done with this nonsense.

Honestly, is it just me or is it confusing how Gin has 32k+ stars and this has 10k+ stars when you can't even make usable routes? I mean sure, you can claim it's fast, but it comes at a cost that most people don't know about until they're in deep since there was no notice and no indication that this is a limitation. I know I am complaining on this library instead of gin, but the gin guys haven't fixed it in 4 years and if this library could have some flag like httprouter.SaneRoutes(true) it would be awesome.

By the way, there should be a massive notice at the top of both Gin and this library that talks about this. It should be:

NOTICE: You cannot make a typical REST API because you WILL run into wildcard conflicts. Use at your OWN risk

AGAIN the issue goes back 4 YEARS and why is no one doing anything about it?

@SamWhited
Copy link

SamWhited commented Mar 20, 2020

I would like to disagree with the assertion that creating ambiguous routes this way is "sane". If you want ambiguous routes or need them to meet legacy requirements, there are plenty of other routers you can use. However, for new products where you get to design the routes, I encourage you to just work within the constraints. I can't tell you the number of times I've seen a new web service broken because someone didn't vet their routes properly and someone created a user called "new" that broke the page "/new" or something along those lines. Or, in this case, someone wants to add a /user/:user/:profile route later and can't because no one had thought that far ahead.

While this may be an acceptable risk for you, or you may have taken great care to avoid these sorts of issues and trust that it won't happen in the future, I find it much safer and easier to just use a router that doesn't allow this sort of ambiguity. It influences how I design my routes and means I end up with a better product in the future with fewer chances for ambiguity. In fact, for me personally, one of my requirements when selecting a new router is that it doesn't let me mix variable route parameters and static routes.

@ImVexed
Copy link

ImVexed commented Jul 22, 2020

I would like to disagree with the assertion that mixing static segments and wildcard segments is somehow legacy or "risky". And that I should "work within the constraints" that are arbitrary to this package and not standard across the industry.

Examples of static and wildcard segment mixing:
https://discord.com/developers/docs/resources/user#get-current-user

https://dev.twitch.tv/docs/v5/reference/videos#get-video + https://dev.twitch.tv/docs/v5/reference/videos#get-top-videos

https://www.reddit.com/dev/api/#GET_api_live_happening_now

There are plenty more examples, but I feel that this sort of functionality is not at all ambiguous and the onus is not on the package to withhold features because it's possible to misuse them. Even if you don't trust yourself to be disciplined enough not to misuse the tools given to you, doesn't mean they aren't useful for others.

@ta1bbty
Copy link

ta1bbty commented Aug 30, 2021

#337
Are these the same 🤔

@Siocki
Copy link

Siocki commented Jan 31, 2022

#337
Are these the same 🤔

These are similar but not the same

OmarMohamed95 added a commit to OmarMohamed95/flashcard that referenced this issue Apr 2, 2023
The package julienschmidt/httprouter has major common issue
julienschmidt/httprouter#183
@steevehook
Copy link

Any updates on v2? It seems it's been 7 years since this was opened. My favorite router, yet the lack of this feature seems pretty annoying

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet