Skip to content

Fix SupressNotFound making subrouters return 404 and add tests for SupressNotFound #940

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ezraisw
Copy link

@ezraisw ezraisw commented Jul 26, 2024

This fixes #939 by using NewRouteContext() to create a new route context before calling the Match function.

Because of Match's behavior of modifying route context, it is necessary to for the route context to be isolated.

chi/mux.go

Lines 361 to 362 in 23c395f

// Note: the *Context state is updated during execution, so manage
// the state carefully or make a NewRouteContext().

Also added tests to enforce this logic.

@ezraisw ezraisw changed the title Fix SupressNotFound making subrouters 404 Fix SupressNotFound making subrouters return 404 and add tests for SupressNotFound Jul 26, 2024
@johnwook
Copy link

johnwook commented Aug 5, 2024

wow, I was wondering why. Looks great!

@devhaozi
Copy link

I'm having the same problem, @pkieltyka could you take a look at this?

@devhaozi
Copy link

devhaozi commented Dec 14, 2024

@VojtechVitek could you take a look at this? It has been around for a long time.

@devhaozi
Copy link

devhaozi commented Jul 7, 2025

@VojtechVitek why hasn't this PR been merged yet?

@jay-babu
Copy link

This is a pretty glaring bug. I unfortunately spent a couple of hours before realizing this issue

@ezraisw
Copy link
Author

ezraisw commented Jul 27, 2025

FYI, this middleware returns 404 instead of 405 for requests with valid paths that have incorrect HTTP methods. Supporting 405 response would require a bigger change within the chi.Mux implementation.

@ezraisw ezraisw requested a review from VojtechVitek July 27, 2025 13:32
@ezraisw ezraisw requested a review from VojtechVitek July 28, 2025 13:02
Copy link
Contributor

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

Can someone please help me test this PR and verify its behavior? 👀

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

Successfully merging this pull request may close these issues.

Sub-routers returning 404 when using middleware.SupressNotFound on parent router
5 participants