Skip to content

Commit 9a0d12c

Browse files
committed
bug: harden cache middleware key generation and restore Methods config
- Restore configurable Methods field (default: GET, HEAD) to replace hardcoded method check, with uppercase normalization and nil vs empty-slice semantics (nil = default, [] = disable caching) - Fix escapeKeyDelimiters fast-path bug: backslash was not checked, allowing collisions between literal "\p" and escaped "|" - Fix path delimiter injection: escape pipe/colon/backslash in request path before boundKeySegment to prevent crafted paths from manipulating cache key structure - Optimize canonicalQueryString: add single-param fast path (skips url.ParseQuery/sort) and use sync.Pool for output buffer - Simplify string conversions: replace utils.CopyString(utils.UnsafeString(buf)) with string(buf) and utils.UnsafeBytes(boundKeySegment(...)) with direct string append - Add comprehensive tests: Methods config (POST caching, bypass, empty-slice, lowercase normalization), escapeKeyDelimiters unit regression test with collision-pair verification - Update docs: Methods field in config table, default config, and migration guide
1 parent 047de64 commit 9a0d12c

6 files changed

Lines changed: 240 additions & 24 deletions

File tree

docs/middleware/cache.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ This prevents common collisions from path-only keys (for example, `/?id=1` vs `/
120120

121121
The middleware **does not include request body/form values in the default cache key**.
122122

123-
Cache lookup/storage is only applied for `GET` and `HEAD` requests. Other HTTP methods always bypass the cache middleware.
123+
Cache lookup/storage is applied only for `GET` and `HEAD` requests by default. Other HTTP methods bypass the cache middleware. You can change this via the `Methods` config field.
124124

125125
If a response sets `Vary`, request lookup/storage is also partitioned by those header values unless `DisableVaryHeaders` is `true`. Responses with `Vary: *` remain uncacheable.
126126

@@ -138,6 +138,7 @@ If a response sets `Vary`, request lookup/storage is also partitioned by those h
138138
| DisableQueryKeys | `bool` | Disables canonicalized query params in keys. | `false` |
139139
| KeyHeaders | `[]string` | Header allow-list used for key partitioning. Names are normalized case-insensitively and sorted. Use `[]string{}` to disable header-based partitioning. | `[]string{"accept","accept-encoding","accept-language"}` |
140140
| KeyCookies | `[]string` | Optional cookie allow-list for key partitioning. Explicit opt-in only; names remain case-sensitive. | `nil` |
141+
| Methods | `[]string` | HTTP methods eligible for caching. Requests whose method is not in this list bypass the cache. | `[]string{fiber.MethodGet, fiber.MethodHead}` |
141142
| DisableVaryHeaders | `bool` | Disables response `Vary` dimensions in cache lookup/storage partitioning. | `false` |
142143
| ExpirationGenerator | `func(fiber.Ctx, *cache.Config) time.Duration` | ExpirationGenerator allows you to generate custom expiration keys based on the request. | `nil` |
143144
| Storage | `fiber.Storage` | Storage is used to store the state of the middleware. | In-memory store |
@@ -162,6 +163,7 @@ var ConfigDefault = Config{
162163
fiber.HeaderAcceptLanguage,
163164
},
164165
KeyCookies: nil,
166+
Methods: []string{fiber.MethodGet, fiber.MethodHead},
165167
DisableVaryHeaders: false,
166168
ExpirationGenerator: nil,
167169
StoreResponseHeaders: false,

docs/whats_new.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1369,7 +1369,7 @@ Cache keys are now redacted in logs and error messages by default, and a `Disabl
13691369

13701370
The default cache key strategy was also hardened. Instead of path-only behavior, keys now use structured request dimensions: method partitioning, path, canonical query string, and selected representation headers (`Accept`, `Accept-Encoding`, `Accept-Language`). This avoids collisions such as `/items?id=1` vs `/items?id=2` while keeping key generation deterministic. New config fields were added for explicit control: `DisableQueryKeys`, `KeyHeaders`, `KeyCookies`, and `DisableVaryHeaders`.
13711371

1372-
As a security/performance default, request body/form values are not part of the default cache key. Cache handling is limited to `GET` and `HEAD` requests.
1372+
As a security/performance default, request body/form values are not part of the default cache key. Cache handling is limited to `GET` and `HEAD` requests by default, configurable via the `Methods` field.
13731373

13741374
:::note
13751375
The deprecated `Store` and `Key` options have been removed in v3. Use `Storage` and `KeyGenerator` instead.
@@ -2890,6 +2890,7 @@ To restore v2 behavior:
28902890
28912891
Additional v3 cache key options:
28922892
2893+
- `Methods`: HTTP methods eligible for caching (default `GET`, `HEAD`)
28932894
- `DisableQueryKeys`: disable canonicalized query args in keys (default `false`)
28942895
- `KeyHeaders`: request header allow-list for key partitioning
28952896
- `KeyCookies`: explicit cookie allow-list for key partitioning

middleware/cache/cache.go

Lines changed: 47 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"fmt"
1111
"math"
1212
"net/url"
13+
"slices"
1314
"sort"
1415
"strings"
1516
"sync"
@@ -253,8 +254,8 @@ func New(config ...Config) fiber.Handler {
253254

254255
requestMethod := c.Method()
255256

256-
// Cache only GET and HEAD requests.
257-
if requestMethod != fiber.MethodGet && requestMethod != fiber.MethodHead {
257+
// Only cache methods listed in cfg.Methods (default: GET, HEAD).
258+
if !slices.Contains(cfg.Methods, requestMethod) {
258259
c.Set(cfg.CacheHeader, cacheUnreachable)
259260
return c.Next()
260261
}
@@ -1284,7 +1285,8 @@ func defaultKeyGenerator(c fiber.Ctx, cfg *Config) string {
12841285
}
12851286

12861287
buf := (*bufPtr)[:0]
1287-
buf = append(buf, boundKeySegment(c.Path())...)
1288+
// Escape delimiters in path to prevent crafted paths from injecting key structure
1289+
buf = append(buf, boundKeySegment(escapeKeyDelimiters(c.Path()))...)
12881290

12891291
if !cfg.DisableQueryKeys {
12901292
buf = append(buf, '|', 'q', '=')
@@ -1301,7 +1303,7 @@ func defaultKeyGenerator(c fiber.Ctx, cfg *Config) string {
13011303
buf = append(buf, canonicalCookieSubset(c, cfg.KeyCookies)...)
13021304
}
13031305

1304-
result := utils.CopyString(utils.UnsafeString(buf))
1306+
result := string(buf)
13051307

13061308
// Reset buffer and return to pool, but discard if it grew too large
13071309
// to prevent pool from retaining oversized buffers
@@ -1314,17 +1316,24 @@ func defaultKeyGenerator(c fiber.Ctx, cfg *Config) string {
13141316
}
13151317

13161318
func canonicalQueryString(uri *fasthttp.URI) string {
1317-
query := utils.CopyString(utils.UnsafeString(uri.QueryString()))
1318-
if query == "" {
1319+
raw := uri.QueryString()
1320+
if len(raw) == 0 {
13191321
return ""
13201322
}
13211323

1322-
// Pre-scan query string to detect excessive parameters before expensive parsing
1323-
// This prevents DoS via url.ParseQuery allocating large maps/slices
1324+
query := utils.CopyString(utils.UnsafeString(raw))
1325+
1326+
// Pre-scan query string to detect excessive parameters before expensive parsing.
1327+
// This prevents DoS via url.ParseQuery allocating large maps/slices.
13241328
if len(query) > maxQueryBufferSize {
13251329
return boundKeySegment(query)
13261330
}
13271331

1332+
// Fast path: single key=value pair needs no parsing or sorting
1333+
if strings.IndexByte(query, '&') < 0 {
1334+
return boundKeySegment(query)
1335+
}
1336+
13281337
// Quick count of potential parameters (ampersands + 1)
13291338
paramCount := 1
13301339
for i := 0; i < len(query); i++ {
@@ -1357,10 +1366,15 @@ func canonicalQueryString(uri *fasthttp.URI) string {
13571366
}
13581367
sort.Strings(keys)
13591368

1360-
// Use a bounded buffer to prevent excessive memory allocation during URL escaping
1361-
// URL escaping can expand strings up to 3x (each byte -> %XX)
1362-
initialCap := min(len(query)*2, maxQueryBufferSize/2)
1363-
buf := make([]byte, 0, initialCap)
1369+
// Use pooled buffer to prevent excessive memory allocation during URL escaping.
1370+
// URL escaping can expand strings up to 3x (each byte -> %XX).
1371+
v := keyBufferPool.Get()
1372+
bufPtr, ok := v.(*[]byte)
1373+
if !ok || bufPtr == nil {
1374+
b := make([]byte, 0, defaultKeyBufferCap)
1375+
bufPtr = &b
1376+
}
1377+
buf := (*bufPtr)[:0]
13641378

13651379
for _, key := range keys {
13661380
values := parsed[key]
@@ -1370,12 +1384,15 @@ func canonicalQueryString(uri *fasthttp.URI) string {
13701384
buf = append(buf, '&')
13711385
}
13721386

1373-
// Check buffer size before appending to prevent unbounded growth
13741387
escapedKey := url.QueryEscape(key)
13751388
escapedValue := url.QueryEscape(value)
13761389

1377-
// If buffer would exceed safe limits, hash the entire query
1390+
// Check buffer size before appending to prevent unbounded growth
13781391
if len(buf)+len(escapedKey)+len(escapedValue)+2 > maxQueryBufferSize {
1392+
if cap(buf) <= defaultKeyBufferCap*4 {
1393+
*bufPtr = buf
1394+
keyBufferPool.Put(bufPtr)
1395+
}
13791396
return boundKeySegment(query)
13801397
}
13811398

@@ -1385,7 +1402,15 @@ func canonicalQueryString(uri *fasthttp.URI) string {
13851402
}
13861403
}
13871404

1388-
return boundKeySegment(utils.CopyString(utils.UnsafeString(buf)))
1405+
result := boundKeySegment(string(buf))
1406+
1407+
// Return buffer to pool if not oversized
1408+
if cap(buf) <= defaultKeyBufferCap*4 {
1409+
*bufPtr = buf
1410+
keyBufferPool.Put(bufPtr)
1411+
}
1412+
1413+
return result
13891414
}
13901415

13911416
func canonicalHeaderSubset(header *fasthttp.RequestHeader, names []string) string {
@@ -1404,10 +1429,10 @@ func canonicalHeaderSubset(header *fasthttp.RequestHeader, names []string) strin
14041429
headerValue := header.Peek(name)
14051430
// Escape value to prevent delimiter injection
14061431
escapedValue := escapeKeyDelimiters(utils.UnsafeString(headerValue))
1407-
buf = append(buf, utils.UnsafeBytes(boundKeySegment(escapedValue))...)
1432+
buf = append(buf, boundKeySegment(escapedValue)...)
14081433
}
14091434

1410-
return utils.CopyString(utils.UnsafeString(buf))
1435+
return string(buf)
14111436
}
14121437

14131438
func canonicalCookieSubset(c fiber.Ctx, names []string) string {
@@ -1426,17 +1451,17 @@ func canonicalCookieSubset(c fiber.Ctx, names []string) string {
14261451
cookieValue := c.Cookies(name)
14271452
// Escape value to prevent delimiter injection
14281453
escapedValue := escapeKeyDelimiters(cookieValue)
1429-
buf = append(buf, utils.UnsafeBytes(boundKeySegment(escapedValue))...)
1454+
buf = append(buf, boundKeySegment(escapedValue)...)
14301455
}
14311456

1432-
return utils.CopyString(utils.UnsafeString(buf))
1457+
return string(buf)
14331458
}
14341459

1435-
// escapeKeyDelimiters escapes pipe and colon characters used as delimiters in cache keys
1460+
// escapeKeyDelimiters escapes pipe, colon, and backslash characters used as delimiters in cache keys
14361461
// to prevent injection attacks where crafted values could collide with different inputs
14371462
func escapeKeyDelimiters(s string) string {
1438-
// Fast path: no delimiters to escape
1439-
if !strings.ContainsAny(s, "|:") {
1463+
// Fast path: no characters to escape
1464+
if !strings.ContainsAny(s, "|:\\") {
14401465
return s
14411466
}
14421467

middleware/cache/cache_security_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,3 +539,59 @@ func Test_Cache_Security_DelimiterCollisionPrevention(t *testing.T) {
539539
seen[resp] = true
540540
}
541541
}
542+
543+
// Test_Cache_Security_EscapeKeyDelimiters_Unit is a direct regression test for the
544+
// escapeKeyDelimiters function, ensuring backslashes are escaped to prevent collisions
545+
// between e.g. a literal "a\pb" and the escaped form of "a|b" → "a\pb".
546+
func Test_Cache_Security_EscapeKeyDelimiters_Unit(t *testing.T) {
547+
t.Parallel()
548+
549+
tests := []struct {
550+
input string
551+
expected string
552+
}{
553+
// Fast path: no special characters
554+
{"hello", "hello"},
555+
{"", ""},
556+
{"foo/bar?baz=1", "foo/bar?baz=1"},
557+
// Pipe escaping
558+
{"a|b", "a\\pb"},
559+
// Colon escaping
560+
{"a:b", "a\\cb"},
561+
// Backslash escaping (regression: fast path must also check for \)
562+
{"a\\b", "a\\\\b"},
563+
// Backslash-pipe sequence must not collide with escaped pipe
564+
{"a\\pb", "a\\\\pb"}, // literal \p → \\p (differs from escaped | → \p)
565+
{"a\\cb", "a\\\\cb"}, // literal \c → \\c (differs from escaped : → \c)
566+
// Mixed delimiters
567+
{"k|v:w\\x", "k\\pv\\cw\\\\x"},
568+
// Multiple consecutive
569+
{"||", "\\p\\p"},
570+
{"::", "\\c\\c"},
571+
{"\\\\", "\\\\\\\\"},
572+
}
573+
574+
for _, tt := range tests {
575+
t.Run(fmt.Sprintf("escape_%q", tt.input), func(t *testing.T) {
576+
t.Parallel()
577+
result := escapeKeyDelimiters(tt.input)
578+
require.Equal(t, tt.expected, result, "escapeKeyDelimiters(%q)", tt.input)
579+
})
580+
}
581+
582+
// Verify no collisions between pairs that would collide without backslash escaping
583+
collisionPairs := [][2]string{
584+
{"a\\pb", "a|b"}, // literal \p vs escaped |
585+
{"a\\cb", "a:b"}, // literal \c vs escaped :
586+
{"\\\\", "\\"}, // double backslash vs single
587+
{"x\\py", "x|y"},
588+
}
589+
for _, pair := range collisionPairs {
590+
t.Run(fmt.Sprintf("no_collision_%q_vs_%q", pair[0], pair[1]), func(t *testing.T) {
591+
t.Parallel()
592+
a := escapeKeyDelimiters(pair[0])
593+
b := escapeKeyDelimiters(pair[1])
594+
require.NotEqual(t, a, b, "escapeKeyDelimiters(%q) must differ from escapeKeyDelimiters(%q)", pair[0], pair[1])
595+
})
596+
}
597+
}

middleware/cache/cache_test.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,6 +964,118 @@ func Test_Cache_Post(t *testing.T) {
964964
require.Equal(t, "3:12345", string(body))
965965
}
966966

967+
func Test_Cache_CustomMethods(t *testing.T) {
968+
t.Parallel()
969+
970+
t.Run("POST cached when in Methods", func(t *testing.T) {
971+
t.Parallel()
972+
app := fiber.New()
973+
app.Use(New(Config{
974+
Methods: []string{fiber.MethodGet, fiber.MethodHead, fiber.MethodPost},
975+
}))
976+
977+
var count atomic.Int32
978+
app.Post("/", func(c fiber.Ctx) error {
979+
current := count.Add(1)
980+
return c.SendString(strconv.Itoa(int(current)))
981+
})
982+
983+
// First POST — cache miss
984+
resp, err := app.Test(httptest.NewRequest(fiber.MethodPost, "/", http.NoBody))
985+
require.NoError(t, err)
986+
body, err := io.ReadAll(resp.Body)
987+
require.NoError(t, err)
988+
require.Equal(t, cacheMiss, resp.Header.Get("X-Cache"))
989+
require.Equal(t, "1", string(body))
990+
991+
// Second POST — cache hit
992+
resp, err = app.Test(httptest.NewRequest(fiber.MethodPost, "/", http.NoBody))
993+
require.NoError(t, err)
994+
body, err = io.ReadAll(resp.Body)
995+
require.NoError(t, err)
996+
require.Equal(t, cacheHit, resp.Header.Get("X-Cache"))
997+
require.Equal(t, "1", string(body))
998+
})
999+
1000+
t.Run("unconfigured method bypasses cache", func(t *testing.T) {
1001+
t.Parallel()
1002+
app := fiber.New()
1003+
app.Use(New(Config{
1004+
Methods: []string{fiber.MethodGet},
1005+
}))
1006+
1007+
var count atomic.Int32
1008+
app.Put("/", func(c fiber.Ctx) error {
1009+
current := count.Add(1)
1010+
return c.SendString(strconv.Itoa(int(current)))
1011+
})
1012+
1013+
// PUT not in Methods — always bypasses cache
1014+
resp, err := app.Test(httptest.NewRequest(fiber.MethodPut, "/", http.NoBody))
1015+
require.NoError(t, err)
1016+
require.Equal(t, cacheUnreachable, resp.Header.Get("X-Cache"))
1017+
require.Equal(t, int32(1), count.Load())
1018+
1019+
resp, err = app.Test(httptest.NewRequest(fiber.MethodPut, "/", http.NoBody))
1020+
require.NoError(t, err)
1021+
require.Equal(t, cacheUnreachable, resp.Header.Get("X-Cache"))
1022+
require.Equal(t, int32(2), count.Load(), "handler must be called on every bypass")
1023+
})
1024+
1025+
t.Run("empty Methods slice disables caching", func(t *testing.T) {
1026+
t.Parallel()
1027+
app := fiber.New()
1028+
app.Use(New(Config{
1029+
Methods: []string{},
1030+
}))
1031+
1032+
var count atomic.Int32
1033+
app.Get("/", func(c fiber.Ctx) error {
1034+
current := count.Add(1)
1035+
return c.SendString(strconv.Itoa(int(current)))
1036+
})
1037+
1038+
// Even GET bypasses cache when Methods is explicitly empty
1039+
resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", http.NoBody))
1040+
require.NoError(t, err)
1041+
require.Equal(t, cacheUnreachable, resp.Header.Get("X-Cache"))
1042+
1043+
resp, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/", http.NoBody))
1044+
require.NoError(t, err)
1045+
require.Equal(t, cacheUnreachable, resp.Header.Get("X-Cache"))
1046+
require.Equal(t, int32(2), count.Load(), "handler must be called each time with empty Methods")
1047+
})
1048+
1049+
t.Run("lowercase method names are normalized", func(t *testing.T) {
1050+
t.Parallel()
1051+
app := fiber.New()
1052+
app.Use(New(Config{
1053+
Methods: []string{"get", "post"},
1054+
}))
1055+
1056+
var count atomic.Int32
1057+
app.Get("/", func(c fiber.Ctx) error {
1058+
current := count.Add(1)
1059+
return c.SendString(strconv.Itoa(int(current)))
1060+
})
1061+
1062+
// "get" should be normalized to "GET" and match
1063+
resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", http.NoBody))
1064+
require.NoError(t, err)
1065+
body, err := io.ReadAll(resp.Body)
1066+
require.NoError(t, err)
1067+
require.Equal(t, cacheMiss, resp.Header.Get("X-Cache"))
1068+
require.Equal(t, "1", string(body))
1069+
1070+
resp, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/", http.NoBody))
1071+
require.NoError(t, err)
1072+
body, err = io.ReadAll(resp.Body)
1073+
require.NoError(t, err)
1074+
require.Equal(t, cacheHit, resp.Header.Get("X-Cache"))
1075+
require.Equal(t, "1", string(body))
1076+
})
1077+
}
1078+
9671079
func Test_Cache_DefaultKeyDimensions(t *testing.T) {
9681080
t.Parallel()
9691081

0 commit comments

Comments
 (0)