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

Feature: Include router option that prevents over writing routes #792

Open
djensen47 opened this issue Feb 3, 2023 · 5 comments
Open

Feature: Include router option that prevents over writing routes #792

djensen47 opened this issue Feb 3, 2023 · 5 comments

Comments

@djensen47
Copy link

djensen47 commented Feb 3, 2023

I discovered a scenario where you can create two handlers that handle the same pattern but the second one "overwrites" the first one.

The expected behavior would be to panic. Because the pattern GET /things being attempted to be added twice

baseRouter := chi.NewRouter()

baseRouter.Get("/things", func(w http.ResponseWriter, r *http.Request) {
	w.Write([]byte("things"))
})

things := chi.NewRouter()
things.Get("/", func(w http.ResponseWriter, r *http.Request) {
	w.Write([]byte("other things"))
})

baseRouter.Mount("/things", things) //should panic

The example below I would not expect to panic because there are no overlapping URL patterns. The are GET /things and GET /things/other

baseRouter := chi.NewRouter()

baseRouter.Get("/things", func(w http.ResponseWriter, r *http.Request) {
	w.Write([]byte("things"))
})

things := chi.NewRouter()
things.Get("/other", func(w http.ResponseWriter, r *http.Request) {
	w.Write([]byte("other things"))
})

baseRouter.Mount("/things", things) //should not panic

For a GET /things the response will be other things

@pkieltyka
Copy link
Member

hi there, so chi does allow overwriting routes. I understand it can be a bit confusing when this happens unintentionally, but, it's very useful when you need it. Perhaps we add some kind of "Options" to a router, and we could specify if we want to allow overwriting routes or not, and if not, then would panic.

@djensen47
Copy link
Author

djensen47 commented Feb 5, 2023 via email

@djensen47 djensen47 changed the title Existing pattern does not panic when mounted Feature: Include router option that prevents over writing routes Feb 6, 2023
@bubbajoe
Copy link

bubbajoe commented Feb 13, 2023

@pkieltyka I was looking for something similar!

+1 for this, but in addition to adding an option for panicking, how about a separate function similar to Match(rctx *Context, method, path string) bool, like MatchPattern(rctx *Context, method, pattern string) (*Route, bool)?

I use chi as a reverse proxy that dynamically adds routes, and i would like to check if that particular route/pattern is already taken (and which one if possible).

@bubbajoe
Copy link

@pkieltyka Is there still a plan to add this?

@pkieltyka
Copy link
Member

hi, this is not planned

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

No branches or pull requests

3 participants