go-chi / chi

lightweight, idiomatic and composable router for building Go HTTP services
https://go-chi.io
MIT License
18.62k stars 989 forks source link

`(mx *Mux) Match(...)` has surprising behaviour when using `(mx *Mux) Route(...)` #794

Open msbit opened 1 year ago

msbit commented 1 year ago

If I add a subrouter using Route, calling Match with the same pattern and any method shows a (what I would consider) false positive. For example:

r := chi.NewRouter()

r.Route("/path", func(r chi.Router) {
  r.Group(func(r chi.Router) {
    r.Get("/all", nil)
  })  
}) 

I would expect:

however, the first case is true.

For what it's worth, the contents of the inner subrouter don't appear to matter, so:

r := chi.NewRouter()

r.Route("/path", func(chi.Router){})

shows the same behaviour in the "/path" case.

As noted this is surprising to me; I would expect no match to occur unless a final handler has been configured (Get(...), Post(...) etc).

Below is a more elaborate test setup:

package routing

import (
  "net/http"
  "testing"

  "github.com/go-chi/chi/v5"
)

func TestWithAll(t *testing.T) {
  r := chi.NewRouter()

  r.Route("/path", func(r chi.Router) {
    r.Group(func(r chi.Router) {
      r.Get("/all", nil)
    })
  })

  assert(t, !r.Match(chi.NewRouteContext(), http.MethodGet, "/path"),     "GET    /path should not be matched")
  assert(t, !r.Match(chi.NewRouteContext(), http.MethodPost, "/path"),    "POST   /path should not be matched")
  assert(t, !r.Match(chi.NewRouteContext(), http.MethodPut, "/path"),     "PUT    /path should not be matched")
  assert(t, !r.Match(chi.NewRouteContext(), http.MethodDelete, "/path"),  "DELETE /path should not be matched")
  assert(t, r.Match(chi.NewRouteContext(), http.MethodGet, "/path/all"),  "GET    /path/all should be matched")
}

func TestWithoutAll(t *testing.T) {
  r := chi.NewRouter()

  r.Route("/path", func(chi.Router){})

  assert(t, !r.Match(chi.NewRouteContext(), http.MethodGet, "/path"),     "GET    /path should not be matched")
  assert(t, !r.Match(chi.NewRouteContext(), http.MethodPost, "/path"),    "POST   /path should not be matched")
  assert(t, !r.Match(chi.NewRouteContext(), http.MethodPut, "/path"),     "PUT    /path should not be matched")
  assert(t, !r.Match(chi.NewRouteContext(), http.MethodDelete, "/path"),  "DELETE /path should not be matched")
}

func assert(t *testing.T, condition bool, message string) {
  if !condition {
    t.Errorf(message)
  }
}
pauloo27 commented 4 months ago

I am also suffering with this. Any updates?