Skip to content

fix(plugin): HEAD encoding parity + conditional-request etag; v0.29.0 - #66

Open
harper-joseph wants to merge 1 commit into
mainfrom
fix/head-response-headers
Open

fix(plugin): HEAD encoding parity + conditional-request etag; v0.29.0#66
harper-joseph wants to merge 1 commit into
mainfrom
fix/head-response-headers

Conversation

@harper-joseph

Copy link
Copy Markdown
Contributor

Two defects on the bot response path, both found by probing the live origin and both fixed in response.js.

1. HEAD advertised an encoding the GET never delivered

Measured against prod, same URL, seconds apart:

Client sends GET returned HEAD returned
no Accept-Encoding (no content-encoding), 430,751 bytes identity content-encoding: gzip
Accept-Encoding: gzip content-encoding: gzip, 92,902 bytes content-encoding: gzip

negotiateEncoding was gated on body, which a HEAD nulls by definition, so it never ran and the stored gzip header passed through untouched. RFC 9110 §9.3.2 requires a HEAD to send the header fields a GET would have sent.

Fixed by gating on resource.content — present regardless of method — and letting negotiateEncoding do its header half when there are no bytes to transcode. The gate also had to exclude responses with no representation at all (a 304, and the render-now 504 fallback whose content is null); without that, a client advertising gzip would be handed content-encoding: gzip on a bodiless reply.

2. Cached pages had no conditional-request validator

The cached response carried neither etag nor last-modified, so applyConditional — already implemented and correct — could never fire. Every conditional request got a full 200, meaning a ~430KB homepage re-sent to crawlers that already had it.

Now synthesizes a weak etag from lastCached, which is the version of a cached page: it changes exactly when a render replaces the content. An upstream etag, if present, is never clobbered.

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. (age is unchanged — it was already being sent, isn't an SEO signal, and is inert here anyway since the origin sends no-cache, no-store.)

Harper compatibility — verified

Checked against the exact core commit prod runs (00295c7, pinned by harper-pro v5.1.23; core is a submodule, so it isn't in the tag tree):

  • No response-compression logic anywhere on the server.http path — the plugin owns encoding end to end.
  • Handler headers pass through verbatim via writeHead(status, toWriteHeadHeaders(headers)).
  • Harper's own etag / If-None-Match handling lives in REST.ts, which this handler returns before ever reaching. No override, no double-304.
  • Harper's Node path has a purpose-built HEAD guard (if (!body) { if (request.method !== 'HEAD') headers.set('Content-Length', '0') }), which is why HEAD responses are already clean.

Considered and dropped: HEAD content-length

HEAD carries no content-length today (the stored page has none, and the plugin pre-streams the Blob so Harper's body.size branch never fires). Setting one manually would work on the Node path but is a trap: 5.2's new uWS writer strips content-length unconditionally (if (lower === 'content-length') continue;), so it would become a silent no-op the day a uWS-enabled build ships. Left alone — that needs a Harper-side fix, not a plugin one.

Testing

385/385 plugin tests pass; 9 new cases cover HEAD/GET encoding parity in both directions, the 304 and bodiless-fallback guards, etag synthesis and non-clobbering, and an etag round-trip to 304 for both methods.

⚠️ Before deploy

Fix 2 is not HEAD-scoped — it adds an etag to every cached 200 and hands the CDN a validator it doesn't have today, which invites conditional revalidation against Harper that currently never happens. Given the origin sets cache-control: max-age=0, no-cache, no-store, the edge's actual behavior is a question for whoever owns the property config. Worth an ack before this rolls out; happy to split it into its own PR if you'd rather land the HEAD fix alone.

🤖 Generated with Claude Code

…e a conditional-request etag; v0.29.0

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) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request bumps the version of @harperfast/prerender to 0.29.0 and introduces improvements to HTTP response handling. Specifically, it synthesizes a weak ETag from the cache timestamp when no upstream ETag is present, and ensures that HEAD requests correctly negotiate and report the same content-encoding headers as GET requests without sending a body. It also adds robust unit tests to verify these behaviors for HEAD requests, 304 responses, and bodiless fallbacks. There are no review comments, so I have no additional feedback to provide.

@harper-joseph

Copy link
Copy Markdown
Contributor Author

Heads-up on a version collision: #67 merged and main is now at 0.30.0, with prerender-v0.30.0 released.

This branch stamps 0.29.0, which was reserved before #67 but never released. Merging as-is would set packages/plugin/package.json backwards on main — the same downgrade that happened in #59 and had to be repaired by #60.

Rebasing onto origin/main and re-stamping as 0.31.0 avoids it. 0.29.0 is simply skipped, which costs nothing since no tarball was ever published for it.

Worth also re-running the version check after the rebase rather than trusting the local checkout — git show origin/main:packages/plugin/package.json plus gh release list, since main has moved three times today (0.28.0 → 0.30.0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant