Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
msbit opened this issue Feb 15, 2023 · 0 comments
Open

Comments

@msbit
Copy link

msbit commented Feb 15, 2023

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:

  • r.Match(chi.NewRouteContext(), http.MethodGet, "/path") to be false, and
  • r.Match(chi.NewRouteContext(), http.MethodGet, "/path/all") to be true

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)
  }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant