[bug] Giving methodNotAllowed even when route is defined

[rkilingr]

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 #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

[elithrar]

Are you willing to submit a PR for this, including tests?

We clearly need more test coverage here, since we continue to trade route matching bugs.

[rkilingr]

I have submitted a PR including tests for the particular case, you can review it and say whether it's fine.

[stale]

This issue has been automatically marked as stale because it hasn't seen a recent update. It'll be automatically closed in a few days.