From 60885d2725de5d1c465a4405fdf86f1808c8434c Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Tue, 15 Oct 2024 15:23:47 +0100 Subject: [PATCH] feat: cache 404s indefinitely for bot probes (#2668) * feat: cache 404s indefinitely for bot probes * test: update unit tests * chore: refactor to clean up function signature with object parameterisation --- src/run/handlers/server.ts | 2 +- src/run/headers.test.ts | 188 +++++++++++++++------------ src/run/headers.ts | 9 +- tests/integration/simple-app.test.ts | 7 + 4 files changed, 121 insertions(+), 85 deletions(-) diff --git a/src/run/handlers/server.ts b/src/run/handlers/server.ts index d7dd4c5c0a..cf0e8c63dd 100644 --- a/src/run/handlers/server.ts +++ b/src/run/handlers/server.ts @@ -112,7 +112,7 @@ export default async (request: Request, context: FutureContext) => { await adjustDateHeader({ headers: response.headers, request, span, tracer, requestContext }) - setCacheControlHeaders(response.headers, request, requestContext) + setCacheControlHeaders(response, request, requestContext) setCacheTagsHeaders(response.headers, requestContext) setVaryHeaders(response.headers, request, nextConfig) setCacheStatusHeader(response.headers) diff --git a/src/run/headers.test.ts b/src/run/headers.test.ts index 6e09f8958a..16f99d50dc 100644 --- a/src/run/headers.test.ts +++ b/src/run/headers.test.ts @@ -199,43 +199,43 @@ describe('headers', () => { const givenHeaders = { 'cdn-cache-control': 'public, max-age=0, must-revalidate', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') const ctx: RequestContext = { ...createRequestContext(), routeHandlerRevalidate: false } - setCacheControlHeaders(headers, request, ctx) + setCacheControlHeaders(response, request, ctx) - expect(headers.set).toHaveBeenCalledTimes(0) + expect(response.headers.set).toHaveBeenCalledTimes(0) }) test('should not set any headers if "netlify-cdn-cache-control" is present', () => { const givenHeaders = { 'netlify-cdn-cache-control': 'public, max-age=0, must-revalidate', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') const ctx: RequestContext = { ...createRequestContext(), routeHandlerRevalidate: false } - setCacheControlHeaders(headers, request, ctx) + setCacheControlHeaders(response, request, ctx) - expect(headers.set).toHaveBeenCalledTimes(0) + expect(response.headers.set).toHaveBeenCalledTimes(0) }) test('should mark content as stale if "{netlify-,}cdn-cache-control" is not present and "x-nextjs-cache" is "STALE" (GET)', () => { const givenHeaders = { 'x-nextjs-cache': 'STALE', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') const ctx: RequestContext = { ...createRequestContext(), routeHandlerRevalidate: false } - setCacheControlHeaders(headers, request, ctx) + setCacheControlHeaders(response, request, ctx) - expect(headers.set).toHaveBeenCalledTimes(1) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenCalledTimes(1) + expect(response.headers.set).toHaveBeenNthCalledWith( 1, 'netlify-cdn-cache-control', 'public, max-age=0, must-revalidate, durable', @@ -246,15 +246,15 @@ describe('headers', () => { const givenHeaders = { 'x-nextjs-cache': 'STALE', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') const ctx: RequestContext = { ...createRequestContext(), routeHandlerRevalidate: false } - setCacheControlHeaders(headers, request, ctx) + setCacheControlHeaders(response, request, ctx) - expect(headers.set).toHaveBeenCalledTimes(1) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenCalledTimes(1) + expect(response.headers.set).toHaveBeenNthCalledWith( 1, 'netlify-cdn-cache-control', 'public, max-age=0, must-revalidate, durable', @@ -262,15 +262,15 @@ describe('headers', () => { }) test('should set durable SWC=1yr with 1yr TTL if "{netlify-,}cdn-cache-control" is not present and `revalidate` is `false` (HEAD)', () => { - const headers = new Headers() const request = new Request(defaultUrl, { method: 'HEAD' }) - vi.spyOn(headers, 'set') + const response = new Response() + vi.spyOn(response.headers, 'set') const ctx: RequestContext = { ...createRequestContext(), routeHandlerRevalidate: false } - setCacheControlHeaders(headers, request, ctx) + setCacheControlHeaders(response, request, ctx) - expect(headers.set).toHaveBeenCalledTimes(1) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenCalledTimes(1) + expect(response.headers.set).toHaveBeenNthCalledWith( 1, 'netlify-cdn-cache-control', 's-maxage=31536000, stale-while-revalidate=31536000, durable', @@ -278,15 +278,15 @@ describe('headers', () => { }) test('should set durable SWC=1yr with given TTL if "{netlify-,}cdn-cache-control" is not present and `revalidate` is a number (GET)', () => { - const headers = new Headers() const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response() + vi.spyOn(response.headers, 'set') const ctx: RequestContext = { ...createRequestContext(), routeHandlerRevalidate: 7200 } - setCacheControlHeaders(headers, request, ctx) + setCacheControlHeaders(response, request, ctx) - expect(headers.set).toHaveBeenCalledTimes(1) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenCalledTimes(1) + expect(response.headers.set).toHaveBeenNthCalledWith( 1, 'netlify-cdn-cache-control', 's-maxage=7200, stale-while-revalidate=31536000, durable', @@ -294,15 +294,15 @@ describe('headers', () => { }) test('should set durable SWC=1yr with 1yr TTL if "{netlify-,}cdn-cache-control" is not present and `revalidate` is a number (HEAD)', () => { - const headers = new Headers() const request = new Request(defaultUrl, { method: 'HEAD' }) - vi.spyOn(headers, 'set') + const response = new Response() + vi.spyOn(response.headers, 'set') const ctx: RequestContext = { ...createRequestContext(), routeHandlerRevalidate: 7200 } - setCacheControlHeaders(headers, request, ctx) + setCacheControlHeaders(response, request, ctx) - expect(headers.set).toHaveBeenCalledTimes(1) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenCalledTimes(1) + expect(response.headers.set).toHaveBeenNthCalledWith( 1, 'netlify-cdn-cache-control', 's-maxage=7200, stale-while-revalidate=31536000, durable', @@ -310,43 +310,65 @@ describe('headers', () => { }) test('should not set any headers on POST request', () => { - const headers = new Headers() const request = new Request(defaultUrl, { method: 'POST' }) - vi.spyOn(headers, 'set') + const response = new Response() + vi.spyOn(response.headers, 'set') const ctx: RequestContext = { ...createRequestContext(), routeHandlerRevalidate: false } - setCacheControlHeaders(headers, request, ctx) + setCacheControlHeaders(response, request, ctx) - expect(headers.set).toHaveBeenCalledTimes(0) + expect(response.headers.set).toHaveBeenCalledTimes(0) }) }) test('should not set any headers if "cache-control" is not set and "requestContext.usedFsRead" is not truthy', () => { - const headers = new Headers() const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response() + vi.spyOn(response.headers, 'set') - setCacheControlHeaders(headers, request, createRequestContext()) + setCacheControlHeaders(response, request, createRequestContext()) - expect(headers.set).toHaveBeenCalledTimes(0) + expect(response.headers.set).toHaveBeenCalledTimes(0) }) test('should set permanent, durable "netlify-cdn-cache-control" if "cache-control" is not set and "requestContext.usedFsRead" is truthy', () => { - const headers = new Headers() const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response() + vi.spyOn(response.headers, 'set') + + const requestContext = createRequestContext() + requestContext.usedFsRead = true + + setCacheControlHeaders(response, request, requestContext) + + expect(response.headers.set).toHaveBeenNthCalledWith( + 1, + 'cache-control', + 'public, max-age=0, must-revalidate', + ) + expect(response.headers.set).toHaveBeenNthCalledWith( + 2, + 'netlify-cdn-cache-control', + 'max-age=31536000, durable', + ) + }) + + test('should set permanent, durable "netlify-cdn-cache-control" if 404 response for URl ending in .php', () => { + const request = new Request(`${defaultUrl}/admin.php`) + const response = new Response(null, { status: 404 }) + vi.spyOn(response.headers, 'set') const requestContext = createRequestContext() requestContext.usedFsRead = true - setCacheControlHeaders(headers, request, requestContext, true) + setCacheControlHeaders(response, request, requestContext) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith( 1, 'cache-control', 'public, max-age=0, must-revalidate', ) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith( 2, 'netlify-cdn-cache-control', 'max-age=31536000, durable', @@ -358,13 +380,13 @@ describe('headers', () => { 'cache-control': 'public, max-age=0, must-revalidate', 'cdn-cache-control': 'public, max-age=0, must-revalidate', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') - setCacheControlHeaders(headers, request, createRequestContext()) + setCacheControlHeaders(response, request, createRequestContext()) - expect(headers.set).toHaveBeenCalledTimes(0) + expect(response.headers.set).toHaveBeenCalledTimes(0) }) test('should not set any headers if "cache-control" is set and "netlify-cdn-cache-control" is present', () => { @@ -372,31 +394,31 @@ describe('headers', () => { 'cache-control': 'public, max-age=0, must-revalidate', 'netlify-cdn-cache-control': 'public, max-age=0, must-revalidate', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') - setCacheControlHeaders(headers, request, createRequestContext()) + setCacheControlHeaders(response, request, createRequestContext()) - expect(headers.set).toHaveBeenCalledTimes(0) + expect(response.headers.set).toHaveBeenCalledTimes(0) }) test('should set expected headers if "cache-control" is set and "cdn-cache-control" and "netlify-cdn-cache-control" are not present (GET request)', () => { const givenHeaders = { 'cache-control': 'public, max-age=0, must-revalidate', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') - setCacheControlHeaders(headers, request, createRequestContext()) + setCacheControlHeaders(response, request, createRequestContext()) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith( 1, 'cache-control', 'public, max-age=0, must-revalidate', ) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith( 2, 'netlify-cdn-cache-control', 'public, max-age=0, must-revalidate, durable', @@ -407,18 +429,18 @@ describe('headers', () => { const givenHeaders = { 'cache-control': 'public, max-age=0, must-revalidate', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl, { method: 'HEAD' }) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') - setCacheControlHeaders(headers, request, createRequestContext()) + setCacheControlHeaders(response, request, createRequestContext()) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith( 1, 'cache-control', 'public, max-age=0, must-revalidate', ) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith( 2, 'netlify-cdn-cache-control', 'public, max-age=0, must-revalidate, durable', @@ -429,27 +451,27 @@ describe('headers', () => { const givenHeaders = { 'cache-control': 'public, max-age=0, must-revalidate', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl, { method: 'POST' }) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') - setCacheControlHeaders(headers, request, createRequestContext()) + setCacheControlHeaders(response, request, createRequestContext()) - expect(headers.set).toHaveBeenCalledTimes(0) + expect(response.headers.set).toHaveBeenCalledTimes(0) }) test('should remove "s-maxage" from "cache-control" header', () => { const givenHeaders = { 'cache-control': 'public, s-maxage=604800', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') - setCacheControlHeaders(headers, request, createRequestContext()) + setCacheControlHeaders(response, request, createRequestContext()) - expect(headers.set).toHaveBeenNthCalledWith(1, 'cache-control', 'public') - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith(1, 'cache-control', 'public') + expect(response.headers.set).toHaveBeenNthCalledWith( 2, 'netlify-cdn-cache-control', 'public, s-maxage=604800, durable', @@ -460,14 +482,14 @@ describe('headers', () => { const givenHeaders = { 'cache-control': 'max-age=604800, stale-while-revalidate=86400', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') - setCacheControlHeaders(headers, request, createRequestContext()) + setCacheControlHeaders(response, request, createRequestContext()) - expect(headers.set).toHaveBeenNthCalledWith(1, 'cache-control', 'max-age=604800') - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith(1, 'cache-control', 'max-age=604800') + expect(response.headers.set).toHaveBeenNthCalledWith( 2, 'netlify-cdn-cache-control', 'max-age=604800, stale-while-revalidate=86400, durable', @@ -478,18 +500,18 @@ describe('headers', () => { const givenHeaders = { 'cache-control': 's-maxage=604800, stale-while-revalidate=86400', } - const headers = new Headers(givenHeaders) const request = new Request(defaultUrl) - vi.spyOn(headers, 'set') + const response = new Response(null, { headers: givenHeaders }) + vi.spyOn(response.headers, 'set') - setCacheControlHeaders(headers, request, createRequestContext()) + setCacheControlHeaders(response, request, createRequestContext()) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith( 1, 'cache-control', 'public, max-age=0, must-revalidate', ) - expect(headers.set).toHaveBeenNthCalledWith( + expect(response.headers.set).toHaveBeenNthCalledWith( 2, 'netlify-cdn-cache-control', 's-maxage=604800, stale-while-revalidate=86400, durable', diff --git a/src/run/headers.ts b/src/run/headers.ts index 411edf0052..cfc1a91984 100644 --- a/src/run/headers.ts +++ b/src/run/headers.ts @@ -210,7 +210,7 @@ export const adjustDateHeader = async ({ * assume the user knows what they are doing if CDN cache controls are set */ export const setCacheControlHeaders = ( - headers: Headers, + { headers, status }: Response, request: Request, requestContext: RequestContext, ) => { @@ -231,6 +231,13 @@ export const setCacheControlHeaders = ( return } + if (status === 404 && request.url.endsWith('.php')) { + // temporary CDN Cache Control handling for bot probes on PHP files + // https://linear.app/netlify/issue/FRB-1344/prevent-excessive-ssr-invocations-due-to-404-routes + headers.set('cache-control', 'public, max-age=0, must-revalidate') + headers.set('netlify-cdn-cache-control', `max-age=31536000, durable`) + } + const cacheControl = headers.get('cache-control') if ( cacheControl !== null && diff --git a/tests/integration/simple-app.test.ts b/tests/integration/simple-app.test.ts index 3b17f7130c..5a04533104 100644 --- a/tests/integration/simple-app.test.ts +++ b/tests/integration/simple-app.test.ts @@ -158,6 +158,13 @@ test('stale-while-revalidate headers should be normalized to ) }) +test('404 responses for PHP pages should be cached indefinitely', async (ctx) => { + await createFixture('simple', ctx) + await runPlugin(ctx) + const index = await invokeFunction(ctx, { url: '/admin.php' }) + expect(index.headers?.['netlify-cdn-cache-control']).toContain('max-age=31536000, durable') +}) + test('handlers receive correct site domain', async (ctx) => { await createFixture('simple', ctx) await runPlugin(ctx)