go-chi / go-chi/chi

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

Open
#794 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
22.8k
Forks
1.2k
Avg merge
5h 17m
Merged PRs (30d)
10

Description

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)
}
}
```

Contributor guide

Open the contributing guide

Research direction

Start with the Mux Match and Route entry points and reproduce the two cases in the issue's TestWithAll and TestWithoutAll examples. Trace how a routed subrouter is represented during matching; done means /path is false for the shown methods, /path/all is true for GET, and the empty Route case also returns false.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.