Skip to content

Commit e411077

Browse files
tigerwill90tigerwill90
authored
Always match routes against percent-encoded paths. (#78)
* feat: decode percent-encoded path segments per RFC 3986 for route matching * refactor: match routes against percent-encoded paths instead of decoding segments
1 parent 353ee02 commit e411077

8 files changed

Lines changed: 168 additions & 68 deletions

File tree

‎benchmark_test.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,12 @@ func BenchmarkGithubAll(b *testing.B) {
7878
require.NoError(b, onlyError(f.Add([]string{route.method}, route.path, emptyHandler)))
7979
}
8080

81-
benchRoute(b, f, githubAPI)
81+
data := make([]route, 0, len(githubAPI))
82+
for _, r := range githubAPI {
83+
data = append(data, route{method: r.method, path: replaceParams.ReplaceAllString(r.path, "xxx")})
84+
}
85+
86+
benchRoute(b, f, data)
8287
}
8388

8489
func BenchmarkStaticAllMux(b *testing.B) {

‎context.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,10 @@ func (c *Context) Method() string {
208208

209209
// Path returns the request [url.URL.RawPath] if not empty, or fallback to the [url.URL.Path].
210210
func (c *Context) Path() string {
211-
path := c.req.URL.Path
212211
if len(c.req.URL.RawPath) > 0 {
213-
// Using RawPath to prevent unintended match (e.g. /search/a%2Fb/1)
214-
path = c.req.URL.RawPath
212+
return c.req.URL.RawPath
215213
}
216-
return path
214+
return c.req.URL.Path
217215
}
218216

219217
// Host returns the request host.

‎fox.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"sync/atomic"
2020

2121
"github.com/fox-toolkit/fox/internal/slicesutil"
22+
"github.com/fox-toolkit/fox/internal/stringsutil"
2223
)
2324

2425
const (
@@ -362,7 +363,7 @@ func (fox *Router) Match(method string, r *http.Request) (route *Route, tsr bool
362363
defer tree.pool.Put(c)
363364
c.resetWithRequest(r)
364365

365-
path := c.Path()
366+
path := routingPath(r)
366367

367368
idx, n, tsr := tree.lookup(method, r.Host, path, c, true)
368369
if n != nil {
@@ -381,7 +382,7 @@ func (fox *Router) Lookup(w ResponseWriter, r *http.Request) (route *Route, cc *
381382
c := tree.pool.Get().(*Context)
382383
c.resetWithWriter(w, r)
383384

384-
path := c.Path()
385+
path := routingPath(r)
385386

386387
idx, n, tsr := tree.lookup(r.Method, r.Host, path, c, false)
387388
if n != nil {
@@ -591,7 +592,7 @@ func (fox *Router) ServeHTTP(w http.ResponseWriter, r *http.Request) {
591592
c := tree.pool.Get().(*Context)
592593
c.reset(w, r)
593594

594-
path := c.Path()
595+
path := routingPath(r)
595596

596597
idx, n, tsr := tree.lookup(r.Method, r.Host, path, c, false)
597598
if !tsr && n != nil {
@@ -998,7 +999,7 @@ func Sub(router *Router) HandlerFunc {
998999
// reslice from the original path to include it, avoiding an allocation from "/" + suffix.
9991000
suffix := cmp.Or((*c.params)[len(*c.params)-1], "/")
10001001
if !strings.HasPrefix(suffix, "/") {
1001-
path := c.Path()
1002+
path := routingPath(c.req)
10021003
slashPos := len(path) - len(suffix) - 1
10031004
if path[slashPos] == slashDelim {
10041005
suffix = path[slashPos:]
@@ -1469,6 +1470,13 @@ func braceIndice(s string, startLevel int) int {
14691470
return -1
14701471
}
14711472

1473+
func routingPath(r *http.Request) string {
1474+
if r.URL.RawPath == "" {
1475+
return r.URL.EscapedPath()
1476+
}
1477+
return stringsutil.NormalizeHexUppercase(r.URL.EscapedPath())
1478+
}
1479+
14721480
func applyMiddleware(scope HandlerScope, mws []middleware, h HandlerFunc) HandlerFunc {
14731481
m := h
14741482
for i := len(mws) - 1; i >= 0; i-- {

‎fox_test.go‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"math/rand/v2"
1313
"net/http"
1414
"net/http/httptest"
15+
"net/url"
1516
"regexp"
1617
"slices"
1718
"strings"
@@ -33,6 +34,8 @@ var (
3334
patternHandler = HandlerFunc(func(c *Context) { _ = c.String(200, c.Pattern()) })
3435
)
3536

37+
var replaceParams = regexp.MustCompile(`[*+]?\{[^}]+}`)
38+
3639
type mockResponseWriter struct{}
3740

3841
func (m mockResponseWriter) Header() (h http.Header) { return http.Header{} }
@@ -1086,7 +1089,8 @@ func TestParamsRoute(t *testing.T) {
10861089
key = match[1 : len(match)-1]
10871090
}
10881091
value := match
1089-
assert.Equal(t, value, c.Param(key))
1092+
ps, _ := url.PathUnescape(c.Param(key))
1093+
assert.Equal(t, value, ps)
10901094
}
10911095
assert.Equal(t, c.Path(), c.Pattern())
10921096
_ = c.String(200, c.Path())
@@ -1213,7 +1217,8 @@ func TestParamsRouteTxn(t *testing.T) {
12131217
key = match[1 : len(match)-1]
12141218
}
12151219
value := match
1216-
assert.Equal(t, value, c.Param(key))
1220+
ps, _ := url.PathUnescape(c.Param(key))
1221+
assert.Equal(t, value, ps)
12171222
}
12181223
assert.Equal(t, c.Path(), c.Pattern())
12191224
_ = c.String(200, c.Path())
@@ -1249,7 +1254,8 @@ func TestParamsRouteWithDomain(t *testing.T) {
12491254
key = match[1 : len(match)-1]
12501255
}
12511256
value := match
1252-
assert.Equal(t, value, c.Param(key))
1257+
ps, _ := url.PathUnescape(c.Param(key))
1258+
assert.Equal(t, value, ps)
12531259
}
12541260

12551261
assert.Equal(t, netutil.StripHostPort(c.Host())+c.Path(), c.Pattern())
@@ -1281,7 +1287,8 @@ func TestParamsRouteWithDomainTxn(t *testing.T) {
12811287
key = match[1 : len(match)-1]
12821288
}
12831289
value := match
1284-
assert.Equal(t, value, c.Param(key))
1290+
ps, _ := url.PathUnescape(c.Param(key))
1291+
assert.Equal(t, value, ps)
12851292
}
12861293

12871294
assert.Equal(t, netutil.StripHostPort(c.Host())+c.Path(), c.Pattern())
@@ -1311,11 +1318,18 @@ func TestParamsRouteMalloc(t *testing.T) {
13111318
for _, route := range githubAPI {
13121319
require.NoError(t, onlyError(r.Add([]string{route.method}, route.path, emptyHandler)))
13131320
}
1314-
for _, route := range githubAPI {
1321+
1322+
data := make([]route, 0, len(githubAPI))
1323+
for _, r := range githubAPI {
1324+
data = append(data, route{method: r.method, path: replaceParams.ReplaceAllString(r.path, "xxx")})
1325+
}
1326+
1327+
for _, route := range data {
13151328
req := httptest.NewRequest(route.method, route.path, nil)
13161329
w := httptest.NewRecorder()
13171330
allocs := testing.AllocsPerRun(100, func() { r.ServeHTTP(w, req) })
13181331
assert.Equal(t, float64(0), allocs)
1332+
assert.Equal(t, http.StatusOK, w.Code)
13191333
}
13201334
}
13211335

@@ -1472,7 +1486,13 @@ func TestParamsRouteWithDomainMalloc(t *testing.T) {
14721486
for _, route := range githubAPI {
14731487
require.NoError(t, onlyError(r.Add([]string{route.method}, "foo.{bar}.com"+route.path, emptyHandler)))
14741488
}
1475-
for _, route := range githubAPI {
1489+
1490+
data := make([]route, 0, len(githubAPI))
1491+
for _, r := range githubAPI {
1492+
data = append(data, route{method: r.method, path: replaceParams.ReplaceAllString(r.path, "xxx")})
1493+
}
1494+
1495+
for _, route := range data {
14761496
req := httptest.NewRequest(route.method, route.path, nil)
14771497
req.Host = "foo.{bar}.com"
14781498
w := httptest.NewRecorder()
@@ -1486,7 +1506,13 @@ func TestOverlappingRouteMalloc(t *testing.T) {
14861506
for _, route := range overlappingRoutes {
14871507
require.NoError(t, onlyError(r.Add([]string{route.method}, route.path, emptyHandler)))
14881508
}
1489-
for _, route := range overlappingRoutes {
1509+
1510+
data := make([]route, 0, len(overlappingRoutes))
1511+
for _, r := range overlappingRoutes {
1512+
data = append(data, route{method: r.method, path: replaceParams.ReplaceAllString(r.path, "xxx")})
1513+
}
1514+
1515+
for _, route := range data {
14901516
req := httptest.NewRequest(route.method, route.path, nil)
14911517
w := httptest.NewRecorder()
14921518
allocs := testing.AllocsPerRun(100, func() { r.ServeHTTP(w, req) })
@@ -4721,7 +4747,8 @@ func TestRouter_Lookup(t *testing.T) {
47214747
key = match[1 : len(match)-1]
47224748
}
47234749
value := match
4724-
assert.Equal(t, value, cc.Param(key))
4750+
ps, _ := url.PathUnescape(cc.Param(key))
4751+
assert.Equal(t, value, ps)
47254752
}
47264753

47274754
cc.Close()
@@ -5273,12 +5300,12 @@ func TestFuzzInsertLookupUpdateAndDelete(t *testing.T) {
52735300
inserted := 0
52745301
_ = r.Updates(func(txn *Txn) error {
52755302
for rte := range routes {
5276-
rte, err := txn.Add(MethodGet, "/"+rte, emptyHandler, WithName("/"+rte))
5303+
rr, err := txn.Add(MethodGet, "/"+rte, emptyHandler, WithName("/"+rte))
52775304
if err != nil {
5278-
assert.Nil(t, rte, "route /%s", rte)
5305+
assert.Nilf(t, rr, "route /%s", rte)
52795306
continue
52805307
}
5281-
assert.NotNilf(t, rte, "route /%v", rte)
5308+
assert.NotNilf(t, rr, "route /%s", rte)
52825309
inserted++
52835310
}
52845311
return nil

internal/stringutil/stringutil.go renamed to internal/stringsutil/stringutils.go

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
package stringutil
1+
package stringsutil
2+
3+
import "strings"
24

35
// EqualStringsASCIIIgnoreCase performs case-insensitive comparison of two strings
46
// containing ASCII characters. Only supports ASCII letters (A-Z, a-z), digits (0-9), hyphen (-) and underscore (_).
@@ -46,3 +48,43 @@ func ToLowerASCII(b byte) byte {
4648
}
4749
return b
4850
}
51+
52+
func NormalizeHexUppercase(s string) string {
53+
var buf strings.Builder
54+
for i := 0; i < len(s); i++ {
55+
if s[i] != '%' || i+2 >= len(s) {
56+
if buf.Len() > 0 {
57+
buf.WriteByte(s[i])
58+
}
59+
continue
60+
}
61+
hi, lo := s[i+1], s[i+2]
62+
hiUpper := upperHex(hi)
63+
loUpper := upperHex(lo)
64+
if hi != hiUpper || lo != loUpper {
65+
if buf.Len() == 0 {
66+
buf.Grow(len(s))
67+
buf.WriteString(s[:i])
68+
}
69+
buf.WriteByte('%')
70+
buf.WriteByte(hiUpper)
71+
buf.WriteByte(loUpper)
72+
} else if buf.Len() > 0 {
73+
buf.WriteByte('%')
74+
buf.WriteByte(hi)
75+
buf.WriteByte(lo)
76+
}
77+
i += 2
78+
}
79+
if buf.Len() == 0 {
80+
return s
81+
}
82+
return buf.String()
83+
}
84+
85+
func upperHex(c byte) byte {
86+
if 'a' <= c && c <= 'f' {
87+
return c - ('a' - 'A')
88+
}
89+
return c
90+
}

0 commit comments

Comments
 (0)