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

[bug] Giving methodNotAllowed even when route is defined #6

Open
tebruno99 opened this issue Dec 15, 2022 · 0 comments
Open

[bug] Giving methodNotAllowed even when route is defined #6

tebruno99 opened this issue Dec 15, 2022 · 0 comments
Labels
bug Something isn't working stale

Comments

@tebruno99
Copy link

tebruno99 commented Dec 15, 2022

MIGRATED
From mux created by rkilingr: gorilla#515

Describe the bug

When there is a route pattern with a different Method after the same pattern ( "OPTIONS /a/b
after GET /a/b") for which there is a middle-ware response, it gives 405. I guess it's related to gorilla#509

Versions

Go version: go1.11.5 linux/amd64
package version: 9536e4053d763b54d935f1ce731a315cfb42b979

Steps to Reproduce

Define a route /a/b with method GET
Define a route /a/{a} with method OPTIONS
Define middleware which gives 200 for defined options routes
It gives StatusMethodMismatch(405) instead of 200
If the steps 1 and 2 are reversed it gives 200

Expected behavior

The expected behaviour is to give 200

Code Snippets

mux_test.go



// testMiddleWare returns 200 on an OPTIONS request
func testOptionsMiddleWare(inner http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if r.Method == http.MethodOptions {
			w.WriteHeader(http.StatusOK)
			return
		}
		inner.ServeHTTP(w, r)
	})
}

// Passes if /a/{a} is defined first
func TestRouterOrder(t *testing.T) {
	handler := func(w http.ResponseWriter, r *http.Request) {}
	router := NewRouter()
	router.Path("/a/b").Handler(http.HandlerFunc(handler)).Methods(http.MethodGet)
	router.Path("/a/{a}").Handler(nil).Methods(http.MethodOptions)
	router.Use(MiddlewareFunc(testOptionsMiddleWare))

	w := NewRecorder()
	req := newRequest(http.MethodOptions, "/a/b")

	router.ServeHTTP(w, req)

	if w.Code != http.StatusOK {
		t.Fatalf("Expected status code 200 (got %d)", w.Code)
	}
}

Possible Solution

route.go Line https://github.com/gorilla/mux/blob/master/route.go#L77

	diff --git a/route.go b/route.go
index 7343d78..1972c77 100644
--- a/route.go
+++ b/route.go
@@ -74,7 +74,7 @@ func (r *Route) Match(req *http.Request, match *RouteMatch) bool {
                return false
        }
 
-       if match.MatchErr == ErrMethodMismatch && r.handler != nil {
+       if match.MatchErr == ErrMethodMismatch && match.Handler == nil {
                // We found a route which matches request method, clear MatchErr
                match.MatchErr = nil
                // Then override the mis-matched handler
@tebruno99 tebruno99 added bug Something isn't working stale labels Dec 15, 2022
tebruno99 added a commit that referenced this issue Dec 15, 2022
…on-order

MIGRATED: Bugfix/method mismatch depending on order (Fixes #6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

1 participant