Skip to content

Commit

Permalink
fix(gorilla#739): Remove else statement on route mismatch
Browse files Browse the repository at this point in the history
When a path with specific query params does not match we should return a ErrMethodMismatch instead of ErrNotFound
  • Loading branch information
marcelom97 committed Dec 19, 2023
1 parent e44017d commit 737cec6
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 12 deletions.
28 changes: 26 additions & 2 deletions mux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2095,8 +2095,8 @@ func TestMultipleDefinitionOfSamePathWithDifferentMethods(t *testing.T) {
if matched {
t.Error("Should not have matched route for methods")
}
if match.MatchErr != ErrNotFound {
t.Error("Should have ErrNotFound error. Found:", match.MatchErr)
if match.MatchErr != ErrMethodMismatch {
t.Error("Should have ErrMethodMismatch error. Found:", match.MatchErr)
}
})

Expand All @@ -2114,6 +2114,30 @@ func TestMultipleDefinitionOfSamePathWithDifferentMethods(t *testing.T) {
}
})

t.Run("A mismach method of a valid path on subrouter should return ErrMethodMismatch", func(t *testing.T) {
r := NewRouter()
r.MethodNotAllowedHandler = http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
rw.WriteHeader(http.StatusMethodNotAllowed)
_, _ = rw.Write([]byte("Method not allowed"))
})

subrouter := r.PathPrefix("/v1").Subrouter()
subrouter.HandleFunc("/api", func(w http.ResponseWriter, e *http.Request) {
_, _ = w.Write([]byte("Logic"))
}).Methods("GET")
subrouter.HandleFunc("/api/{id}", func(w http.ResponseWriter, e *http.Request) {
_, _ = w.Write([]byte("Logic"))
}).Methods("GET")

rw := NewRecorder()
req, _ := http.NewRequest("POST", "/v1/api", nil)
r.ServeHTTP(rw, req)

if rw.Code != http.StatusMethodNotAllowed {
t.Fatalf("Should have status code 405 (StatusMethodNotAllowed). Got %v\n", rw.Code)
}
})

}

func TestErrMatchNotFound(t *testing.T) {
Expand Down
10 changes: 0 additions & 10 deletions route.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,6 @@ func (r *Route) Match(req *http.Request, match *RouteMatch) bool {

matchErr = nil // nolint:ineffassign
return false
} else {
// Multiple routes may share the same path but use different HTTP methods. For instance:
// Route 1: POST "/users/{id}".
// Route 2: GET "/users/{id}", parameters: "id": "[0-9]+".
//
// The router must handle these cases correctly. For a GET request to "/users/abc" with "id" as "-2",
// The router should return a "Not Found" error as no route fully matches this request.
if match.MatchErr == ErrMethodMismatch {
match.MatchErr = nil
}
}
}

Expand Down

0 comments on commit 737cec6

Please sign in to comment.