diff --git a/.changeset/large-pants-press.md b/.changeset/large-pants-press.md new file mode 100644 index 000000000..cafd0b817 --- /dev/null +++ b/.changeset/large-pants-press.md @@ -0,0 +1,5 @@ +--- +"@opennextjs/aws": patch +--- + +Fix cache-control header set in middleware being overriden for ISR route diff --git a/packages/open-next/src/core/routing/util.ts b/packages/open-next/src/core/routing/util.ts index e1ce7d3a7..149adfded 100644 --- a/packages/open-next/src/core/routing/util.ts +++ b/packages/open-next/src/core/routing/util.ts @@ -356,6 +356,13 @@ export async function revalidateIfRequired( * @__PURE__ */ export function fixISRHeaders(headers: OutgoingHttpHeaders) { + const sMaxAgeRegex = /s-maxage=(\d+)/; + const match = headers[CommonHeaders.CACHE_CONTROL]?.match(sMaxAgeRegex); + const sMaxAge = match ? Number.parseInt(match[1]) : undefined; + // We only apply the fix if the cache-control header contains s-maxage + if (!sMaxAge) { + return; + } if (headers[CommonHeaders.NEXT_CACHE] === "REVALIDATED") { headers[CommonHeaders.CACHE_CONTROL] = "private, no-cache, no-store, max-age=0, must-revalidate"; @@ -363,18 +370,17 @@ export function fixISRHeaders(headers: OutgoingHttpHeaders) { } const _lastModified = globalThis.__openNextAls.getStore()?.lastModified ?? 0; if (headers[CommonHeaders.NEXT_CACHE] === "HIT" && _lastModified > 0) { - // calculate age - const age = Math.round((Date.now() - _lastModified) / 1000); - // extract s-maxage from cache-control - const regex = /s-maxage=(\d+)/; - const cacheControl = headers[CommonHeaders.CACHE_CONTROL]; - debug("cache-control", cacheControl, _lastModified, Date.now()); - if (typeof cacheControl !== "string") return; - const match = cacheControl.match(regex); - const sMaxAge = match ? Number.parseInt(match[1]) : undefined; + debug( + "cache-control", + headers[CommonHeaders.CACHE_CONTROL], + _lastModified, + Date.now(), + ); // 31536000 is the default s-maxage value for SSG pages if (sMaxAge && sMaxAge !== 31536000) { + // calculate age + const age = Math.round((Date.now() - _lastModified) / 1000); const remainingTtl = Math.max(sMaxAge - age, 1); headers[CommonHeaders.CACHE_CONTROL] = `s-maxage=${remainingTtl}, stale-while-revalidate=2592000`; diff --git a/packages/open-next/src/http/openNextResponse.ts b/packages/open-next/src/http/openNextResponse.ts index 3a8187950..ec6e8047b 100644 --- a/packages/open-next/src/http/openNextResponse.ts +++ b/packages/open-next/src/http/openNextResponse.ts @@ -22,6 +22,7 @@ export class OpenNextNodeResponse extends Transform implements ServerResponse { headers: OutgoingHttpHeaders = {}; headersSent = false; _chunks: Buffer[] = []; + headersAlreadyFixed = false; private _cookies: string[] = []; private responseStream?: Writable; @@ -69,7 +70,7 @@ export class OpenNextNodeResponse extends Transform implements ServerResponse { } constructor( - private fixHeaders: (headers: OutgoingHttpHeaders) => void, + private fixHeadersFn: (headers: OutgoingHttpHeaders) => void, private onEnd: (headers: OutgoingHttpHeaders) => Promise, private streamCreator?: StreamCreator, private initialHeaders?: OutgoingHttpHeaders, @@ -271,6 +272,14 @@ export class OpenNextNodeResponse extends Transform implements ServerResponse { * OpenNext specific method */ + fixHeaders(headers: OutgoingHttpHeaders) { + if (this.headersAlreadyFixed) { + return; + } + this.fixHeadersFn(headers); + this.headersAlreadyFixed = true; + } + getFixedHeaders(): OutgoingHttpHeaders { // Do we want to apply this on writeHead? this.fixHeaders(this.headers); diff --git a/packages/tests-e2e/tests/appRouter/isr.test.ts b/packages/tests-e2e/tests/appRouter/isr.test.ts index 3a9eabda9..2479f05eb 100644 --- a/packages/tests-e2e/tests/appRouter/isr.test.ts +++ b/packages/tests-e2e/tests/appRouter/isr.test.ts @@ -45,13 +45,26 @@ test("headers", async ({ page }) => { return response.status() === 200; }); await page.goto("/isr"); + let hasBeenStale = false; + let hasBeenHit = false; while (true) { const response = await responsePromise; const headers = response.headers(); + expect(headers["cache-control"]).toBe( + "max-age=10, stale-while-revalidate=999", + ); + const cacheHeader = + headers["x-nextjs-cache"] ?? headers["x-opennext-cache"]; + if (cacheHeader === "STALE") { + hasBeenStale = true; + } + if (cacheHeader === "HIT") { + hasBeenHit = true; + } // this was set in middleware - if (headers["cache-control"] === "max-age=10, stale-while-revalidate=999") { + if (hasBeenStale && hasBeenHit) { break; } await page.waitForTimeout(1000); diff --git a/packages/tests-unit/tests/core/routing/util.test.ts b/packages/tests-unit/tests/core/routing/util.test.ts index 0b3a6169b..e4ba8dbab 100644 --- a/packages/tests-unit/tests/core/routing/util.test.ts +++ b/packages/tests-unit/tests/core/routing/util.test.ts @@ -748,6 +748,7 @@ describe("fixISRHeaders", () => { it("should set cache-control directive to must-revalidate when x-nextjs-cache is REVALIDATED", () => { const headers: Record = { "x-nextjs-cache": "REVALIDATED", + "cache-control": "s-maxage=10, stale-while-revalidate=86400", }; fixISRHeaders(headers); @@ -771,6 +772,7 @@ describe("fixISRHeaders", () => { it("should set cache-control directive to stale-while-revalidate when x-nextjs-cache is STALE", () => { const headers: Record = { "x-nextjs-cache": "STALE", + "cache-control": "s-maxage=86400", // 1 day }; fixISRHeaders(headers); @@ -778,6 +780,25 @@ describe("fixISRHeaders", () => { "s-maxage=2, stale-while-revalidate=2592000", ); }); + + it("should not modify cache-control when cache-control is missing", () => { + const headers: Record = { + "x-nextjs-cache": "HIT", + }; + fixISRHeaders(headers); + + expect(headers["cache-control"]).toBeUndefined(); + }); + + it("should not modify cache-control when cache-control has no s-maxage", () => { + const headers: Record = { + "x-nextjs-cache": "HIT", + "cache-control": "private, max-age=0", + }; + fixISRHeaders(headers); + + expect(headers["cache-control"]).toBe("private, max-age=0"); + }); }); describe("invalidateCDNOnRequest", () => {