Skip to content

Commit

Permalink
Ignore ErrNotFound while matching Subrouters (gorilla#438)
Browse files Browse the repository at this point in the history
MatchErr is set by the router to ErrNotFound if no route matches. If
no route of a Subrouter matches the error can by safely ignored. This
implementation only ignores these errors and does not ignore other
errors like ErrMethodMismatch.
  • Loading branch information
g-w authored and elithrar committed Jan 8, 2019
1 parent f3ff42f commit 08e7f80
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 5 deletions.
32 changes: 32 additions & 0 deletions mux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2715,6 +2715,38 @@ func Test_copyRouteConf(t *testing.T) {
}
}

func TestMethodNotAllowed(t *testing.T) {
handler := func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }
router := NewRouter()
router.HandleFunc("/thing", handler).Methods(http.MethodGet)
router.HandleFunc("/something", handler).Methods(http.MethodGet)

w := NewRecorder()
req := newRequest(http.MethodPut, "/thing")

router.ServeHTTP(w, req)

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

func TestSubrouterNotFound(t *testing.T) {
handler := func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }
router := NewRouter()
router.Path("/a").Subrouter().HandleFunc("/thing", handler).Methods(http.MethodGet)
router.Path("/b").Subrouter().HandleFunc("/something", handler).Methods(http.MethodGet)

w := NewRecorder()
req := newRequest(http.MethodPut, "/not-present")

router.ServeHTTP(w, req)

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

// mapToPairs converts a string map to a slice of string pairs
func mapToPairs(m map[string]string) []string {
var i int
Expand Down
17 changes: 12 additions & 5 deletions route.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ func (r *Route) Match(req *http.Request, match *RouteMatch) bool {
return false
}

// Set MatchErr to nil to prevent
// subsequent matching subrouters from failing to run middleware.
// If not reset, the middleware would see a non-nil MatchErr and be skipped,
// even when there was a matching route.
match.MatchErr = nil
var matchErr error

// Match everything.
Expand All @@ -57,6 +52,18 @@ func (r *Route) Match(req *http.Request, match *RouteMatch) bool {
matchErr = ErrMethodMismatch
continue
}

// Ignore ErrNotFound errors. These errors arise from match call
// to Subrouters.
//
// This prevents subsequent matching subrouters from failing to
// run middleware. If not ignored, the middleware would see a
// non-nil MatchErr and be skipped, even when there was a
// matching route.
if match.MatchErr == ErrNotFound {
match.MatchErr = nil
}

matchErr = nil
return false
}
Expand Down

0 comments on commit 08e7f80

Please sign in to comment.