From e09ab35d0e8b891dc7d9dcdc9a277d2eb955155b Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 5 Aug 2026 12:48:32 -0400 Subject: [PATCH] fix(plugin): HEAD reports the encoding a GET would return + synthesize a conditional-request etag; v0.29.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects found by probing the live bot path, both in the response builder. 1. A HEAD advertised the STORED content-encoding while the GET re-encoded. Measured against prod: a client sending no accept-encoding got `content-encoding: gzip` on the HEAD and identity bytes (430,751 of them) on the GET — the HEAD described a representation the GET never delivered (RFC 9110 §9.3.2). Cause: negotiateEncoding was gated on `body`, which a HEAD nulls by definition, so it never ran. Gate on `resource.content` instead — present regardless of method — and let the function do its header half with nothing to transcode. The gate also has to exclude responses with no representation at all (a 304, and the render-now 504 fallback whose content is null), or a client advertising gzip would be handed `content-encoding: gzip` on a bodiless reply. 2. Cached pages carried no validator, so applyConditional could never fire and every conditional request got a full 200 — a ~430KB homepage re-sent to crawlers that already had it. Synthesize a weak etag from lastCached, which IS the version of a cached page: it changes exactly when a render replaces the content. Deliberately no last-modified. A date-semantic validator would flap on every re-render even when the content is byte-identical, misrepresenting content modification to crawlers; an etag is opaque and carries no freshness claim. An upstream etag, if present, is never clobbered. Verified against the core commit prod runs (00295c7, pinned by harper-pro v5.1.23): Harper has no response-compression logic on the server.http path, passes handler headers through verbatim, and its own etag/If-None-Match handling lives in REST.ts, which the plugin's handler returns before ever reaching. Co-Authored-By: Claude Opus 5 (1M context) --- package-lock.json | 2 +- packages/plugin/package.json | 2 +- packages/plugin/src/http_handlers/response.js | 31 ++++- packages/plugin/test/response.test.js | 117 ++++++++++++++++++ 4 files changed, 149 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 05e5836..260ac04 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9007,7 +9007,7 @@ }, "packages/plugin": { "name": "@harperfast/prerender", - "version": "0.28.0", + "version": "0.29.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..5d5620c 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -1,6 +1,6 @@ { "name": "@harperfast/prerender", - "version": "0.28.0", + "version": "0.29.0", "type": "module", "description": "Configurable Harper plugin for prerendering pages for bots and crawlers", "license": "Apache-2.0", diff --git a/packages/plugin/src/http_handlers/response.js b/packages/plugin/src/http_handlers/response.js index 937bae0..1056c7f 100644 --- a/packages/plugin/src/http_handlers/response.js +++ b/packages/plugin/src/http_handlers/response.js @@ -57,6 +57,21 @@ export function buildResponseHeaders(resource) { if (!isNaN(lastCachedMs)) { const ageSec = Math.max(0, Math.floor((Date.now() - lastCachedMs) / 1000)); headers.set('age', String(ageSec)); + + // Conditional-request validator for a cached page. The stored representation + // changes only when a render replaces it, so lastCached IS its version. + // + // Deliberately NO `last-modified`: an ETag is an opaque version token with no date + // semantics, so it buys 304s without publishing a freshness date that would flap on + // every re-render even when the content is byte-identical. Weak is the honest + // strength — the same page is served gzip or identity depending on negotiation, and + // weak comparison is what RFC 7232 conditional GET/HEAD uses regardless. + // + // Never clobber an upstream validator: if the origin/render supplied its own, that + // one describes the content more precisely than our render timestamp does. + if (!headers.has('etag')) { + headers.set('etag', `W/"${lastCachedMs.toString(36)}"`); + } } } @@ -161,6 +176,9 @@ export function applyConditional(status, headers, request, body) { * Re-encode the body to the client's best accepted encoding when it differs from what the * upstream sent. Mutates `content-encoding`/`content-length` on `headers` and returns the * (possibly re-encoded) body. + * + * Called for bodiless responses too — a HEAD must report the encoding a GET would have + * returned, so the header half runs even when there is nothing to transcode. */ export function negotiateEncoding(body, headers, request) { const contentEncoding = headers.get('content-encoding') || null; @@ -175,6 +193,10 @@ export function negotiateEncoding(body, headers, request) { } headers.delete('content-length'); + // No bytes to transcode (HEAD). The headers above have already been corrected, which is + // the whole point of running this for a bodiless response. + if (!body) return body; + return reencode(Readable.fromWeb(body), contentEncoding, bestEncoding, false); } @@ -216,7 +238,14 @@ export function deliverResource(resource, request, info = {}) { headers.set('x-harper-render-now', info.renderNowStatus); } - if (body) { + // Negotiate whenever the resource HAS a representation — including a HEAD, whose headers + // must describe what a GET would have returned even though no bytes follow (RFC 9110 + // §9.3.2). Gated on `resource.content` rather than `body` precisely because a HEAD nulls + // the body above: gating on `body` left HEAD advertising the stored `content-encoding: + // gzip` while the GET re-encoded to identity. A 304 carries no representation, and neither + // does the render-now 504 fallback (content: null) — both keep their headers untouched, or + // we would invent a content-encoding for a body that does not exist. + if (status !== 304 && resource.content) { body = negotiateEncoding(body, headers, request); } diff --git a/packages/plugin/test/response.test.js b/packages/plugin/test/response.test.js index 251d263..e0c8359 100644 --- a/packages/plugin/test/response.test.js +++ b/packages/plugin/test/response.test.js @@ -1,5 +1,6 @@ import { test, beforeEach } from 'node:test'; import assert from 'node:assert/strict'; +import zlib from 'node:zlib'; import { applyOptions } from '../src/config.js'; // response.js transitively imports PrerenderedPage, which extends a Harper `databases` @@ -176,3 +177,119 @@ test('deliverResource gates debug headers on the debug request header, and repor const debug = deliverResource(resource, mockRequest({ 'x-harper-prerender-debug': 'true' }), { source: 'origin' }); assert.equal(debug.headers.get('x-harper-source'), 'origin'); }); + +// --- HEAD (RFC 9110 §9.3.2): no body, but headers identical to the GET's --- + +const headRequest = (headers = {}) => ({ ...mockRequest(headers), method: 'HEAD' }); + +// A gzip payload so the GET half of the comparison can actually decode rather than +// erroring asynchronously on a stream that only claims to be gzip. +const gzipStream = () => { + const gzipped = zlib.gzipSync(Buffer.from('hydrated')); + return new ReadableStream({ + start(c) { + c.enqueue(new Uint8Array(gzipped)); + c.close(); + }, + }); +}; + +const gzipResource = () => ({ + statusCode: 200, + miss: true, + headers: { 'content-type': 'text/html', 'content-encoding': 'gzip' }, + content: gzipStream(), + url: 'https://x/', + deviceType: 'desktop', + cacheKey: 'https://x/|desktop', +}); + +test('deliverResource: a HEAD reports the encoding a GET would have returned, not the stored one', () => { + // The live prod regression: HEAD kept the stored `content-encoding: gzip` while the GET + // re-encoded to identity, so HEAD described a representation the GET never delivered. + const get = deliverResource(gzipResource(), mockRequest(), {}); + const head = deliverResource(gzipResource(), headRequest(), {}); + + assert.equal(head.body, undefined, 'HEAD must carry no body'); + assert.notEqual(get.body, undefined, 'GET still carries a body'); + + // The whole point: identical encoding metadata for identical requests. + assert.equal(get.headers.has('content-encoding'), false); + assert.equal(head.headers.has('content-encoding'), false); + assert.equal(head.headers.get('content-type'), get.headers.get('content-type')); +}); + +test('deliverResource: a HEAD that accepts the stored encoding keeps it', () => { + const head = deliverResource(gzipResource(), headRequest({ 'accept-encoding': 'gzip' }), {}); + assert.equal(head.headers.get('content-encoding'), 'gzip'); + assert.equal(head.body, undefined); +}); + +test('deliverResource: a 304 never gains a content-encoding', () => { + // Negotiation must not run on a response with no representation — otherwise a client + // advertising gzip would get `content-encoding: gzip` on a bodiless 304. + const lastCached = new Date('2026-08-01T00:00:00Z'); + const etag = `W/"${lastCached.getTime().toString(36)}"`; + const resource = { ...gzipResource(), lastCached }; + + const res = deliverResource(resource, mockRequest({ 'if-none-match': etag, 'accept-encoding': 'gzip' }), {}); + assert.equal(res.status, 304); + assert.equal(res.body, undefined); + assert.equal(res.headers.has('content-encoding'), false); +}); + +test('deliverResource: a bodiless fallback (content: null) keeps its headers untouched', () => { + // The render-now timeout 504 has no representation at all. + const resource = { + miss: true, + statusCode: 504, + url: 'https://x/', + deviceType: 'desktop', + headers: {}, + content: null, + }; + const res = deliverResource(resource, mockRequest({ 'accept-encoding': 'gzip' }), {}); + assert.equal(res.status, 504); + assert.equal(res.headers.has('content-encoding'), false); +}); + +// --- Conditional-request validator synthesized from lastCached --- + +test('buildResponseHeaders synthesizes a weak etag from lastCached, and no last-modified', () => { + const lastCached = new Date('2026-08-01T00:00:00Z'); + const headers = buildResponseHeaders({ statusCode: 200, headers: { 'content-type': 'text/html' }, lastCached }); + + assert.equal(headers.get('etag'), `W/"${lastCached.getTime().toString(36)}"`); + // Deliberately absent: a date-semantic validator would flap on every re-render even when + // the content is byte-identical. + assert.equal(headers.has('last-modified'), false); +}); + +test('buildResponseHeaders never clobbers an upstream etag', () => { + const headers = buildResponseHeaders({ + statusCode: 200, + headers: { etag: '"from-origin"' }, + lastCached: new Date('2026-08-01T00:00:00Z'), + }); + assert.equal(headers.get('etag'), '"from-origin"'); +}); + +test('buildResponseHeaders omits the etag when there is no usable lastCached', () => { + assert.equal(buildResponseHeaders({ statusCode: 200, headers: {} }).has('etag'), false); + assert.equal(buildResponseHeaders({ statusCode: 200, headers: {}, lastCached: 'nonsense' }).has('etag'), false); + // Not a cached 200 => no validator to offer. + assert.equal(buildResponseHeaders({ statusCode: 404, headers: {}, lastCached: new Date() }).has('etag'), false); +}); + +test('the synthesized etag round-trips to a 304 for both GET and HEAD', () => { + const lastCached = new Date('2026-08-01T00:00:00Z'); + const etag = buildResponseHeaders({ statusCode: 200, headers: {}, lastCached }).get('etag'); + const resource = { ...gzipResource(), lastCached }; + + for (const request of [mockRequest({ 'if-none-match': etag }), headRequest({ 'if-none-match': etag })]) { + const res = deliverResource({ ...resource, content: gzipStream() }, request, {}); + assert.equal(res.status, 304); + assert.equal(res.body, undefined); + assert.equal(res.headers.get('etag'), etag); + } +});