feat: tag invalidation, conditional requests, and Cache-Control for cache middleware#4041
Conversation
…ache middleware - Tag association via Tags (request-time) and ResponseTags (response-time) hooks - Bidirectional tag index with O(1) lookup; automatic distributed backend when Config.Storage is set, persisting the index in the shared store so that InvalidateTags propagates across all instances - Tag re-population from persisted entry data on cache hit (survives restarts) - RejectTags glob-pattern matching with pre-classified exact/prefix/general buckets - InvalidateTags public API: collects keys from the tag index, evicts from heap, and deletes from storage - ETag: auto-generation (SHA-256 body hash) and custom ETagGenerator; weak If-None-Match validation on hit and miss paths - Last-Modified: auto-generation and custom LastModifiedGenerator; If-Modified-Since validation with RFC 9110 §8.3 precedence - Cache-Control: automatic "public, max-age=<N>[, must-revalidate]" on miss and hit; handler-set Cache-Control stored and replayed verbatim - Benchmarks covering has(), add(), invalidate(), and encode/decode - README documenting tag strategies, invalidation patterns, conditional requests, and Cache-Control configuration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
WalkthroughThis PR extends the cache middleware with tag-based invalidation, RFC 9110 conditional request support (ETag/Last-Modified), and distributed tag store coordination. It introduces a new public API for tag invalidation, tag computation hooks, rejection patterns, and automatic conditional header generation with fallback generators. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as Handler/App
participant Middleware as Cache Middleware
participant TagIndex as Tag Index
participant Storage as Storage Backend
Client->>Handler: GET /resource (no cache)
Handler->>Middleware: Process request
Middleware->>Middleware: Tags(ctx) - compute request tags
Handler->>Handler: Execute handler
Middleware->>Middleware: ResponseTags(ctx, body) - compute response tags
Middleware->>Middleware: Merge Tags + ResponseTags
Middleware->>Middleware: Check RejectTags patterns
alt Not rejected
Middleware->>Middleware: ETagGenerator(ctx, body)
Middleware->>Middleware: LastModifiedGenerator(ctx)
Middleware->>TagIndex: Add key with tags
Middleware->>Storage: Store item + metadata
end
Middleware->>Client: Response + headers (ETag, Last-Modified, Cache-Control)
sequenceDiagram
participant Client
participant Middleware as Cache Middleware
participant Storage as Storage Backend
participant TagIndex as Tag Index
participant DistStore as Distributed TagStore
Client->>Middleware: InvalidateTags(c, tag1, tag2)
Middleware->>Middleware: Retrieve invalidation function from Locals
Middleware->>DistStore: Invalidate(tag1, tag2)
DistStore->>Storage: Read forward indices for tag1, tag2
DistStore->>DistStore: Gather all keys across tags
DistStore->>Storage: Delete forward indices
DistStore->>Storage: Read & delete reverse indices for each key
DistStore->>TagIndex: Update local index
Middleware->>Middleware: Evict gathered keys from cache
Middleware->>Client: Success
sequenceDiagram
participant Client
participant Middleware as Cache Middleware
participant Cache as Cache Store
Client->>Middleware: GET /resource (If-None-Match: "abc123")
Middleware->>Cache: Lookup key
alt Cache hit
Middleware->>Middleware: Retrieve stored ETag
alt ETag matches (weak or strong)
Middleware->>Middleware: Build 304 response
Middleware->>Middleware: Apply Cache-Control (must-revalidate, proxy-revalidate)
Middleware->>Client: 304 Not Modified + headers
else ETag mismatch
Middleware->>Client: 200 + full response
end
else Cache miss
Middleware->>Client: Forward to handler
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @zishanjawed, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the Fiber cache middleware by integrating advanced caching strategies. It introduces a flexible tag-based invalidation system that works seamlessly across single-instance and distributed environments, allowing developers to group and invalidate cache entries logically. Furthermore, it enhances HTTP compliance by adding comprehensive support for RFC 9110 conditional requests, including ETag and Last-Modified header management, which reduces unnecessary data transfers. The middleware also gains intelligent Cache-Control header generation and processing, ensuring optimal cache behavior and adherence to web standards. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant new features to the cache middleware, including tag-based invalidation, conditional request support (ETag/Last-Modified), and improved Cache-Control header handling, with comprehensive documentation and testing. However, a critical vulnerability has been identified in the distributedTagStore due to race conditions, which can lead to inconsistent cache states and failed invalidations in multi-instance deployments. Further security concerns include potential memory exhaustion and Denial of Service risks from manual binary decoding and unbounded tag set growth. Additionally, there are medium-severity issues related to performance in the glob matching logic and robustness in ETag parsing. Addressing these concerns will make this an excellent addition to the Fiber framework.
| func (d *distributedTagStore) add(key string, tags []string) { | ||
| if len(tags) == 0 { | ||
| return | ||
| } | ||
| d.local.add(key, tags) | ||
|
|
||
| d.mu.Lock() | ||
| defer d.mu.Unlock() | ||
|
|
||
| for _, tag := range tags { | ||
| fwdKey := tagKeyPrefix + tag | ||
| fwd := d.readSet(fwdKey) | ||
| if !sliceContains(fwd, key) { | ||
| d.writeSet(fwdKey, append(fwd, key)) | ||
| } | ||
| } | ||
| revKey := tagRevKeyPrefix + key | ||
| rev := d.readSet(revKey) | ||
| for _, tag := range tags { | ||
| if !sliceContains(rev, tag) { | ||
| rev = append(rev, tag) | ||
| } | ||
| } | ||
| d.writeSet(revKey, rev) | ||
| } |
There was a problem hiding this comment.
The distributedTagStore.add method is vulnerable to race conditions in a distributed environment. The use of a local sync.Mutex is insufficient, as concurrent read-modify-write operations across multiple instances will overwrite each other's updates, leading to a corrupted tag index. This can result in inconsistent cache states and failed invalidations, posing a security risk where sensitive or deleted data might persist in the cache. This issue also affects the remove and invalidate methods. To mitigate this, consider using atomic operations provided by your storage backend or implementing a distributed locking mechanism.
| count := binary.LittleEndian.Uint32(data[0:4]) | ||
| ss := make([]string, 0, count) |
There was a problem hiding this comment.
The decodeStringSet function reads a 32-bit count from the input data and uses it to pre-allocate a slice with make([]string, 0, count). If the storage backend contains malformed or malicious data with a very large count, this will trigger a massive memory allocation, potentially leading to a Denial of Service (DoS) via an Out-Of-Memory (OOM) crash. Since each string entry in the encoded format requires at least 4 bytes for its length, the count should be validated against the total length of the data before allocation.
count := binary.LittleEndian.Uint32(data[0:4])
if count > uint32(len(data))/4 {
return nil
}
ss := make([]string, 0, count)| func (d *distributedTagStore) add(key string, tags []string) { | ||
| if len(tags) == 0 { | ||
| return | ||
| } | ||
| d.local.add(key, tags) | ||
|
|
||
| d.mu.Lock() | ||
| defer d.mu.Unlock() | ||
|
|
||
| for _, tag := range tags { | ||
| fwdKey := tagKeyPrefix + tag | ||
| fwd := d.readSet(fwdKey) | ||
| if !sliceContains(fwd, key) { | ||
| d.writeSet(fwdKey, append(fwd, key)) | ||
| } | ||
| } | ||
| revKey := tagRevKeyPrefix + key | ||
| rev := d.readSet(revKey) | ||
| for _, tag := range tags { | ||
| if !sliceContains(rev, tag) { | ||
| rev = append(rev, tag) | ||
| } | ||
| } | ||
| d.writeSet(revKey, rev) | ||
| } |
There was a problem hiding this comment.
The distributedTagStore.add method has O(N) time and space complexity relative to the number of keys associated with a tag, as it must read and write the entire set of keys for every addition. Since there is no limit on the number of keys per tag, an attacker can associate a large number of unique cache keys with a single tag (e.g., by generating many unique URL paths). This will cause the tag's key set to grow indefinitely, making every subsequent request that uses that tag increasingly slow and resource-intensive, eventually leading to a Denial of Service.
| header = header[end+1:] | ||
| } | ||
|
|
||
| candidate = strings.TrimRight(candidate, " ") |
There was a problem hiding this comment.
Using strings.TrimRight here only removes trailing whitespace from the ETag candidate. The If-None-Match header can have leading whitespace as well (e.g., "etag1", "etag2"). To make the parsing more robust, you should trim whitespace from both sides.
I suggest changing this to strings.TrimSpace(candidate).
| candidate = strings.TrimRight(candidate, " ") | |
| candidate = strings.TrimSpace(candidate) |
| for i := range len(s) + 1 { | ||
| if globMatch(pattern, s[i:]) { | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
This recursive implementation of globMatch creates a new string slice s[i:] in each iteration of the loop. For long strings or complex patterns, this can lead to a significant number of allocations and performance degradation.
Consider rewriting this function to be non-recursive or to use indices instead of slicing to avoid these allocations. The standard library's path.Match uses an iterative approach which could serve as a good reference.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
middleware/cache/config.go (1)
81-194:⚠️ Potential issue | 🟠 MajorEnableETag/EnableLastModified defaults drop to false when Config is provided.
configDefaultdoesn’t populate these fields, socache.New(cache.Config{Expiration: ...})turns both features off despite the documented default oftrue. This will surprise most callers who pass a partial config.Consider using pointers (or explicit Disable* flags) so defaults remain true unless explicitly overridden.
💡 One possible fix (pointer-based defaults)
type Config struct { - EnableETag bool + EnableETag *bool - EnableLastModified bool + EnableLastModified *bool } var ConfigDefault = Config{ - EnableETag: true, - EnableLastModified: true, + EnableETag: boolPtr(true), + EnableLastModified: boolPtr(true), } func configDefault(config ...Config) Config { if len(config) < 1 { return ConfigDefault } - cfg := config[0] + cfg := ConfigDefault + user := config[0] // existing defaulting... + if user.EnableETag != nil { + cfg.EnableETag = user.EnableETag + } + if user.EnableLastModified != nil { + cfg.EnableLastModified = user.EnableLastModified + } return cfg } + +func boolPtr(v bool) *bool { return &v }(You’d also need to update cache.go call sites to dereference these pointers.)
🤖 Fix all issues with AI agents
In `@middleware/cache/cache.go`:
- Around line 530-542: The conditional-request logic currently falls back to
evaluating If-Modified-Since when If-None-Match is present but the cached item
has no ETag; change the control flow in the block that computes notModified (and
the analogous block around lines 701-712) so that If-Modified-Since is only
considered when If-None-Match is absent (i.e., check ifIfNoneMatch header first
and if it's non-empty then only evaluate etagWeakMatch with e.etag, otherwise if
If-None-Match is empty evaluate If-Modified-Since using fasthttp.ParseHTTPDate
and secondsToTime(e.lastModified)); ensure you reference and preserve existing
helpers etagWeakMatch, secondsToTime and fields e.etag and e.lastModified when
updating the logic.
- Around line 1129-1132: When registering tags for a cached key (where the code
currently calls ti.add(key, entryTags)), first clear any existing tag mappings
for that key to avoid stale associations; update the logic around ti.add so it
either calls a removal method (e.g., ti.remove(key) or ti.deleteKeyTags(key))
before ti.add, or use/implement a single replace-style method (e.g.,
ti.replace(key, entryTags) or ti.setTags(key, entryTags)) that atomically clears
old tags and adds the new set; ensure this change is applied where key and
entryTags (and ResponseTags) are used so re-caching replaces prior tag mappings
instead of appending.
In `@middleware/cache/README.md`:
- Around line 145-152: Several fenced code blocks in the README lack language
identifiers; update each triple-backtick fence to include a language (use
```text for plain text snippets and ```go for Go code examples) so MD040 is
satisfied—specifically change the block containing the sample starting with
Entry "k1" tagged ["product:1", "region:us"] to ```text and likewise update the
other similar blocks referenced (the snippets around the same example and the
blocks near the other occurrences) to the appropriate language tag.
- Around line 20-23: Several Markdown tables in middleware/cache/README.md
(e.g., the table with headers "Hook", "When it runs", "Signature" and rows for
`Tags` and `ResponseTags`) have incorrect spacing around pipe characters causing
MD060; update all affected tables (lines referenced in the review: around rows
for `Tags`/`ResponseTags` and the other ranges) to include single spaces before
and after each pipe in every row and header so they conform to markdownlint
table spacing rules, then run the repository lint target (`make markdown`) to
normalize formatting and ensure no remaining MD060 violations.
🧹 Nitpick comments (1)
middleware/cache/tags_distributed.go (1)
166-186: Guard decodeStringSet against oversized counts.
countcomes from storage and is used as slice capacity; corrupted data can trigger huge allocations. Consider validatingcountagainst available bytes before allocating.🛡️ Suggested defensive check
count := binary.LittleEndian.Uint32(data[0:4]) + maxCount := uint32((len(data) - 4) / 4) // each entry needs at least a uint32 len + if count > maxCount { + return nil + } ss := make([]string, 0, count)
| // Check conditional request headers (RFC 7232). | ||
| // ETag and Last-Modified are in the cached item; no body load needed for 304. | ||
| { | ||
| ifNoneMatch := c.Request().Header.Peek(fiber.HeaderIfNoneMatch) | ||
| ifModSince := c.Request().Header.Peek(fiber.HeaderIfModifiedSince) | ||
| notModified := false | ||
| if len(ifNoneMatch) > 0 && len(e.etag) > 0 { | ||
| notModified = etagWeakMatch(ifNoneMatch, e.etag) | ||
| } else if len(ifModSince) > 0 && e.lastModified != 0 { | ||
| if modTime, parseErr := fasthttp.ParseHTTPDate(ifModSince); parseErr == nil { | ||
| notModified = !secondsToTime(e.lastModified).After(modTime) | ||
| } | ||
| } |
There was a problem hiding this comment.
If-None-Match should suppress If-Modified-Since even when ETag is missing.
Right now the code falls back to If-Modified-Since when If-None-Match is present but no ETag is stored, which violates RFC 9110 §8.3 (and the README). Evaluate IMS only when If-None-Match is absent.
✅ Suggested fix for both hit and miss paths
- if len(ifNoneMatch) > 0 && len(e.etag) > 0 {
- notModified = etagWeakMatch(ifNoneMatch, e.etag)
- } else if len(ifModSince) > 0 && e.lastModified != 0 {
+ if len(ifNoneMatch) > 0 {
+ if len(e.etag) > 0 {
+ notModified = etagWeakMatch(ifNoneMatch, e.etag)
+ }
+ } else if len(ifModSince) > 0 && e.lastModified != 0 {
if modTime, parseErr := fasthttp.ParseHTTPDate(ifModSince); parseErr == nil {
notModified = !secondsToTime(e.lastModified).After(modTime)
}
}- if len(ifNoneMatch) > 0 && len(storedETag) > 0 {
- notModified = etagWeakMatch(ifNoneMatch, storedETag)
- } else if ifModSince := c.Request().Header.Peek(fiber.HeaderIfModifiedSince); len(ifModSince) > 0 {
+ if len(ifNoneMatch) > 0 {
+ if len(storedETag) > 0 {
+ notModified = etagWeakMatch(ifNoneMatch, storedETag)
+ }
+ } else if ifModSince := c.Request().Header.Peek(fiber.HeaderIfModifiedSince); len(ifModSince) > 0 {
if lm := c.Response().Header.Peek(fiber.HeaderLastModified); len(lm) > 0 {
if modTime, parseErr := fasthttp.ParseHTTPDate(ifModSince); parseErr == nil {
if lmTime, lmErr := fasthttp.ParseHTTPDate(lm); lmErr == nil {
notModified = !lmTime.After(modTime)
}
}
}
}Also applies to: 701-712
🤖 Prompt for AI Agents
In `@middleware/cache/cache.go` around lines 530 - 542, The conditional-request
logic currently falls back to evaluating If-Modified-Since when If-None-Match is
present but the cached item has no ETag; change the control flow in the block
that computes notModified (and the analogous block around lines 701-712) so that
If-Modified-Since is only considered when If-None-Match is absent (i.e., check
ifIfNoneMatch header first and if it's non-empty then only evaluate
etagWeakMatch with e.etag, otherwise if If-None-Match is empty evaluate
If-Modified-Since using fasthttp.ParseHTTPDate and
secondsToTime(e.lastModified)); ensure you reference and preserve existing
helpers etagWeakMatch, secondsToTime and fields e.etag and e.lastModified when
updating the logic.
| // Register tags for this cache entry | ||
| if ti != nil && len(entryTags) > 0 { | ||
| ti.add(key, entryTags) | ||
| } |
There was a problem hiding this comment.
Refresh tag index when re-caching a key to avoid stale tag associations.
ti.add only appends new tags; if a key is re-cached with a different tag set (e.g., ResponseTags changes), old tags remain and future invalidations can evict the wrong entries. Clear existing mappings for the key before adding new tags.
🔁 Suggested update
- if ti != nil && len(entryTags) > 0 {
- ti.add(key, entryTags)
- }
+ if ti != nil {
+ ti.remove(key)
+ if len(entryTags) > 0 {
+ ti.add(key, entryTags)
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Register tags for this cache entry | |
| if ti != nil && len(entryTags) > 0 { | |
| ti.add(key, entryTags) | |
| } | |
| // Register tags for this cache entry | |
| if ti != nil { | |
| ti.remove(key) | |
| if len(entryTags) > 0 { | |
| ti.add(key, entryTags) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@middleware/cache/cache.go` around lines 1129 - 1132, When registering tags
for a cached key (where the code currently calls ti.add(key, entryTags)), first
clear any existing tag mappings for that key to avoid stale associations; update
the logic around ti.add so it either calls a removal method (e.g.,
ti.remove(key) or ti.deleteKeyTags(key)) before ti.add, or use/implement a
single replace-style method (e.g., ti.replace(key, entryTags) or ti.setTags(key,
entryTags)) that atomically clears old tags and adds the new set; ensure this
change is applied where key and entryTags (and ResponseTags) are used so
re-caching replaces prior tag mappings instead of appending.
| | Hook | When it runs | Signature | | ||
| |---|---|---| | ||
| | `Tags` | Before the handler, using only request data | `func(c fiber.Ctx) []string` | | ||
| | `ResponseTags` | After the handler, with access to the response body | `func(c fiber.Ctx, body []byte) []string` | |
There was a problem hiding this comment.
Markdown table spacing fails MD060.
markdownlint reports missing spaces around table pipes in multiple tables. Please reformat or run the markdown linter to normalize spacing.
As per coding guidelines: **/*.md: Run make markdown to lint all Markdown files when modifying code.
Also applies to: 47-51, 60-63, 97-101, 284-292, 320-327
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 21-21: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 21-21: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 21-21: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 21-21: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 21-21: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 21-21: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
🤖 Prompt for AI Agents
In `@middleware/cache/README.md` around lines 20 - 23, Several Markdown tables in
middleware/cache/README.md (e.g., the table with headers "Hook", "When it runs",
"Signature" and rows for `Tags` and `ResponseTags`) have incorrect spacing
around pipe characters causing MD060; update all affected tables (lines
referenced in the review: around rows for `Tags`/`ResponseTags` and the other
ranges) to include single spaces before and after each pipe in every row and
header so they conform to markdownlint table spacing rules, then run the
repository lint target (`make markdown`) to normalize formatting and ensure no
remaining MD060 violations.
| ``` | ||
| Entry "k1" tagged ["product:1", "region:us"] | ||
|
|
||
| InvalidateTags(c, "region:us") | ||
| → k1 is evicted from the cache | ||
| → forward index for "region:us" is cleared | ||
| → reverse index for k1 retains ["product:1"] | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
MD040 flags fenced blocks without a language; use text/go where appropriate.
Also applies to: 172-174, 254-256
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 145-145: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@middleware/cache/README.md` around lines 145 - 152, Several fenced code
blocks in the README lack language identifiers; update each triple-backtick
fence to include a language (use ```text for plain text snippets and ```go for
Go code examples) so MD040 is satisfied—specifically change the block containing
the sample starting with Entry "k1" tagged ["product:1", "region:us"] to ```text
and likewise update the other similar blocks referenced (the snippets around the
same example and the blocks near the other occurrences) to the appropriate
language tag.
|
same as #4038 |
3 participants
Summary
Test plan
go test -race ./middleware/cache/passes cleanInvalidateTagsworks end-to-enddistributedTagStoreinstances sharing one storage backend; invalidation on one finds keys written by the otherhas()is O(1) across index sizes; distributedhas()matches in-memory cost; invalidation scales linearly with keys per tag🤖 Generated with Claude Code