middleware/cache/cache.go (1)

595-604: Simplify redundant DisableVaryHeaders gating.

shouldStoreVaryManifest already includes !cfg.DisableVaryHeaders, so the repeated conjunctions on lines 596, 600, and 845 are redundant. Collapsing them improves readability and keeps the invariant in one place.

♻️ Proposed simplification

-		shouldStoreVaryManifest := !cfg.DisableVaryHeaders && len(varyNames) > 0
-		if !cfg.DisableVaryHeaders && len(varyNames) > 0 {
+		shouldStoreVaryManifest := !cfg.DisableVaryHeaders && len(varyNames) > 0
+		if shouldStoreVaryManifest {
 			if key == baseKey {
 				key += buildVaryKey(varyNames, &c.Request().Header)
 			}
 		} else if !cfg.DisableVaryHeaders && hasVaryManifest {

And at line 845:

-		if !cfg.DisableVaryHeaders && shouldStoreVaryManifest {
+		if shouldStoreVaryManifest {
 			if err := storeVaryManifest(reqCtx, manager, manifestKey, varyNames, storageExpiration); err != nil {
 				return err
 			}
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@middleware/cache/cache.go` around lines 595 - 604, The gating on
cfg.DisableVaryHeaders is redundant because shouldStoreVaryManifest already
embeds !cfg.DisableVaryHeaders; update the conditions to use
shouldStoreVaryManifest instead of repeating !cfg.DisableVaryHeaders &&
len(varyNames) > 0 (affecting the block that mutates key when key == baseKey and
the else-if that checks hasVaryManifest). Replace those repeated conjunctions
with shouldStoreVaryManifest where appropriate (referencing variables:
shouldStoreVaryManifest, varyNames, key, baseKey, buildVaryKey) and keep the
stale-manifest deletion branch to only run when !shouldStoreVaryManifest &&
hasVaryManifest (using manager.del, manifestKey, maskKey) so logic and behavior
remain unchanged but the DisableVaryHeaders check is centralized.

middleware/cache/config.go (1)

129-164: Closure in configDefault captures a pointer to the local cfg, not the returned copy.

cfg := config[0]
// ...
if cfg.KeyGenerator == nil {
    cfg.KeyGenerator = func(c fiber.Ctx) string {
        return defaultKeyGenerator(c, &cfg) // &cfg == address of the local, heap-escaped
    }
}
return cfg // struct copy; closure still references the original

In practice this is fine because callers don't mutate the returned Config after configDefault, and at return time the local and the copy are value-equal. But it is a subtle footgun: if a caller ever does cfg := configDefault(...); cfg.KeyHeaders = ..., the default key generator will keep using the pre-mutation values. Consider either documenting this or snapshotting KeyHeaders/KeyCookies explicitly in the closure (e.g. keyHeaders := cfg.KeyHeaders; keyCookies := cfg.KeyCookies; ... defaultKeyGenerator(c, &snapshotCfg)), so the generator is clearly bound to the config observed at construction time.

Non-blocking — optional hardening.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@middleware/cache/config.go` around lines 129 - 164, configDefault's closure
captures the local cfg variable so subsequent mutations to the returned Config
won't be reflected as expected; fix by snapshotting the config before creating
the closure: create a local copy (e.g. snapshot := cfg) and set KeyGenerator to
use &snapshot (or capture individual slices like keyHeaders := cfg.KeyHeaders,
keyCookies := cfg.KeyCookies and pass those) instead of &cfg, and apply this
change in both the "no config provided" branch and the "override default config"
branch where KeyGenerator is assigned, ensuring the closure binds to the
immutable snapshot observed at construction time.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

middleware/cache/config.go (1)

167-176: ⚠️ Potential issue | 🟡 Minor

Caller's Methods slice is still mutated in place.

The utilsstrings.ToUpper swap was applied, but the defensive copy wasn't. Since cfg := config[0] at line 153 is a shallow copy, cfg.Methods shares the backing array with the caller's slice, so the loop at lines 173–175 writes back into the caller's storage. A caller passing []string{"get","head"} will observe it mutated to ["GET","HEAD"] after cache.New(...).

🛠️ Proposed fix

 	if cfg.Methods == nil {
 		cfg.Methods = ConfigDefault.Methods
 	} else {
 		// Normalize method names to uppercase (HTTP methods are case-sensitive
-		// and c.Method() returns uppercase, e.g. "GET" not "get")
-		for i, m := range cfg.Methods {
-			cfg.Methods[i] = utilsstrings.ToUpper(m)
-		}
+		// and c.Method() returns uppercase, e.g. "GET" not "get").
+		// Copy first to avoid mutating the caller's slice.
+		normalized := make([]string, len(cfg.Methods))
+		for i, m := range cfg.Methods {
+			normalized[i] = utilsstrings.ToUpper(m)
+		}
+		cfg.Methods = normalized
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@middleware/cache/config.go` around lines 167 - 176, The Methods slice is
being mutated in place (cfg.Methods) when normalizing to uppercase, which also
mutates the caller's slice because cfg := config[0] is a shallow copy; to fix,
make a defensive copy of cfg.Methods before normalizing (allocate a new
[]string, copy elements, assign back to cfg.Methods), preserve the special-case
nil => ConfigDefault.Methods behavior, and then run utilsstrings.ToUpper on the
copied slice (reference symbols: cfg.Methods, ConfigDefault.Methods,
utilsstrings.ToUpper, config[0]).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes