From 99de2a55d428c26f06dc7ef24dff8826b1e47860 Mon Sep 17 00:00:00 2001 From: Soheil Rahmat Date: Fri, 14 Jul 2023 19:57:40 +0200 Subject: [PATCH] [BUG] Inconsistent HTTP status code on query mismatch The logical behavour of a router should return http status code of 404 when a request fails to stisfy a route validation logics. Before this, mux was returning 405 http status code in some rare scenarios which is not a valid on its case. For more info, See: https://github.com/gorilla/mux/issues/704 --- mux_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ route.go | 10 ++++++++++ 2 files changed, 57 insertions(+) diff --git a/mux_test.go b/mux_test.go index 2d8d2b3e..9b0f78c2 100644 --- a/mux_test.go +++ b/mux_test.go @@ -2068,6 +2068,53 @@ func TestNoMatchMethodErrorHandler(t *testing.T) { } } +func TestMultipleDefinitionOfSamePathWithDifferentMethods(t *testing.T) { + emptyHandler := func(w http.ResponseWriter, r *http.Request) {} + + r := NewRouter() + r.HandleFunc("/api", emptyHandler).Methods("POST") + r.HandleFunc("/api", emptyHandler).Queries("time", "{time:[0-9]+}").Methods("GET") + + t.Run("Post Method should be matched properly", func(t *testing.T) { + req, _ := http.NewRequest("POST", "http://localhost/api", nil) + 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) + } + }) + + t.Run("Get Method with invalid query value should not match", func(t *testing.T) { + req, _ := http.NewRequest("GET", "http://localhost/api?time=-4", nil) + match := new(RouteMatch) + matched := r.Match(req, match) + if matched { + t.Error("Should not have matched route for methods") + } + if match.MatchErr != ErrNotFound { + t.Error("Should have ErrNotFound error. Found:", match.MatchErr) + } + }) + + t.Run("A mismach method of a valid path should return ErrMethodMismatch", func(t *testing.T) { + r := NewRouter() + r.HandleFunc("/api2", emptyHandler).Methods("POST") + req, _ := http.NewRequest("GET", "http://localhost/api2", 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 have ErrMethodMismatch error. Found:", match.MatchErr) + } + }) + +} + func TestErrMatchNotFound(t *testing.T) { emptyHandler := func(w http.ResponseWriter, r *http.Request) {} diff --git a/route.go b/route.go index 750afe57..cfdede03 100644 --- a/route.go +++ b/route.go @@ -66,6 +66,16 @@ func (r *Route) Match(req *http.Request, match *RouteMatch) bool { matchErr = nil 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 + } } }