fix(prod-server): send headers-only for HEAD in sendCompressed (#1980) - #2058
Merged
james-elicx merged 2 commits intoJun 18, 2026
Merged
Conversation
…flare#1980) sendCompressed wrote the full body for every request. HEAD requests routed through it (bot/crawler-buffered Pages HTML and Pages API routes) still compressed and emitted a payload that Node then discards at the socket level. Short-circuit with res.end() after writeHead when req.method is HEAD, mirroring sendWebResponse. This skips spinning up a compressor for a body that would be thrown away and keeps the buffered sender consistent with the streamed and static-file senders, which already handle HEAD.
commit: |
Member
|
/bigbonk review for issues |
Contributor
There was a problem hiding this comment.
Reviewed the change. The fix is correct, minimal, and well-justified.
Verified:
- The HEAD short-circuit in both branches of
sendCompressedfaithfully mirrorssendWebResponse(prod-server.ts:894-901): write headers (includingContent-Encodingon the compressed branch), thenres.end()with no body.tryServeStatic(prod-server.ts:696-729) uses the identical pattern, so all three buffered/streamed senders are now consistent. - Moving
createCompressor(encoding)below the HEAD guard correctly avoids spinning up a compressor whose output Node would discard anyway. - In the compressed branch,
Content-Lengthis (correctly) not emitted — matchingsendWebResponseandtryServeStatic, where compressed responses omit it since the encoded size isn't known ahead of time. The PR description's RFC 9110 wording could be read as implying HEAD should always carry the full-bodyContent-Length, but for transfer-coded/compressed responses omitting it is the right call and matches the GET path. No change needed. - The integration tests exercise the real path: buffered bot SSR and Pages API responses both flow through
sendCompressed(prod-server.ts:1846-1880) when adefaultContentTypeis present, so the/api/helloHEAD test genuinely covers the guard rather than only Node's socket-level suppression. - The PR description's nuance (Node already suppresses HEAD bodies via
_hasBody = false) is accurate, so this is an optimization + consistency fix with no wire-behavior change.
Tested: vp test run tests/features.test.ts -t "sendCompressed" → 9/9 pass.
No blocking issues. LGTM.
Contributor
|
Review posted to PR #2058. SummaryThe PR fixes
No blocking issues. The change is correct, minimal, and well-tested. |
Contributor
Performance benchmarksCompared 0 improved · 3 regressed · 3 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sendCompressed(the Pages Router prod-server sender for buffered responses) wrote the full body for every request, including HEAD. HEAD requests that route through it — bot/crawler-buffered Pages HTML (pages-page-response.tsreturnsnew Response(fullHtml, …)forisPagesStreamingBot) and Pages API routes — would compress and emit a payload. Every other sender already strips the body for HEAD:sendWebResponse(streamed path),tryServeStatic(static assets).sendCompressedwas the lone exception.This change short-circuits HEAD with
res.end()(headers only) right afterwriteHeadin both branches, mirroringsendWebResponse. The compressor is now created only when a body will actually be written.Closes #1980.
Important nuance — this is an optimization + consistency fix, not a wire-behavior change
While implementing, I verified that Node's
http.ServerResponsealready suppresses HEAD response bodies at the socket level (_hasBody = falsefor HEAD): even an explicitres.end(buf)yields a 0-byte body while preservingContent-Length. So the observable HTTP response was already correct — for the HTML render path and API routes, before and after this change.What the fix actually buys:
pipeline()only for Node to discard it on HEAD.sendCompressednow matchessendWebResponse/tryServeStatic.This also means the "API-route parity" angle from the issue is moot: Next.js runs on the same Node http server, so its API-route HEAD bodies are suppressed too — there is no observable divergence either way. The guard is therefore applied unconditionally inside
sendCompressed(the issue's primary suggested fix), rather than scoped by content type.Changes
packages/vinext/src/server/prod-server.ts—sendCompressedreturns headers-only forreq.method === "HEAD";createCompressormoved below the guard.tests/features.test.ts— 3 unit tests: HEAD on a compressible response (headers incl.Content-Encoding, empty body, no compressor); HEAD uncompressed (keepsContent-Length, empty body); non-HEAD GET still writes the full body (guard doesn't over-trigger).tests/pages-router.test.ts— 2 integration tests: bot-buffered Pages SSR HEAD and Pages API HEAD both return status + headers with an empty body.Test plan
sendCompressedunit tests: 9/9 passtests/pages-router.test.ts: 315/315 ·tests/app-router-production-server.test.ts: 58/58