Skip to content

Commit a779888

Browse files
gabygaby
authored
🧹 chore: Improve cache middleware RFC compliance (#3488)
* test: cover cache enhancements * Fix lint issues and tests names * Fix test * Use utils.ToLower() * Use utils.Trim * Add missing param
1 parent 9562053 commit a779888

File tree

4 files changed

+160
-15
lines changed

4 files changed

+160
-15
lines changed

‎docs/whats_new.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,7 @@ The BasicAuth middleware was updated for improved robustness in parsing the Auth
977977

978978
We are excited to introduce a new option in our caching middleware: Cache Invalidator. This feature provides greater control over cache management, allowing you to define a custom conditions for invalidating cache entries.
979979
Additionally, the caching middleware has been optimized to avoid caching non-cacheable status codes, as defined by the [HTTP standards](https://datatracker.ietf.org/doc/html/rfc7231#section-6.1). This improvement enhances cache accuracy and reduces unnecessary cache storage usage.
980+
Cached responses now include an RFC-compliant Age header, providing a standardized indication of how long a response has been stored in cache since it was originally generated. This enhancement improves HTTP compliance and facilitates better client-side caching strategies.
980981

981982
### CORS
982983

‎middleware/cache/cache.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,23 @@ func New(config ...Config) fiber.Handler {
162162
for k, v := range e.headers {
163163
c.Response().Header.SetBytesV(k, v)
164164
}
165-
// Set Cache-Control header if enabled
166-
if cfg.CacheControl {
165+
// Set Cache-Control header if enabled and not already set
166+
if cfg.CacheControl && len(c.Response().Header.Peek(fiber.HeaderCacheControl)) == 0 {
167167
maxAge := strconv.FormatUint(e.exp-ts, 10)
168168
c.Set(fiber.HeaderCacheControl, "public, max-age="+maxAge)
169169
}
170170

171+
// RFC-compliant Age header
172+
age := strconv.FormatUint(e.ttl-(e.exp-ts), 10)
173+
c.Response().Header.Set(fiber.HeaderAge, age)
174+
171175
c.Set(cfg.CacheHeader, cacheHit)
172176

177+
// release item allocated from storage
178+
if cfg.Storage != nil {
179+
manager.release(e)
180+
}
181+
173182
mux.Unlock()
174183

175184
// Return response
@@ -185,6 +194,12 @@ func New(config ...Config) fiber.Handler {
185194
return err
186195
}
187196

197+
// Respect server cache-control: no-store
198+
if strings.Contains(utils.ToLower(string(c.Response().Header.Peek(fiber.HeaderCacheControl))), noStore) {
199+
c.Set(cfg.CacheHeader, cacheUnreachable)
200+
return nil
201+
}
202+
188203
// Don't cache response if status code is not cacheable
189204
if !cacheableStatusCodes[c.Response().StatusCode()] {
190205
c.Set(cfg.CacheHeader, cacheUnreachable)
@@ -224,6 +239,10 @@ func New(config ...Config) fiber.Handler {
224239
e.ctype = utils.CopyBytes(c.Response().Header.ContentType())
225240
e.cencoding = utils.CopyBytes(c.Response().Header.Peek(fiber.HeaderContentEncoding))
226241

242+
if len(c.Response().Header.Peek(fiber.HeaderAge)) == 0 {
243+
c.Response().Header.Set(fiber.HeaderAge, "0")
244+
}
245+
227246
// Store all response headers
228247
// (more: https://datatracker.ietf.org/doc/html/rfc2616#section-13.5.1)
229248
if cfg.StoreResponseHeaders {
@@ -241,11 +260,15 @@ func New(config ...Config) fiber.Handler {
241260

242261
// default cache expiration
243262
expiration := cfg.Expiration
263+
if v, ok := parseMaxAge(string(c.Response().Header.Peek(fiber.HeaderCacheControl))); ok {
264+
expiration = v
265+
}
244266
// Calculate expiration by response header or other setting
245267
if cfg.ExpirationGenerator != nil {
246268
expiration = cfg.ExpirationGenerator(c, &cfg)
247269
}
248270
e.exp = ts + uint64(expiration.Seconds())
271+
e.ttl = uint64(expiration.Seconds())
249272

250273
// Store entry in heap
251274
if cfg.MaxBytes > 0 {
@@ -276,3 +299,16 @@ func New(config ...Config) fiber.Handler {
276299
func hasRequestDirective(c fiber.Ctx, directive string) bool {
277300
return strings.Contains(c.Get(fiber.HeaderCacheControl), directive)
278301
}
302+
303+
// parseMaxAge extracts the max-age directive from a Cache-Control header.
304+
func parseMaxAge(cc string) (time.Duration, bool) {
305+
for _, part := range strings.Split(cc, ",") {
306+
part = utils.Trim(utils.ToLower(part), ' ')
307+
if strings.HasPrefix(part, "max-age=") {
308+
if sec, err := strconv.Atoi(strings.TrimPrefix(part, "max-age=")); err == nil {
309+
return time.Duration(sec) * time.Second, true
310+
}
311+
}
312+
}
313+
return 0, false
314+
}

‎middleware/cache/cache_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -999,6 +999,111 @@ func Test_Cache_UncacheableStatusCodes(t *testing.T) {
999999
}
10001000
}
10011001

1002+
func TestCacheAgeHeader(t *testing.T) {
1003+
t.Parallel()
1004+
app := fiber.New()
1005+
app.Use(New(Config{Expiration: 10 * time.Second}))
1006+
app.Get("/", func(c fiber.Ctx) error { return c.SendString("ok") })
1007+
1008+
resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
1009+
require.NoError(t, err)
1010+
require.Equal(t, "0", resp.Header.Get(fiber.HeaderAge))
1011+
1012+
time.Sleep(4 * time.Second)
1013+
1014+
resp, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
1015+
require.NoError(t, err)
1016+
require.Equal(t, cacheHit, resp.Header.Get("X-Cache"))
1017+
age, err := strconv.Atoi(resp.Header.Get(fiber.HeaderAge))
1018+
require.NoError(t, err)
1019+
require.Positive(t, age)
1020+
}
1021+
1022+
func Test_CacheNoStoreDirective(t *testing.T) {
1023+
t.Parallel()
1024+
app := fiber.New()
1025+
app.Use(New())
1026+
app.Get("/", func(c fiber.Ctx) error {
1027+
c.Set(fiber.HeaderCacheControl, "no-store")
1028+
return c.SendString("ok")
1029+
})
1030+
1031+
resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
1032+
require.NoError(t, err)
1033+
require.Equal(t, cacheUnreachable, resp.Header.Get("X-Cache"))
1034+
1035+
resp, err = app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
1036+
require.NoError(t, err)
1037+
require.Equal(t, cacheUnreachable, resp.Header.Get("X-Cache"))
1038+
}
1039+
1040+
func Test_CacheControlNotOverwritten(t *testing.T) {
1041+
t.Parallel()
1042+
app := fiber.New()
1043+
app.Use(New(Config{CacheControl: true, Expiration: 10 * time.Second, StoreResponseHeaders: true}))
1044+
app.Get("/", func(c fiber.Ctx) error {
1045+
c.Set(fiber.HeaderCacheControl, "private")
1046+
return c.SendString("ok")
1047+
})
1048+
1049+
_, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
1050+
require.NoError(t, err)
1051+
1052+
resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
1053+
require.NoError(t, err)
1054+
require.Equal(t, "private", resp.Header.Get(fiber.HeaderCacheControl))
1055+
}
1056+
1057+
func Test_CacheMaxAgeDirective(t *testing.T) {
1058+
t.Parallel()
1059+
app := fiber.New()
1060+
app.Use(New(Config{Expiration: 10 * time.Second}))
1061+
app.Get("/", func(c fiber.Ctx) error {
1062+
c.Set(fiber.HeaderCacheControl, "max-age=1")
1063+
return c.SendString("1")
1064+
})
1065+
1066+
_, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
1067+
require.NoError(t, err)
1068+
1069+
time.Sleep(1500 * time.Millisecond)
1070+
1071+
resp, err := app.Test(httptest.NewRequest(fiber.MethodGet, "/", nil))
1072+
require.NoError(t, err)
1073+
require.Equal(t, cacheMiss, resp.Header.Get("X-Cache"))
1074+
}
1075+
1076+
func Test_ParseMaxAge(t *testing.T) {
1077+
t.Parallel()
1078+
tests := []struct {
1079+
header string
1080+
expect time.Duration
1081+
ok bool
1082+
}{
1083+
{"max-age=60", 60 * time.Second, true},
1084+
{"public, max-age=86400", 86400 * time.Second, true},
1085+
{"no-store", 0, false},
1086+
{"max-age=invalid", 0, false},
1087+
{"public, s-maxage=100, max-age=50", 50 * time.Second, true},
1088+
{"MAX-AGE=20", 20 * time.Second, true},
1089+
{"public , max-age=0", 0, true},
1090+
{"public , max-age", 0, false},
1091+
}
1092+
1093+
for _, tt := range tests {
1094+
t.Run(tt.header, func(t *testing.T) {
1095+
t.Parallel()
1096+
d, ok := parseMaxAge(tt.header)
1097+
if tt.ok != ok {
1098+
t.Fatalf("expected ok=%v got %v", tt.ok, ok)
1099+
}
1100+
if ok && d != tt.expect {
1101+
t.Fatalf("expected %v got %v", tt.expect, d)
1102+
}
1103+
})
1104+
}
1105+
}
1106+
10021107
// go test -v -run=^$ -bench=Benchmark_Cache -benchmem -count=4
10031108
func Benchmark_Cache(b *testing.B) {
10041109
app := fiber.New()

‎middleware/cache/manager.go

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ type item struct {
1818
cencoding []byte
1919
status int
2020
exp uint64
21+
ttl uint64
2122
// used for finding the item in an indexed heap
2223
heapidx int
2324
}
@@ -55,8 +56,8 @@ func (m *manager) acquire() *item {
5556

5657
// release and reset *entry to sync.Pool
5758
func (m *manager) release(e *item) {
58-
// don't release item if we using memory storage
59-
if m.storage != nil {
59+
// don't release item if we using in-memory storage
60+
if m.storage == nil {
6061
return
6162
}
6263
e.body = nil
@@ -69,24 +70,26 @@ func (m *manager) release(e *item) {
6970

7071
// get data from storage or memory
7172
func (m *manager) get(key string) *item {
72-
var it *item
7373
if m.storage != nil {
74-
it = m.acquire()
7574
raw, err := m.storage.Get(key)
76-
if err != nil {
77-
return it
75+
if err != nil || raw == nil {
76+
return nil
7877
}
79-
if raw != nil {
80-
if _, err := it.UnmarshalMsg(raw); err != nil {
81-
return it
82-
}
78+
79+
it := m.acquire()
80+
if _, err := it.UnmarshalMsg(raw); err != nil {
81+
m.release(it)
82+
return nil
8383
}
84+
8485
return it
8586
}
86-
if it, _ = m.memory.Get(key).(*item); it == nil { //nolint:errcheck // We store nothing else in the pool
87-
return nil
87+
88+
if it, _ := m.memory.Get(key).(*item); it != nil { //nolint:errcheck // We store nothing else in the pool
89+
return it
8890
}
89-
return it
91+
92+
return nil
9093
}
9194

9295
// get raw data from storage or memory

0 commit comments

Comments
 (0)