fix: ignore Cloudflare __clfb cookie when deciding cache eligibility#1100
fix: ignore Cloudflare __clfb cookie when deciding cache eligibility#1100
Conversation
The Cloudflare Load Balancer sets a __clfb cookie for session affinity which does not affect page content. Allow caching when __clfb is the only Set-Cookie present.
Tagging OptionsShould a new tag be published when this PR is merged?
|
📝 WalkthroughWalkthroughModified middleware Set-Cookie header detection logic to only disable caching for non-"__cflb" cookies. Previously, any Set-Cookie header would mark pages as non-cacheable; now the "__cflb" cookie is ignored when making caching decisions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="runtime/middleware.ts">
<violation number="1" location="runtime/middleware.ts:434">
P1: The ignored cookie name is misspelled (`__clfb`), so Cloudflare's actual `__cflb` cookie is not excluded and cache eligibility remains blocked.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
runtime/middleware.ts
Outdated
| const hasSetCookie = getSetCookies(newHeaders).length > 0; | ||
| const setCookies = getSetCookies(newHeaders); | ||
| const hasSetCookie = setCookies.some( | ||
| (cookie: { name: string }) => cookie.name !== "__clfb", |
There was a problem hiding this comment.
P1: The ignored cookie name is misspelled (__clfb), so Cloudflare's actual __cflb cookie is not excluded and cache eligibility remains blocked.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At runtime/middleware.ts, line 434:
<comment>The ignored cookie name is misspelled (`__clfb`), so Cloudflare's actual `__cflb` cookie is not excluded and cache eligibility remains blocked.</comment>
<file context>
@@ -429,7 +429,10 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
- const hasSetCookie = getSetCookies(newHeaders).length > 0;
+ const setCookies = getSetCookies(newHeaders);
+ const hasSetCookie = setCookies.some(
+ (cookie: { name: string }) => cookie.name !== "__clfb",
+ );
const contentType = newHeaders.get("Content-Type") ?? "";
</file context>
| (cookie: { name: string }) => cookie.name !== "__clfb", | |
| (cookie: { name: string }) => cookie.name !== "__cflb", |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
runtime/middleware.ts (1)
432-435: Good fix; extract__cflbinto a constant and clarify the cache comment.This works as intended. To reduce future typo regressions (already happened in this PR) and keep behavior self-documenting, please avoid the string literal and update the nearby comment to reflect the exemption.
Proposed refactor
+const CLOUDFLARE_AFFINITY_COOKIE = "__cflb"; + const setCookies = getSetCookies(newHeaders); const hasSetCookie = setCookies.some( - (cookie: { name: string }) => cookie.name !== "__cflb", + (cookie: { name: string }) => cookie.name !== CLOUDFLARE_AFFINITY_COOKIE, ); if (hasSetCookie) { - // Set-cookie present: never cache (same behavior as main) + // Non-Cloudflare-affinity Set-Cookie present: never cache newHeaders.set("Cache-Control", "no-store, no-cache, must-revalidate"); }Also applies to: 441-443
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/middleware.ts` around lines 432 - 435, Extract the magic string "__cflb" into a clearly named constant (e.g., CF_LOAD_BALANCER_COOKIE or EXEMPT_COOKIE_NAME) and replace all occurrences in the getSetCookies/setCookies/hasSetCookie logic (references: getSetCookies, setCookies, hasSetCookie) with that constant to avoid typos; also update the nearby cache-related comment to explicitly state that this cookie is exempt from cache/Set-Cookie handling and why, and apply the same constant replacement to the other occurrence that mirrors this logic (the block around the alternate hasSetCookie check).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@runtime/middleware.ts`:
- Around line 432-435: Extract the magic string "__cflb" into a clearly named
constant (e.g., CF_LOAD_BALANCER_COOKIE or EXEMPT_COOKIE_NAME) and replace all
occurrences in the getSetCookies/setCookies/hasSetCookie logic (references:
getSetCookies, setCookies, hasSetCookie) with that constant to avoid typos; also
update the nearby cache-related comment to explicitly state that this cookie is
exempt from cache/Set-Cookie handling and why, and apply the same constant
replacement to the other occurrence that mirrors this logic (the block around
the alternate hasSetCookie check).
The Cloudflare Load Balancer sets a __clfb cookie for session affinity which does not affect page content. Allow caching when __clfb is the only Set-Cookie present.
Summary by cubic
Ignore the Cloudflare __cflb session-affinity cookie when deciding if a response can be cached. This enables caching when __cflb is the only Set-Cookie and fixes a previous typo that used __clfb.
Written for commit 0f2f131. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Performance