diff --git a/package-lock.json b/package-lock.json index 05e5836..1363bfc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9007,7 +9007,7 @@ }, "packages/plugin": { "name": "@harperfast/prerender", - "version": "0.28.0", + "version": "0.30.0", "license": "Apache-2.0", "dependencies": { "fast-xml-parser": "^5.0.9", diff --git a/packages/plugin/package.json b/packages/plugin/package.json index 8f73787..124d825 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -1,6 +1,6 @@ { "name": "@harperfast/prerender", - "version": "0.28.0", + "version": "0.30.0", "type": "module", "description": "Configurable Harper plugin for prerendering pages for bots and crawlers", "license": "Apache-2.0", diff --git a/packages/plugin/src/configSchema.js b/packages/plugin/src/configSchema.js index 18a548f..93880fd 100644 --- a/packages/plugin/src/configSchema.js +++ b/packages/plugin/src/configSchema.js @@ -249,6 +249,27 @@ export const configSchema = group('Prerender plugin configuration.', { 'authorization, and the security-token/debug header names). Matched case-insensitively.', { movedFrom: 'ignoredHeaders', itemType: 'string' } ), + maxResponseHeaderBytes: option( + 64 * 1024, + 'Largest response head Harper will accept from the origin, summed across every header name ' + + 'and value in the response (not per header).\n\n' + + 'Undici defaults this to Node’s `http.maxHeaderSize` (16 KiB), which is a header-flood ' + + 'mitigation for servers accepting untrusted requests — too strict for a reverse proxy reading ' + + 'its own origin. A real origin can exceed 16 KiB on a single page (several Set-Cookie plus ' + + 'CSP, Link rel=preload, NEL, Report-To), and undici responds by destroying the connection ' + + 'with UND_ERR_HEADERS_OVERFLOW, so the crawler gets a 500 for a page browsers and the CDN ' + + 'load normally. It fails deterministically for those URLs, since it is a property of the ' + + 'origin’s response rather than a transient. Hence a default well above Node’s, matching what ' + + 'a CDN in front of the same origin already tolerates.\n\n' + + 'Raising it raises the worst-case memory held per connection while a response head is ' + + 'parsed, which is why it is bounded at both ends. The 1 MiB ceiling is far above any ' + + 'legitimate response head — it exists to catch a typo (a stray factor of a thousand) ' + + 'before it becomes an out-of-memory risk multiplied across concurrent connections.\n\n' + + 'Restart-scoped: undici fixes `maxHeaderSize` when the dispatcher is constructed and offers ' + + 'no way to change it afterwards, so a live edit is reported as pending-restart and the ' + + 'running dispatchers keep the value they were built with.', + { unit: 'bytes', min: 16 * 1024, max: 1024 * 1024, scope: 'restart' } + ), }), debugHeader: group('Debug response headers, emitted when the request carries this header (any value).', { diff --git a/packages/plugin/src/util/upstream.js b/packages/plugin/src/util/upstream.js index b2f8445..43009b8 100644 --- a/packages/plugin/src/util/upstream.js +++ b/packages/plugin/src/util/upstream.js @@ -3,8 +3,6 @@ import { isIP } from 'node:net'; import { Agent } from 'undici'; import { config } from '../config.js'; -const agent = new Agent({}); - /** * The staging IP to connect to for this origin fetch, or undefined for a normal fetch. * Staging passthrough is active only when a staging `ip` is configured (and valid) AND @@ -28,19 +26,42 @@ export const configuredStagingIp = () => { return ip && isIP(ip) ? ip : undefined; }; -// Dispatchers that pin DNS resolution to a fixed IP (staging passthrough), one per IP. -// Only the connect address is overridden — the origin (so Host header + TLS SNI + cert -// validation) stays the real origin host, the server-side equivalent of Chrome's -// --host-resolver-rules=MAP host ip. In practice there is at most one entry (the single -// configured staging IP); the map just keeps it stable across requests and across a -// config reload that changes the IP. +// `maxHeaderSize` is fixed at Agent construction — undici exposes no way to change it on a live +// Agent — so `origin.maxResponseHeaderBytes` is restart-scoped: config.js reports a live change +// as pending-restart and the running dispatchers keep the value they were built with. Without it +// undici falls back to Node's http.maxHeaderSize (16 KiB), which a real origin can exceed on a +// single page (a Set-Cookie pile-up plus CSP/Link-preload is enough), and undici answers by +// DESTROYING THE SOCKET with UND_ERR_HEADERS_OVERFLOW. The crawler then gets a 500 for a page +// browsers and the CDN load fine, deterministically, because it is a property of that response. +// Captured on first use and reused by every dispatcher built afterwards, so restart scope holds +// for all of them. Re-reading config per construction would not: `origin.staging.ip` is +// live-scoped, so a pinned dispatcher can be built long after boot, and it would then pick up a +// cap edited in the meantime while the unpinned singleton kept the boot value — two dispatchers +// disagreeing, and a pending-restart notice that was only half true. +let capturedMaxHeaderSize; +const agentOptions = () => ({ + maxHeaderSize: (capturedMaxHeaderSize ??= config.origin.maxResponseHeaderBytes), +}); + +// The unpinned dispatcher carries every cache-miss and passthrough fetch, so it stays a plain +// lazily-built singleton: one `??=` test on the hot path, no key to build and no Map to probe. +// It cannot be built at import time because the cap is not known until the component applies +// its options; by the first origin fetch it always is. +let agent; + +// Dispatchers that pin DNS resolution to a fixed IP (staging passthrough), one per IP. Only the +// connect address is overridden — the origin (so Host header + TLS SNI + cert validation) stays +// the real origin host, the server-side equivalent of Chrome's --host-resolver-rules=MAP host ip. +// In practice there is at most one entry (the single configured staging IP); the map just keeps +// it stable across requests and across a config reload that changes the IP. const pinnedDispatchers = new Map(); export const dispatcherFor = (ip) => { - if (!ip) return agent; + if (!ip) return (agent ??= new Agent(agentOptions())); let dispatcher = pinnedDispatchers.get(ip); if (!dispatcher) { const family = isIP(ip); dispatcher = new Agent({ + ...agentOptions(), connect: { // Node's lookup callback has two shapes depending on the `all` option. lookup: (_hostname, options, callback) => diff --git a/packages/plugin/test/config.test.js b/packages/plugin/test/config.test.js index 0b88ecb..a06fa6f 100644 --- a/packages/plugin/test/config.test.js +++ b/packages/plugin/test/config.test.js @@ -203,7 +203,11 @@ test('defaultConfig returns fresh deep copies (no shared references)', () => { test('secret and restart paths are what the schema declares', () => { assert.deepEqual(secretPaths().sort(), ['origin.securityToken.value', 'renderNow.token']); - assert.deepEqual(restartPaths().sort(), ['render.reconcile.startDelay', 'render.reconcile.startJitter']); + assert.deepEqual(restartPaths().sort(), [ + 'origin.maxResponseHeaderBytes', + 'render.reconcile.startDelay', + 'render.reconcile.startJitter', + ]); }); test('describeConfigSchema is JSON-serializable and carries the editor contract', () => { diff --git a/packages/plugin/test/upstream.test.js b/packages/plugin/test/upstream.test.js index 9452e77..956543d 100644 --- a/packages/plugin/test/upstream.test.js +++ b/packages/plugin/test/upstream.test.js @@ -1,8 +1,11 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; +import http from 'node:http'; import { applyOptions, config } from '../src/config.js'; +import { restartPaths } from '../src/configSchema.js'; import { configuredStagingIp, + dispatcherFor, resolveUpstreamHeaders, sanitizeOriginResponseHeaders, stagingTargetIp, @@ -194,3 +197,160 @@ test('resolveUpstreamHeaders picks up ignoredHeaders changes across applyOptions assert.equal(upstream['x-first'], 'a'); assert.equal(upstream['x-second'], undefined); }); + +// --- origin response-header cap ------------------------------------------------------------- +// +// Asserted behaviorally against a real server rather than by reading undici's internal +// kMaxHeadersSize symbol, so the tests survive an undici refactor and actually prove the thing +// that broke in production: a large-but-legitimate origin response head must not kill the request. + +// Serve a response whose head sums to `bytes` across many headers — the shape a real origin +// produces (a Set-Cookie pile-up plus CSP/Link-preload), since the cap is cumulative over the +// whole head, not per header. +// +// undici counts header NAME and VALUE bytes (Parser.onHeaderField / onHeaderValue each call +// trackHeader with their own buffer length) and not the `: ` / CRLF delimiters, so budgeting +// `name.length + value.length` per header is exactly what the cap sees. +const HEADER_BYTES = 1024; +const serverWithHeadBytes = async (bytes) => { + const server = http.createServer((_req, res) => { + const headers = {}; + for (let i = 0; i < Math.ceil(bytes / HEADER_BYTES); i++) { + const name = `x-pad-${i}`; + headers[name] = 'a'.repeat(HEADER_BYTES - name.length); + } + res.writeHead(200, headers); + res.end('ok'); + }); + // Reject on a listen error rather than leaving the await to hang forever (EADDRINUSE, or a + // sandbox that forbids binding) — a hung test is far harder to diagnose than a failed one. + await new Promise((resolve, reject) => { + server.once('error', reject); + server.listen(0, '127.0.0.1', resolve); + }); + return { server, origin: `http://127.0.0.1:${server.address().port}` }; +}; + +const withServer = async (bytes, fn) => { + const { server, origin } = await serverWithHeadBytes(bytes); + try { + return await fn(origin); + } finally { + // close() is async; awaiting it keeps a lingering handle from leaking into the next test. + await new Promise((resolve) => server.close(resolve)); + } +}; + +test('a 32 KiB origin response head succeeds under the default cap', async () => { + applyOptions({}); + assert.equal(config.origin.maxResponseHeaderBytes, 64 * 1024); + // The whole point: the default must clear Node's http.maxHeaderSize, which is what undici + // falls back to and what produced UND_ERR_HEADERS_OVERFLOW -> 500 for the crawler. + assert.ok(config.origin.maxResponseHeaderBytes > http.maxHeaderSize); + + await withServer(32 * 1024, async (origin) => { + const res = await dispatcherFor(undefined).request({ origin, path: '/', method: 'GET' }); + assert.equal(res.statusCode, 200); + await res.body.text(); + }); +}); + +test('the cap is genuinely enforced (a head above it still overflows)', async () => { + // Proves the option is wired to undici rather than merely stored. A fresh module instance is + // needed because the unpinned dispatcher is built once per process — which is the restart + // scope, asserted below. The query string gives a distinct module URL; config.js resolves to + // the same URL either way, so the singleton config this sets is what the fresh module reads. + applyOptions({ origin: { maxResponseHeaderBytes: 16 * 1024 } }); + const fresh = await import('../src/util/upstream.js?fresh=low-cap'); + try { + await withServer(32 * 1024, async (origin) => { + await assert.rejects( + () => fresh.dispatcherFor(undefined).request({ origin, path: '/', method: 'GET' }), + (err) => err.code === 'UND_ERR_HEADERS_OVERFLOW' + ); + }); + } finally { + applyOptions({}); + } +}); + +test('the unpinned dispatcher is built once and ignores a live cap change', async () => { + // Both halves of restart scope: the hot path must not rebuild per request (efficiency), and a + // live edit must not silently take effect (correctness — config.js reports pending-restart). + applyOptions({}); + const first = dispatcherFor(undefined); + assert.equal(dispatcherFor(undefined), first); + + applyOptions({ origin: { maxResponseHeaderBytes: 128 * 1024 } }); + assert.equal(dispatcherFor(undefined), first, 'a live cap change must not swap the dispatcher'); + + // ...and it still honors the cap it was constructed with, not the newly configured one. + await withServer(32 * 1024, async (origin) => { + const res = await first.request({ origin, path: '/', method: 'GET' }); + assert.equal(res.statusCode, 200); + await res.body.text(); + }); + applyOptions({}); +}); + +test('maxResponseHeaderBytes is declared restart-scoped', () => { + // Guards the scope declaration itself: dropping it would make the option look live while the + // running dispatcher quietly kept the old cap. + assert.ok(restartPaths().includes('origin.maxResponseHeaderBytes')); +}); + +test('a dispatcher built after a live cap edit still uses the captured cap', async () => { + // origin.staging.ip IS live-scoped, so enabling staging mints a pinned dispatcher long after + // boot. If that construction re-read config it would pick up a cap edited in the meantime + // while the unpinned singleton kept the boot value — two dispatchers disagreeing, and a + // pending-restart notice that was only half true. The cap is captured once instead. + // A fresh module instance so the pinned entry is genuinely built here rather than reused from + // an earlier test, and so the capture starts unset. + applyOptions({}); + const fresh = await import('../src/util/upstream.js?fresh=capture-once'); + fresh.dispatcherFor(undefined); // force the capture at the default + + applyOptions({ origin: { maxResponseHeaderBytes: 16 * 1024, staging: { ip: '127.0.0.1' } } }); + const pinnedAfterEdit = fresh.dispatcherFor('127.0.0.1'); + + // Built after the edit, but still honors the captured 64 KiB — a 32 KiB head must pass. Were + // it reading config at construction it would have taken the 16 KiB cap and overflowed. + await withServer(32 * 1024, async (origin) => { + const res = await pinnedAfterEdit.request({ origin, path: '/', method: 'GET' }); + assert.equal(res.statusCode, 200); + await res.body.text(); + }); + applyOptions({}); +}); + +test('the staging-pinned dispatcher carries the cap too', async () => { + // Constructed on its own branch, so it is the easy one to miss — and a staging deploy that + // 500s on every large-header page would look like a staging-edge fault, not a config gap. + applyOptions({}); + const pinned = dispatcherFor('127.0.0.1'); + assert.notEqual(pinned, dispatcherFor(undefined)); + + await withServer(32 * 1024, async (origin) => { + // The pin rewrites DNS to 127.0.0.1; the port still comes from the origin URL. + const res = await pinned.request({ origin, path: '/', method: 'GET' }); + assert.equal(res.statusCode, 200); + await res.body.text(); + }); +}); + +test('a cap outside the schema bounds is rejected back to the default', () => { + // enforceSchemaConstraints warns and restores the default rather than throwing, so a typo + // degrades to the safe 64 KiB instead of silently reintroducing the 16 KiB failure... + applyOptions({ origin: { maxResponseHeaderBytes: 1024 } }); + assert.equal(config.origin.maxResponseHeaderBytes, 64 * 1024); + + // ...and the ceiling catches the opposite typo — a stray factor of a thousand — before it + // becomes an out-of-memory risk multiplied across concurrent connections. + applyOptions({ origin: { maxResponseHeaderBytes: 64 * 1024 * 1024 } }); + assert.equal(config.origin.maxResponseHeaderBytes, 64 * 1024); + + // The bounds themselves are inclusive and must stay usable. + applyOptions({ origin: { maxResponseHeaderBytes: 1024 * 1024 } }); + assert.equal(config.origin.maxResponseHeaderBytes, 1024 * 1024); + applyOptions({}); +});