fix: always set Vary: Accept on HTML even when the Link header is disabled#73
Open
abhay-codes07 wants to merge 1 commit into
Open
Conversation
…abled Several adapters gated the Vary: Accept header on the Link-header feature flag, so setting enableLinkHeader/injectLinkHeader to false left negotiable HTML responses with no Vary header. A shared cache keyed on URL alone could then serve HTML to a markdown client or the reverse. Per content-negotiation.md section 3, Vary: Accept is required on every response whose representation depends on Accept, independent of the Link header. cloudflare, netlify, vercel, nextjs and sveltekit now always emit Vary: Accept on negotiable HTML and gate only the Link header, matching the deno and fastly adapters.
|
@abhay-codes07 is attempting to deploy a commit to the Dodo Payments Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a cache-correctness bug where disabling Link-header injection also (incorrectly) removed Vary: Accept from negotiable HTML responses, potentially causing shared caches to serve HTML to markdown clients (and vice versa). The change decouples Vary: Accept from Link-header injection across several adapters and adds regression tests to lock the behavior in.
Changes:
- Decouple
Vary: Acceptfrom Link-header injection in the cloudflare, netlify, vercel, nextjs, and sveltekit adapters soVary: Acceptis always emitted for negotiable HTML. - Add regression tests in each adapter to verify: with Link disabled, HTML responses still include
Vary: Acceptand do not includeLink. - Add a changeset for patch releases of the affected adapter packages.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vercel/src/middleware.ts | Always appends Vary: Accept for negotiable HTML; gates only Link on enableLinkHeader. |
| packages/vercel/test/middleware.test.ts | Adds regression test for Vary: Accept when Link header is disabled. |
| packages/sveltekit/src/handle.ts | Ensures Vary: Accept is set even when injectLinkHeader is false (while keeping Link injection optional). |
| packages/sveltekit/test/handle.test.ts | Adds regression test for Vary: Accept when Link header is disabled. |
| packages/nextjs/src/middleware.ts | Moves appendVaryAccept outside the Link-header injection block so it always runs on the negotiable HTML branch. |
| packages/nextjs/test/middleware.test.ts | Adds regression test for Vary: Accept when Link header is disabled. |
| packages/netlify/src/worker.ts | Ensures Vary: Accept is set independently of enableLinkHeader; Link injection remains gated. |
| packages/netlify/test/worker.test.ts | Adds regression test for Vary: Accept when Link header is disabled. |
| packages/cloudflare/src/worker.ts | Ensures Vary: Accept is set independently of enableLinkHeader; Link injection remains gated. |
| packages/cloudflare/test/worker.test.ts | Adds regression test for Vary: Accept when Link header is disabled. |
| .changeset/vary-accept-independent-of-link-header.md | Patch changeset documenting the behavior fix across the five adapters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
290
to
294
| const ct = upstreamResponse.headers.get("content-type"); | ||
| // Passthrough responses (e.g. NextResponse.next()) have no content-type yet — | ||
| // always inject. For concrete responses, only inject on text/html. | ||
| if (!ct || ct.includes("text/html")) { | ||
| const mdPath = toMarkdownPath(pathname); |
| enableLinkHeader && | ||
| !shouldSkip(pathname, skipPrefixes, skipExtensions) && | ||
| !pathname.endsWith(".md") && | ||
| originResponse.headers.get("content-type")?.includes("text/html") |
| enableLinkHeader && | ||
| !shouldSkip(pathname, skipPrefixes, skipExtensions) && | ||
| !pathname.endsWith(".md") && | ||
| upstreamResponse.headers.get("content-type")?.includes("text/html") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Turning off the Link header (
enableLinkHeader: false, orinjectLinkHeader: falseon nextjs/sveltekit) also droppedVary: Acceptfrom negotiable HTML responses, because the Vary logic was nested inside the Link-header block. That is a cache-correctness bug: a shared cache keyed on the URL can then serve HTML to a markdown client or the reverse. This decouples the two so Vary is always set.Closes #72.
Changes
Vary: Accepton a negotiable HTML response, and gate only theLinkheader on the feature flag. This matches the deno and fastly adapters, which already did it this way.Vary: Acceptand noLinkheader.Type
AEO_SPEC_VERSION)Verification
bun run buildpassesbun run testpassesbun run typecheckpassesdualmark verify(no examples touched)/spec/: ran sync-spec (no spec files touched)Changeset
cc @thepushkaraj @aagarwal1012. This follows the same pattern deno/fastly already use, so it should be low risk, but happy to adjust if you would rather I factor the shared Vary-append into a small core helper.