diff --git a/e2e-tests/directory.s3.test.ts b/e2e-tests/directory.s3.test.ts index 22eb76e5..0c162882 100644 --- a/e2e-tests/directory.s3.test.ts +++ b/e2e-tests/directory.s3.test.ts @@ -72,6 +72,10 @@ for (const path of ['/dist/', '/docs/']) { await res.text(); expect(res.status).toBe(200); + // Directory listings change as files are added, so they're cached mutably. + expect(res.headers.get('cache-control')).toStrictEqual( + CACHE_HEADERS.mutable + ); }); } diff --git a/e2e-tests/fallback.test.ts b/e2e-tests/fallback.test.ts index a14290c4..c6a0da2d 100644 --- a/e2e-tests/fallback.test.ts +++ b/e2e-tests/fallback.test.ts @@ -61,7 +61,7 @@ test('grabs file from fallback server if r2 request fails', async () => { ); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toStrictEqual(CACHE_HEADERS.success); + expect(res.headers.get('cache-control')).toStrictEqual(CACHE_HEADERS.mutable); expect(originCalled).toBeTruthy(); expect(await res.text()).toStrictEqual(originResponse); }); diff --git a/e2e-tests/file.test.ts b/e2e-tests/file.test.ts index 8d446a22..29ef9fcb 100644 --- a/e2e-tests/file.test.ts +++ b/e2e-tests/file.test.ts @@ -1,5 +1,5 @@ import { env, createExecutionContext } from 'cloudflare:test'; -import { test, beforeAll, expect } from 'vitest'; +import { test, beforeAll, afterEach, expect, vi } from 'vitest'; import { populateR2WithDevBucket } from './util'; import worker from '../src/worker'; import type { Env } from '../src/env'; @@ -16,6 +16,86 @@ beforeAll(async () => { await populateR2WithDevBucket(); }); +afterEach(() => { + vi.restoreAllMocks(); +}); + +test('GET a versioned asset is cached immutably', async () => { + const ctx = createExecutionContext(); + + const res = await worker.fetch( + new Request('https://localhost/dist/v20.0.0/SHASUMS256.txt'), + mockedEnv, + ctx + ); + + // Consume the body promise + await res.text(); + + expect(res.status).toBe(200); + expect(res.headers.get('cache-control')).toStrictEqual( + CACHE_HEADERS.immutable + ); +}); + +test('HEAD a versioned asset is cached immutably', async () => { + const ctx = createExecutionContext(); + + const res = await worker.fetch( + new Request('https://localhost/dist/v20.0.0/SHASUMS256.txt', { + method: 'HEAD', + }), + mockedEnv, + ctx + ); + + // Consume the body promise + await res.text(); + + expect(res.status).toBe(200); + expect(res.headers.get('cache-control')).toStrictEqual( + CACHE_HEADERS.immutable + ); +}); + +test('GET an invalid object name returns 400 with the failure cache policy', async () => { + vi.spyOn(env.R2_BUCKET, 'get').mockImplementation(() => { + // R2 error 10020: object name not valid + throw new Error('10020: The specified key does not exist.'); + }); + + const ctx = createExecutionContext(); + + const res = await worker.fetch( + new Request('https://localhost/dist/v20.0.0/SHASUMS256.txt'), + mockedEnv, + ctx + ); + + expect(res.status).toBe(400); + expect(res.headers.get('cache-control')).toStrictEqual(CACHE_HEADERS.failure); +}); + +test('GET an out-of-bounds range returns 416 with the failure cache policy', async () => { + vi.spyOn(env.R2_BUCKET, 'get').mockImplementation(() => { + // R2 error 10039: range not satisfiable + throw new Error('10039: The requested range is not satisfiable.'); + }); + + const ctx = createExecutionContext(); + + const res = await worker.fetch( + new Request('https://localhost/dist/v20.0.0/SHASUMS256.txt', { + headers: { range: 'bytes=999999-1000000' }, + }), + mockedEnv, + ctx + ); + + expect(res.status).toBe(416); + expect(res.headers.get('cache-control')).toStrictEqual(CACHE_HEADERS.failure); +}); + test('GET `/dist/index.json` returns 200', async () => { const ctx = createExecutionContext(); @@ -29,7 +109,7 @@ test('GET `/dist/index.json` returns 200', async () => { await res.text(); expect(res.status).toBe(200); - expect(res.headers.get('cache-control')).toStrictEqual(CACHE_HEADERS.success); + expect(res.headers.get('cache-control')).toStrictEqual(CACHE_HEADERS.mutable); }); test('GET `/dist/asd123.json` returns 404', async () => { @@ -223,7 +303,7 @@ test('`if-match` header', async () => { expect(res.status).toBe(200); expect(res.headers.get('cache-control')).toStrictEqual( - CACHE_HEADERS.success + CACHE_HEADERS.mutable ); } }); diff --git a/src/constants/cache.ts b/src/constants/cache.ts index 7a326c78..17e4fe9b 100644 --- a/src/constants/cache.ts +++ b/src/constants/cache.ts @@ -1,4 +1,5 @@ export const CACHE_HEADERS = { - success: 'public, max-age=3600, s-maxage=14400', + immutable: 'public, immutable, max-age=31536000, s-maxage=31536000', + mutable: 'public, max-age=86400, s-maxage=86400, must-revalidate', failure: 'private, no-cache, no-store, max-age=0, must-revalidate', }; diff --git a/src/middleware/originMiddleware.ts b/src/middleware/originMiddleware.ts index 1d09803d..5e4f0df3 100644 --- a/src/middleware/originMiddleware.ts +++ b/src/middleware/originMiddleware.ts @@ -3,6 +3,8 @@ import { CACHE_HEADERS } from '../constants/cache'; import type { Context } from '../context'; import type { Request } from '../routes/request'; import { isDirectoryPath } from '../utils/path'; +import { cacheControlFor } from '../utils/cache'; +import { getOriginalUrl } from '../utils/request'; import type { Middleware } from './middleware'; /** @@ -19,6 +21,8 @@ export class OriginMiddleware implements Middleware { message: 'hit', }); + const originPathname = getOriginalUrl(request).pathname; + const res = await fetch( `${ctx.env.ORIGIN_HOST}${request.urlObj.pathname}`, { @@ -53,9 +57,9 @@ export class OriginMiddleware implements Middleware { // Don't cache this response on the client if it's a directory listing, // since our listing response might end up differently from nginx's at // some point - 'cache-control': isDirectoryPath(request.urlObj.pathname) + 'cache-control': isDirectoryPath(originPathname) ? CACHE_HEADERS.failure - : CACHE_HEADERS.success, + : cacheControlFor(originPathname, res.status), }, }); } diff --git a/src/middleware/r2Middleware.ts b/src/middleware/r2Middleware.ts index 1b81912c..bc84a2d2 100644 --- a/src/middleware/r2Middleware.ts +++ b/src/middleware/r2Middleware.ts @@ -2,15 +2,16 @@ import * as Sentry from '@sentry/cloudflare'; import { CACHE_HEADERS } from '../constants/cache'; import docsDirectory from '../constants/docsDirectory.json' assert { type: 'json' }; import type { Context } from '../context'; -import type { GetFileResult } from '../providers/provider'; +import type { GetFileResult, HeadFileResult } from '../providers/provider'; import { R2Provider } from '../providers/r2Provider'; import responses from '../responses'; import { hasTrailingSlash, isDirectoryPath } from '../utils/path'; +import { cacheControlFor } from '../utils/cache'; import type { Middleware } from './middleware'; import latestVersions from '../constants/latestVersions.json' assert { type: 'json' }; import type { Request } from '../routes/request'; import { renderDirectoryListing } from '../utils/directoryListing'; -import { parseConditionalHeaders } from '../utils/request'; +import { getOriginalUrl, parseConditionalHeaders } from '../utils/request'; import { once } from '../utils/memo'; const getProvider = once((ctx: Context) => new R2Provider({ ctx })); @@ -41,7 +42,7 @@ async function handleDirectory( ): Promise { if (!hasTrailingSlash(request.urlObj.pathname)) { // We always want directory listing requests to have a trailing slash - const url = request.unsubstitutedUrl ?? request.urlObj; + const url = getOriginalUrl(request); return Response.redirect(`${url}/`, 301); } @@ -58,21 +59,29 @@ async function handleDirectory( let responseBody; if (request.method === 'GET') { - responseBody = renderDirectoryListing( - request.unsubstitutedUrl ?? request.urlObj, - result - ); + responseBody = renderDirectoryListing(getOriginalUrl(request), result); } return new Response(responseBody, { headers: { 'last-modified': result.lastModified.toUTCString(), 'content-type': 'text/html', - 'cache-control': CACHE_HEADERS.success, + // Directory listings change as files are added. + 'cache-control': CACHE_HEADERS.mutable, }, }); } +function setCacheControl( + result: GetFileResult | HeadFileResult, + request: Request +): void { + result.httpHeaders['cache-control'] = cacheControlFor( + getOriginalUrl(request).pathname, + result.httpStatusCode + ); +} + function handleFile( request: Request, r2Path: string, @@ -99,6 +108,8 @@ async function headFile( return responses.fileNotFound(request.method); } + setCacheControl(result, request); + return new Response(undefined, { status: result.httpStatusCode, headers: result.httpHeaders, @@ -121,10 +132,16 @@ async function getFile( if (err instanceof Error) { if (err.message.includes('10020')) { // Object name not valid, url probably has some weirdness in it - return new Response(undefined, { status: 400 }); + return new Response(undefined, { + status: 400, + headers: { 'cache-control': CACHE_HEADERS.failure }, + }); } else if (err.message.includes('10039')) { // Range not compatible, probably out of bounds - return new Response(undefined, { status: 416 }); + return new Response(undefined, { + status: 416, + headers: { 'cache-control': CACHE_HEADERS.failure }, + }); } } @@ -135,6 +152,8 @@ async function getFile( return responses.fileNotFound(request.method); } + setCacheControl(result, request); + return new Response(result.contents, { status: result.httpStatusCode, headers: result.httpHeaders, diff --git a/src/providers/r2Provider.ts b/src/providers/r2Provider.ts index 3b361207..764487a9 100644 --- a/src/providers/r2Provider.ts +++ b/src/providers/r2Provider.ts @@ -1,4 +1,3 @@ -import { CACHE_HEADERS } from '../constants/cache'; import { R2_RETRY_LIMIT } from '../../lib/limits.mjs'; import CACHED_DIRECTORIES from '../constants/cachedDirectories.json' assert { type: 'json' }; import { CONTENT_TYPE_OVERRIDES } from '../constants/contentTypeOverrides'; @@ -58,7 +57,7 @@ export class R2Provider implements Provider { return { httpStatusCode: 200, - httpHeaders: r2MetadataToHeaders(object, 200), + httpHeaders: r2MetadataToHeaders(object), }; } @@ -97,7 +96,7 @@ export class R2Provider implements Provider { return { contents: hasBody ? (object as R2ObjectBody).body : undefined, httpStatusCode, - httpHeaders: r2MetadataToHeaders(object, httpStatusCode), + httpHeaders: r2MetadataToHeaders(object), }; } @@ -143,10 +142,7 @@ export class R2Provider implements Provider { } } -function r2MetadataToHeaders( - object: R2Object, - httpStatusCode: number -): HttpResponseHeaders { +function r2MetadataToHeaders(object: R2Object): HttpResponseHeaders { const { httpMetadata } = object; const fileExtension = object.key.substring(object.key.lastIndexOf('.') + 1); @@ -164,8 +160,9 @@ function r2MetadataToHeaders( 'accept-range': 'bytes', // https://github.com/nodejs/build/blob/e3df25d6a23f033db317a53ab1e904c953ba1f00/ansible/www-standalone/resources/config/nodejs.org?plain=1#L194-L196 'access-control-allow-origin': object.key.endsWith('.json') ? '*' : '', - 'cache-control': - httpStatusCode === 200 ? CACHE_HEADERS.success : CACHE_HEADERS.failure, + // Set by R2Middleware, which has access to the original request URL needed + // to decide between immutable/mutable/failure cache policies. + 'cache-control': '', expires: httpMetadata?.cacheExpiry?.toUTCString() ?? '', 'last-modified': getLastModified(object), 'content-language': httpMetadata?.contentLanguage ?? '', diff --git a/src/utils/cache.test.ts b/src/utils/cache.test.ts new file mode 100644 index 00000000..2c80959a --- /dev/null +++ b/src/utils/cache.test.ts @@ -0,0 +1,67 @@ +import { describe, test, expect } from 'vitest'; +import { isImmutablePath, cacheControlFor } from './cache'; +import { CACHE_HEADERS } from '../constants/cache'; + +describe('isImmutablePath', () => { + test.each([ + '/dist/v20.0.0/node-v20.0.0-linux-x64.tar.gz', + '/download/release/v20.0.0/node-v20.0.0.pkg', + '/docs/v18.0.0/api/fs.html', + '/metrics/processed/total-csv.csv', + ])('returns true for immutable asset `%s`', path => { + expect(isImmutablePath(path)).toEqual(true); + }); + + test.each([ + // Directory listings + '/dist/', + '/dist/v20.0.0/', + // Index/metadata files + '/dist/index.json', + '/dist/index.tab', + '/llms.txt', + '/node-config-schema.json', + // `/api/*` always serves the latest version's docs + '/api', + '/api/fs.html', + // `latest` aliases point at moving content + '/dist/latest/node-v20.0.0-linux-x64.tar.gz', + '/docs/latest/api/fs.html', + '/download/release/latest/win-x64/node_pdb.7z', + '/dist/latest-v20.x/node-v20.0.0.tar.gz', + // Legacy `latest-v0.X.x` aliases + '/dist/latest-v0.10.x/node-v0.10.48.tar.gz', + '/dist/latest-v0.12.x/node-v0.12.18.tar.gz', + // Codename aliases + '/dist/latest-argon/node-v4.9.1.tar.gz', + '/download/release/latest-iron/win-x64/node_pdb.7z', + '/docs/latest-hydrogen/api/fs.html', + // `node-latest.tar.gz` is a moving alias too + '/dist/node-latest.tar.gz', + ])('returns false for mutable path `%s`', path => { + expect(isImmutablePath(path)).toEqual(false); + }); +}); + +describe('cacheControlFor', () => { + test('immutable asset with 200 gets the immutable policy', () => { + expect( + cacheControlFor('/dist/v20.0.0/node-v20.0.0-linux-x64.tar.gz', 200) + ).toEqual(CACHE_HEADERS.immutable); + }); + + test('mutable resource with 200 gets the mutable policy', () => { + expect(cacheControlFor('/dist/index.json', 200)).toEqual( + CACHE_HEADERS.mutable + ); + }); + + test.each([206, 304, 412, 404, 500])( + 'status %i always gets the failure policy', + status => { + expect( + cacheControlFor('/dist/v20.0.0/node-v20.0.0.tar.gz', status) + ).toEqual(CACHE_HEADERS.failure); + } + ); +}); diff --git a/src/utils/cache.ts b/src/utils/cache.ts new file mode 100644 index 00000000..e7670727 --- /dev/null +++ b/src/utils/cache.ts @@ -0,0 +1,75 @@ +import { CACHE_HEADERS } from '../constants/cache'; +import latestVersions from '../constants/latestVersions.json' assert { type: 'json' }; +import { isDirectoryPath } from './path'; + +// Files whose contents change without a version/URL change. +const MUTABLE_FILENAMES = new Set([ + 'index.json', + 'index.tab', + 'llms.txt', + 'node-config-schema.json', +]); + +// Moving aliases (`latest`, `latest-v20.x`, `latest-argon`, `node-latest.tar.gz`, +// …). Each key of `latestVersions` is a path segment that points at a different +// release over time, so anything served under one must not be cached immutably. +// Sourcing this from `latestVersions` keeps it in sync with the routes +// registered in `routes/index.ts`. +const MOVING_ALIASES = new Set(Object.keys(latestVersions)); + +/** + * Decides whether a response can be cached immutably (effectively forever) based + * on the ORIGINAL (unsubstituted) request pathname. + * + * Most content we serve is immutable: a published release asset under a concrete + * version never changes. The exceptions are resources whose contents change + * while their URL stays the same: + * - directory listings (a new file can appear at any time) + * - the `/api/*` route (always serves the current `latest` version's docs) + * - `latest`/`latest-vXX.x`/`latest-` aliases (point at a different + * release over time) + * - index/metadata files like `index.json` + * + * @param pathname Original request pathname (use `unsubstitutedUrl` if present) + */ +export function isImmutablePath(pathname: string): boolean { + // Directory listings change as files are added. + if (isDirectoryPath(pathname)) { + return false; + } + + // `/api/*` always serves the current `latest` version's docs. + if (pathname === '/api' || pathname.startsWith('/api/')) { + return false; + } + + // `/dist/latest/`, `/docs/latest-v18.x/`, `/dist/latest-argon/`, etc. are + // moving aliases. Any path segment matching a known alias is mutable. + if (pathname.split('/').some(segment => MOVING_ALIASES.has(segment))) { + return false; + } + + const filename = pathname.slice(pathname.lastIndexOf('/') + 1); + if (MUTABLE_FILENAMES.has(filename)) { + return false; + } + + return true; +} + +/** + * Single source of truth for the `cache-control` header on a response. + * + * Only fully-successful (200) responses are given a long-lived cache policy. + * Conditional/range responses (206/304/412) and errors keep the `failure` + * policy so they're always re-validated. + */ +export function cacheControlFor(pathname: string, statusCode: number): string { + if (statusCode !== 200) { + return CACHE_HEADERS.failure; + } + + return isImmutablePath(pathname) + ? CACHE_HEADERS.immutable + : CACHE_HEADERS.mutable; +} diff --git a/src/utils/request.test.ts b/src/utils/request.test.ts index 057db40d..0253ad95 100644 --- a/src/utils/request.test.ts +++ b/src/utils/request.test.ts @@ -1,5 +1,26 @@ import { describe, expect, test } from 'vitest'; -import { parseConditionalHeaders, parseRangeHeader } from './request'; +import { + getOriginalUrl, + parseConditionalHeaders, + parseRangeHeader, +} from './request'; + +describe('getOriginalUrl', () => { + test('returns the unsubstituted url when an alias was substituted', () => { + const unsubstitutedUrl = new URL('https://localhost/dist/latest/'); + const urlObj = new URL('https://localhost/dist/v20.0.0/'); + + expect(getOriginalUrl({ unsubstitutedUrl, urlObj })).toBe(unsubstitutedUrl); + }); + + test('falls back to urlObj when no substitution happened', () => { + const urlObj = new URL('https://localhost/dist/v20.0.0/'); + + expect(getOriginalUrl({ unsubstitutedUrl: undefined, urlObj })).toBe( + urlObj + ); + }); +}); describe('parseRangeHeader', () => { test('`bytes=0-10`', () => { diff --git a/src/utils/request.ts b/src/utils/request.ts index d16b9b6b..35a3dfb2 100644 --- a/src/utils/request.ts +++ b/src/utils/request.ts @@ -1,4 +1,15 @@ import { parseHttpDate } from './http-date'; +import type { Request } from '../routes/request'; + +/** + * Returns the URL the client originally requested, before any alias + * substitution (e.g. `/dist/latest/`) was applied by SubtitutionMiddleware. + */ +export function getOriginalUrl( + request: Pick +): URL { + return request.unsubstitutedUrl ?? request.urlObj; +} /** * Etags will have quotes removed from them