MatchErr is set to ErrNotFound if NotFoundHandler is used (gorilla#311)

maplebed · Nov 5, 2017 · 7f08801 · 7f08801
1 parent 9f48112
commit 7f08801
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 43 deletions.
diff --git a/mux.go b/mux.go
@@ -14,6  14,7 @@ import (
 
 var (
 	ErrMethodMismatch = errors.New("method is not allowed")
 	ErrNotFound       = errors.New("no matching route was found")
 )
 
 // NewRouter returns a new router instance.
@@ -82,16  83,23 @@ func (r *Router) Match(req *http.Request, match *RouteMatch) bool {
 		}
 	}
 
-	if match.MatchErr == ErrMethodMismatch && r.MethodNotAllowedHandler != nil {
-		match.Handler = r.MethodNotAllowedHandler
-		return true
 	if match.MatchErr == ErrMethodMismatch {
 		if r.MethodNotAllowedHandler != nil {
 			match.Handler = r.MethodNotAllowedHandler
 			return true
 		} else {
 			return false
 		}
 	}
 
 	// Closest match for a router (includes sub-routers)
 	if r.NotFoundHandler != nil {
 		match.Handler = r.NotFoundHandler
 		match.MatchErr = ErrNotFound
 		return true
 	}
 
 	match.MatchErr = ErrNotFound
 	return false
 }
 

diff --git a/mux_test.go b/mux_test.go
@@ -1877,6  1877,96 @@ func TestSubrouterHeader(t *testing.T) {
 	}
 }
 
 func TestNoMatchMethodErrorHandler(t *testing.T) {
 	func1 := func(w http.ResponseWriter, r *http.Request) {}
 
 	r := NewRouter()
 	r.HandleFunc("/", func1).Methods("GET", "POST")
 
 	req, _ := http.NewRequest("PUT", "http://localhost/", nil)
 	match := new(RouteMatch)
 	matched := r.Match(req, match)
 
 	if matched {
 		t.Error("Should not have matched route for methods")
 	}
 
 	if match.MatchErr != ErrMethodMismatch {
 		t.Error("Should get ErrMethodMismatch error")
 	}
 
 	resp := NewRecorder()
 	r.ServeHTTP(resp, req)
 	if resp.Code != 405 {
 		t.Errorf("Expecting code %v", 405)
 	}
 
 	// Add matching route
 	r.HandleFunc("/", func1).Methods("PUT")
 
 	match = new(RouteMatch)
 	matched = r.Match(req, match)
 
 	if !matched {
 		t.Error("Should have matched route for methods")
 	}
 
 	if match.MatchErr != nil {
 		t.Error("Should not have any matching error. Found:", match.MatchErr)
 	}
 }
 
 func TestErrMatchNotFound(t *testing.T) {
 	emptyHandler := func(w http.ResponseWriter, r *http.Request) {}
 
 	r := NewRouter()
 	r.HandleFunc("/", emptyHandler)
 	s := r.PathPrefix("/sub/").Subrouter()
 	s.HandleFunc("/", emptyHandler)
 
 	// Regular 404 not found
 	req, _ := http.NewRequest("GET", "/sub/whatever", nil)
 	match := new(RouteMatch)
 	matched := r.Match(req, match)
 
 	if matched {
 		t.Errorf("Subrouter should not have matched that, got %v", match.Route)
 	}
 	// Even without a custom handler, MatchErr is set to ErrNotFound
 	if match.MatchErr != ErrNotFound {
 		t.Errorf("Expected ErrNotFound MatchErr, but was %v", match.MatchErr)
 	}
 
 	// Now lets add a 404 handler to subrouter
 	s.NotFoundHandler = http.NotFoundHandler()
 	req, _ = http.NewRequest("GET", "/sub/whatever", nil)
 
 	// Test the subrouter first
 	match = new(RouteMatch)
 	matched = s.Match(req, match)
 	// Now we should get a match
 	if !matched {
 		t.Errorf("Subrouter should have matched %s", req.RequestURI)
 	}
 	// But MatchErr should be set to ErrNotFound anyway
 	if match.MatchErr != ErrNotFound {
 		t.Errorf("Expected ErrNotFound MatchErr, but was %v", match.MatchErr)
 	}
 
 	// Now test the parent (MatchErr should propagate)
 	match = new(RouteMatch)
 	matched = r.Match(req, match)
 
 	// Now we should get a match
 	if !matched {
 		t.Errorf("Router should have matched %s via subrouter", req.RequestURI)
 	}
 	// But MatchErr should be set to ErrNotFound anyway
 	if match.MatchErr != ErrNotFound {
 		t.Errorf("Expected ErrNotFound MatchErr, but was %v", match.MatchErr)
 	}
 }
 
 // mapToPairs converts a string map to a slice of string pairs
 func mapToPairs(m map[string]string) []string {
 	var i int
@@ -1943,42  2033,3 @@ func newRequest(method, url string) *http.Request {
 	}
 	return req
 }
-
-func TestNoMatchMethodErrorHandler(t *testing.T) {
-	func1 := func(w http.ResponseWriter, r *http.Request) {}
-
-	r := NewRouter()
-	r.HandleFunc("/", func1).Methods("GET", "POST")
-
-	req, _ := http.NewRequest("PUT", "http://localhost/", nil)
-	match := new(RouteMatch)
-	matched := r.Match(req, match)
-
-	if matched {
-		t.Error("Should not have matched route for methods")
-	}
-
-	if match.MatchErr != ErrMethodMismatch {
-		t.Error("Should get ErrMethodMismatch error")
-	}
-
-	resp := NewRecorder()
-	r.ServeHTTP(resp, req)
-	if resp.Code != 405 {
-		t.Errorf("Expecting code %v", 405)
-	}
-
-	// Add matching route
-	r.HandleFunc("/", func1).Methods("PUT")
-
-	match = new(RouteMatch)
-	matched = r.Match(req, match)
-
-	if !matched {
-		t.Error("Should have matched route for methods")
-	}
-
-	if match.MatchErr != nil {
-		t.Error("Should not have any matching error. Found:", match.MatchErr)
-	}
-}
diff --git a/route.go b/route.go
@@ -72,7  72,11 @@ func (r *Route) Match(req *http.Request, match *RouteMatch) bool {
 		return false
 	}
 
-	match.MatchErr = nil
 	if match.MatchErr == ErrMethodMismatch {
 		// We found a route which matches request method, clear MatchErr
 		match.MatchErr = nil
 	}
 
 	// Yay, we have a match. Let's collect some info about it.
 	if match.Route == nil {
 		match.Route = r