Skip to content

Commit a715012

Browse files
committed
fix(subrouter): stop aliasing route.params via Context.paramsKeys
1 parent 14bab0f commit a715012

4 files changed

Lines changed: 49 additions & 13 deletions

File tree

context.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ func (c *Context) reset(w http.ResponseWriter, r *http.Request) {
9696
c.cachedQueries = nil
9797
c.scope = RouteHandler
9898
*c.params = (*c.params)[:0]
99+
*c.paramsKeys = (*c.paramsKeys)[:0]
99100
*c.subPatterns = (*c.subPatterns)[:0]
100101
}
101102

@@ -105,6 +106,7 @@ func (c *Context) resetNil() {
105106
c.cachedQueries = nil
106107
c.route = nil
107108
*c.params = (*c.params)[:0]
109+
*c.paramsKeys = (*c.paramsKeys)[:0]
108110
*c.subPatterns = (*c.subPatterns)[:0]
109111
}
110112

@@ -126,6 +128,7 @@ func (c *Context) resetWithWriter(w ResponseWriter, r *http.Request) {
126128
c.cachedQueries = nil
127129
c.scope = RouteHandler
128130
*c.params = (*c.params)[:0]
131+
*c.paramsKeys = (*c.paramsKeys)[:0]
129132
*c.subPatterns = (*c.subPatterns)[:0]
130133
}
131134

@@ -188,8 +191,9 @@ func (c *Context) ClientIP() (*net.IPAddr, error) {
188191
// Params returns an iterator over the matched wildcard parameters for the current route.
189192
func (c *Context) Params() iter.Seq[Param] {
190193
return func(yield func(Param) bool) {
194+
keys := c.keys()
191195
for i, p := range *c.params {
192-
if !yield(Param{Key: (*c.paramsKeys)[i], Value: p}) {
196+
if !yield(Param{Key: keys[i], Value: p}) {
193197
return
194198
}
195199
}
@@ -198,15 +202,25 @@ func (c *Context) Params() iter.Seq[Param] {
198202

199203
// Param retrieve a matching wildcard segment by name.
200204
func (c *Context) Param(name string) string {
201-
for i := range *c.params {
202-
key := (*c.paramsKeys)[i]
203-
if key == name {
204-
return (*c.params)[i]
205+
keys := c.keys()
206+
for i, p := range *c.params {
207+
if keys[i] == name {
208+
return p
205209
}
206210
}
207211
return ""
208212
}
209213

214+
func (c *Context) keys() []string {
215+
if len(*c.paramsKeys) > 0 {
216+
return *c.paramsKeys
217+
}
218+
if c.route != nil {
219+
return c.route.params
220+
}
221+
return nil
222+
}
223+
210224
// Method returns the request method.
211225
func (c *Context) Method() string {
212226
return c.req.Method

fox.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,6 @@ func (fox *Router) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc *
398398
if n != nil {
399399
c.route = n.routes[idx]
400400
c.pattern = c.route.pattern.str
401-
*c.paramsKeys = c.route.params
402401
return c.route, c, tsr
403402
}
404403

@@ -611,7 +610,6 @@ func (fox *Router) ServeHTTP(w http.ResponseWriter, r *http.Request) {
611610
if !tsr && n != nil {
612611
c.route = n.routes[idx]
613612
c.pattern = c.route.pattern.str
614-
*c.paramsKeys = c.route.params
615613
c.route.hall(c)
616614
return
617615
}
@@ -622,7 +620,6 @@ func (fox *Router) ServeHTTP(w http.ResponseWriter, r *http.Request) {
622620
if route.handleSlash == RelaxedSlash {
623621
c.route = route
624622
c.pattern = c.route.pattern.str
625-
*c.paramsKeys = c.route.params
626623
route.hall(c)
627624
return
628625
}
@@ -643,7 +640,6 @@ func (fox *Router) ServeHTTP(w http.ResponseWriter, r *http.Request) {
643640
if idx, n, tsr := tree.lookup(r.Method, r.Host, CleanPath(path), c, false); n != nil && (!tsr || n.routes[idx].handleSlash == RelaxedSlash) {
644641
c.route = n.routes[idx]
645642
c.pattern = c.route.pattern.str
646-
*c.paramsKeys = c.route.params
647643
c.route.hall(c)
648644
return
649645
}
@@ -977,18 +973,25 @@ func Sub(router *Router) HandlerFunc {
977973
*subCtx.subPatterns = append(*subCtx.subPatterns, *c.subPatterns...)
978974

979975
lastTk := route.pattern.tokens[len(route.pattern.tokens)-1]
976+
// For a top-level parent c.paramsKeys is empty, so we
977+
// read the last key directly from c.route.params. For nested
978+
// sub-routers, c.paramsKeys holds the composed chain.
979+
keys := *c.paramsKeys
980+
if len(keys) == 0 {
981+
keys = c.route.params
982+
}
980983
var p string
981984
switch lastTk.typ {
982985
case nodeWildcard:
983-
key := (*c.paramsKeys)[len(*c.paramsKeys)-1]
986+
key := keys[len(keys)-1]
984987
extra := len(key) + wildcardExtraChar
985988
if lastTk.regexp != nil {
986989
// +1 for the ':' separator between name and regex source.
987990
extra += 1 + len(rawExpr(lastTk.regexp))
988991
}
989992
p = strings.TrimSuffix(c.pattern[:len(c.pattern)-extra], "/")
990993
case nodeParam:
991-
key := (*c.paramsKeys)[len(*c.paramsKeys)-1]
994+
key := keys[len(keys)-1]
992995
extra := len(key) + paramExtraChar
993996
if lastTk.regexp != nil {
994997
extra += 1 + len(rawExpr(lastTk.regexp))
@@ -1029,7 +1032,7 @@ func Sub(router *Router) HandlerFunc {
10291032
// Subrouters are never evaluated in lazy lookup mode, so params are always
10301033
// captured. If parent has no params beyond the catch-all, this is a no-op.
10311034
*subCtx.params = append(*subCtx.params, (*c.params)[:len(*c.params)-1]...)
1032-
*subCtx.paramsKeys = append((*subCtx.paramsKeys)[:0], (*c.paramsKeys)[:len(*c.paramsKeys)-1]...)
1035+
*subCtx.paramsKeys = append((*subCtx.paramsKeys)[:0], keys[:len(keys)-1]...)
10331036

10341037
// Serve the sub router
10351038
router.serveSubRouter(subCtx, suffix)

fox_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,6 +1506,26 @@ func TestRouter_ServeHTTP_HandleSubRouter(t *testing.T) {
15061506
assert.Equal(t, "/api/users", w.Body.String())
15071507
})
15081508

1509+
t.Run("sub-router used directly then via Sub preserves route.params", func(t *testing.T) {
1510+
sub := MustRouter()
1511+
subRoute, err := sub.Add(MethodGet, "/users/{id}", emptyHandler)
1512+
require.NoError(t, err)
1513+
originalParams := slices.Clone(subRoute.params)
1514+
1515+
req := httptest.NewRequest(http.MethodGet, "/users/42", nil)
1516+
w := httptest.NewRecorder()
1517+
sub.ServeHTTP(w, req)
1518+
1519+
fx := MustRouter()
1520+
require.NoError(t, onlyError(fx.Add(MethodGet, "/api/{tenant}/+{rest}", Sub(sub))))
1521+
1522+
req = httptest.NewRequest(http.MethodGet, "/api/acme/users/42", nil)
1523+
w = httptest.NewRecorder()
1524+
fx.ServeHTTP(w, req)
1525+
1526+
assert.Equal(t, originalParams, subRoute.params)
1527+
})
1528+
15091529
t.Run("sub-router no route handler sees clean context", func(t *testing.T) {
15101530
var pat, tenant string
15111531
var route *Route

txn.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,6 @@ func (txn *Txn) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc *Con
327327
if n != nil {
328328
c.route = n.routes[idx]
329329
c.pattern = c.route.pattern.str
330-
*c.paramsKeys = c.route.params
331330
return c.route, c, tsr
332331
}
333332

0 commit comments

Comments
 (0)