diff --git a/src/http/app.test.ts b/src/http/app.test.ts index 8bc23ca..c67bf50 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -451,6 +451,15 @@ test("every /public/ href the host renders is versioned and answers a year-long } }); +test("an error page rendered before the request has a context still versions its stylesheet", async () => { + const port = Number(new URL(base).port); + const res = await rawGet(port, "//", `localhost:${port}`); // `new URL("//", …)` throws before any context exists + assert.equal(res.status, 500); + const refs = publicRefs(res.body); + assert.ok(refs.length > 0); + for (const ref of refs) assert.match(ref, /\?v=[\w-]+$/, ref); +}); + test("the standalone error and consent views version their stylesheets", async () => { for (const view of ["403", "404", "500", "503", "error", "oauth-consent"]) { const html = await ejs.renderFile(join(viewsDir, `${view}.ejs`), { diff --git a/src/http/app.ts b/src/http/app.ts index 584bc57..a774d72 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -42,7 +42,7 @@ const rootDir = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); export interface AppOptions { appUrl?: string; // canonical public URL (config.appUrl); off-host GET/HEAD visitors are 308'd here. Omitted ⇒ no redirect auth?: VerifyOptions; // expected JWT issuer/audience + clock skew (config); used with jwks - cache?: boolean; // cache compiled EJS templates (config.cacheTemplates); off ⇒ edits show live + cache?: boolean; // cache compiled EJS templates and /public/ asset versions (config.cacheTemplates); off ⇒ edits show live csrfSecret?: string; // HMAC key for the double-submit CSRF token (config.csrfSecret); random if omitted denylist?: Denylist; // optional instant-revoke; the hot path rejects revoked subjects, admin writes record revokes hydra?: HydraAdmin; // Hydra admin client; with kratos enables the OAuth2 login challenge @@ -98,7 +98,7 @@ export function createApp(options: AppOptions = {}): Server { const anyResponseHooks = plugins.some((p) => p.hooks?.onResponse); const pluginsDir = options.pluginsDir ?? PLUGINS_DIR; const publicDir = options.publicDir ?? join(rootDir, "public"); - const assets = publicAssets(publicDir, pluginsDir, pluginIds); + const assets = publicAssets(publicDir, pluginsDir, pluginIds, { cache }); const viewsDir = options.viewsDir ?? join(rootDir, "views"); // Response security headers, fixed at boot (only HSTS depends on the https deployment signal). const secHeaderEntries = Object.entries(securityHeaders({ secure: secureCookies })); @@ -197,7 +197,7 @@ export function createApp(options: AppOptions = {}): Server { // Before auth: assets don't need a verified user, and the JWT cookie rides every request. if (pathname.startsWith("/public/") && (method === "GET" || method === "HEAD")) { - await assets.serve(url, req, res, (err) => reqLog.error("static stream error", { error: String(err) })); + await assets.serve(url, req, res, (err) => reqLog.error("static file error", { error: String(err) })); return; } diff --git a/src/http/static.test.ts b/src/http/static.test.ts index d5b2eaa..1836fe2 100644 --- a/src/http/static.test.ts +++ b/src/http/static.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { mkdirSync, mkdtempSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from "node:fs"; import { createServer } from "node:http"; import type { AddressInfo } from "node:net"; import { tmpdir } from "node:os"; @@ -17,11 +17,12 @@ mkdirSync(join(publicDir, "css"), { recursive: true }); mkdirSync(join(pluginsDir, "demo", "public"), { recursive: true }); writeFileSync(join(publicDir, "css", "a.css"), "body { color: red }"); writeFileSync(join(pluginsDir, "demo", "public", "app.css"), ".demo {}"); -const assets = publicAssets(publicDir, pluginsDir, new Set(["demo"])); +const assets = publicAssets(publicDir, pluginsDir, new Set(["demo"]), { cache: false }); const assetHref = assets.href; +const readErrors: Error[] = []; let base = ""; -const server = createServer((req, res) => void assets.serve(new URL(req.url ?? "/", "http://localhost"), req, res, (err) => assert.fail(err))); +const server = createServer((req, res) => void assets.serve(new URL(req.url ?? "/", "http://localhost"), req, res, (err) => readErrors.push(err))); before(() => new Promise((r) => server.listen(0, () => { base = `http://localhost:${(server.address() as AddressInfo).port}`; r(); @@ -68,6 +69,40 @@ test("assetHref changes when the file's content does", () => { assert.notEqual(assetHref("/public/css/changing.css"), first); }); +test("assetHref changes when the file's size does, even with its mtime restored", () => { + const file = join(publicDir, "css", "resized.css"); + writeFileSync(file, "a {}"); + const { atime, mtime } = statSync(file); + const first = assetHref("/public/css/resized.css"); + writeFileSync(file, "a { color: blue }"); + utimesSync(file, atime, mtime); + assert.notEqual(assetHref("/public/css/resized.css"), first); +}); + +test("with cache on, assetHref keeps a known version; a request naming it after an edit gets the short cache and the new ETag", async () => { + const cached = publicAssets(publicDir, pluginsDir, new Set(), { cache: true }); + const file = join(publicDir, "css", "cached.css"); + writeFileSync(file, "a {}"); + const stale = cached.href("/public/css/cached.css"); + writeFileSync(file, "a { color: green }"); + assert.equal(cached.href("/public/css/cached.css"), stale); + + const res = await fetch(base + stale); + assert.equal(res.status, 200); + assert.equal(await res.text(), "a { color: green }"); + assert.equal(res.headers.get("cache-control"), SHORT); + const fresh = assetHref("/public/css/cached.css"); + assert.notEqual(fresh, stale); + assert.equal(res.headers.get("etag"), `"${fresh.split("?v=")[1]}"`); +}); + +test("a version hashed while rendering equals the ETag hashed while serving the same bytes", async () => { + writeFileSync(join(publicDir, "css", "served-first.css"), "p {}"); + writeFileSync(join(publicDir, "css", "rendered-first.css"), "p {}"); + const etag = (await fetch(base + "/public/css/served-first.css")).headers.get("etag"); + assert.equal(etag, `"${assetHref("/public/css/rendered-first.css").split("?v=")[1]}"`); +}); + test("a URL carrying the current version is cached for a year as immutable", async () => { for (const method of ["GET", "HEAD"]) { const res = await fetch(base + assetHref("/public/css/a.css"), { method }); @@ -117,13 +152,21 @@ test("If-Modified-Since at or after the file's mtime answers 304; before it, or const earlier = new Date(Date.parse(lastModified) - 1000).toUTCString(); assert.equal((await fetch(base + "/public/css/a.css", { headers: { "if-modified-since": earlier } })).status, 200); assert.equal((await fetch(base + "/public/css/a.css", { headers: { "if-modified-since": "not a date" } })).status, 200); - // RFC 9110 §13.2.2: If-None-Match, when present, decides alone. assert.equal((await fetch(base + "/public/css/a.css", { headers: { "if-modified-since": lastModified, "if-none-match": '"other"' } })).status, 200); }); -test("unsafe or missing paths still answer 400, 403 and 404", async () => { +test("unsafe or missing paths answer 400, 403 and 404", async () => { assert.equal((await fetch(base + "/public/%E0%A4%A")).status, 400); assert.equal((await fetch(base + "/public/..%2f..%2fetc%2fpasswd")).status, 403); assert.equal((await fetch(base + "/public/css/missing.css")).status, 404); + assert.equal((await fetch(base + "/public/css/a.css/x")).status, 404); // ENOTDIR assert.equal((await fetch(base + "/public/css")).status, 404); + assert.deepEqual(readErrors, []); +}); + +test("a read failure other than a missing file answers 500 and reaches onError", async () => { + symlinkSync("loop.css", join(publicDir, "css", "loop.css")); // root reads a mode-000 file, so a symlink loop makes the failure + const res = await fetch(base + "/public/css/loop.css"); + assert.equal(res.status, 500); + assert.deepEqual(readErrors.splice(0).map((err) => (err as NodeJS.ErrnoException).code), ["ELOOP"]); }); diff --git a/src/http/static.ts b/src/http/static.ts index f42b27b..1f69005 100644 --- a/src/http/static.ts +++ b/src/http/static.ts @@ -1,4 +1,4 @@ -import { createHash } from "node:crypto"; +import { createHash, type Hash } from "node:crypto"; import { createReadStream, readFileSync, statSync, type Stats } from "node:fs"; import { stat } from "node:fs/promises"; import type { IncomingMessage, ServerResponse } from "node:http"; @@ -63,24 +63,43 @@ const SHORT = "public, max-age=300"; // gets a new hash. const hashes = new Map(); -function contentHash(filePath: string, info: Stats): string { +function memoized(filePath: string, info: Stats): string | undefined { const known = hashes.get(filePath); - if (known?.mtimeMs === info.mtimeMs && known.size === info.size) return known.hash; - const hash = createHash("sha256").update(readFileSync(filePath)).digest("base64url").slice(0, 16); + return known?.mtimeMs === info.mtimeMs && known.size === info.size ? known.hash : undefined; +} + +// The one definition of a version. Two readers feed it. A view renders synchronously, so hashFileSync +// reads the file whole; hashFile streams it, so a large file doesn't stall every other request. +function remember(filePath: string, info: Stats, sha256: Hash): string { + const hash = sha256.digest("base64url").slice(0, 16); hashes.set(filePath, { hash, mtimeMs: info.mtimeMs, size: info.size }); return hash; } -// Synchronous because EJS templates are: a view calls it mid-render. The stat per href is the -// price of picking up an edited file; the file is read and hashed once per version. -function versioned(href: string, route: StaticRoute): string { +function hashFileSync(filePath: string, info: Stats): string { + return memoized(filePath, info) ?? remember(filePath, info, createHash("sha256").update(readFileSync(filePath))); +} + +async function hashFile(filePath: string, info: Stats): Promise { + const known = memoized(filePath, info); + if (known !== undefined) return known; + const sha256 = createHash("sha256"); + for await (const chunk of createReadStream(filePath)) sha256.update(chunk); + return remember(filePath, info, sha256); +} + +// Without `cache`, a stat per href picks up an edited file. With it, a known version skips the stat: +// an edit reaches the hrefs once a request for the file hashes it again. +function versioned(href: string, route: StaticRoute, cache: boolean): string { try { const filePath = resolveStaticPath(route.dir, decodeURIComponent(route.subPath)); if (filePath === null) return href; + const known = cache ? hashes.get(filePath) : undefined; + if (known) return `${href}?v=${known.hash}`; const info = statSync(filePath); - return info.isFile() ? `${href}?v=${contentHash(filePath, info)}` : href; + return info.isFile() ? `${href}?v=${hashFileSync(filePath, info)}` : href; } catch { - return href; // malformed %-encoding or a missing file: the request answers 400/404 on its own + return href; // malformed %-encoding, or a missing or unreadable file: serving it answers the error } } @@ -122,12 +141,12 @@ async function serveStatic(route: StaticRequest, req: IncomingMessage, res: Serv try { const info = await stat(filePath); if (!info.isFile()) return plain(res, 404, "Not Found"); - const hash = contentHash(filePath, info); + const hash = await hashFile(filePath, info); const etag = `"${hash}"`; const validators = { "cache-control": route.version === hash ? IMMUTABLE : SHORT, etag, "last-modified": info.mtime.toUTCString() }; if (notModified(req, etag, info)) return void res.writeHead(304, validators).end(); res.writeHead(200, { ...validators, "content-length": info.size, "content-type": contentTypeFor(filePath) }); - if (req.method === "HEAD") return void res.end(); // headers only — skip opening the file + if (req.method === "HEAD") return void res.end(); // Headers are already sent, so a mid-stream read error can't become an HTTP status — // log and destroy the response to signal a truncated body, not a hung socket. createReadStream(filePath) @@ -136,21 +155,24 @@ async function serveStatic(route: StaticRequest, req: IncomingMessage, res: Serv res.destroy(); }) .pipe(res); - } catch { - plain(res, 404, "Not Found"); + } catch (err) { + const { code } = err as NodeJS.ErrnoException; + if (code === "ENOENT" || code === "ENOTDIR") return plain(res, 404, "Not Found"); + onError(err as Error); + plain(res, 500, "Internal Server Error"); } } export interface PublicAssets { href: (href: string) => string; // a /public/ href with its version; any other href unchanged - // onError handles a mid-stream read failure (headers already sent). + // onError receives every failure to read a file except a missing one, which answers 404. serve: (url: URL, req: IncomingMessage, res: ServerResponse, onError: (err: Error) => void) => Promise; } -export function publicAssets(publicDir: string, pluginsDir: string, pluginIds: Set): PublicAssets { +export function publicAssets(publicDir: string, pluginsDir: string, pluginIds: Set, { cache }: { cache: boolean }): PublicAssets { const route = (pathname: string): StaticRoute => routePublic(pathname.slice("/public/".length), publicDir, pluginsDir, pluginIds); return { - href: (href) => (href.startsWith("/public/") && !/[?#]/.test(href) ? versioned(href, route(href)) : href), + href: (href) => (href.startsWith("/public/") && !/[?#]/.test(href) ? versioned(href, route(href), cache) : href), serve: (url, req, res, onError) => serveStatic({ ...route(url.pathname), version: url.searchParams.get("v") }, req, res, onError), }; }