rest: gzip strips stale Content-Length at every commit; coarse ErrHijacked carve-out (#175)#184
Merged
Conversation
…lose-log carve-out (#175) Part 1 — a handler-set uncompressed Content-Length behind GzipMiddleware truncated the longer compressed stream on the wire (client: unexpected EOF; handler: every call returns nil) on two residual commit paths Write/#167's FlushError did not cover: - explicit WriteHeader: net/http latches the length at the final WriteHeader, so gzipResponseWriter.WriteHeader now Dels it first. Forwarded informational 1xx (minus 101) strip nothing (#165 convention); gzip joins the shared wrapper table in informational_internal_test.go. - a bodyless handler (sets CL, returns without writing): the commit is the deferred gz.Close()'s own direct downstream writes, which bypass the wrapper entirely — the issue's "WriteHeader override closes both residual variants" did not hold for this one (proved RED) — so the middleware Dels in the deferred close before gz.Close(). Dead-map no-op on every committed response. Latent today; #134 mounts http.FileServer/ServeContent (which set Content-Length) behind this middleware. Part 2 — after a deliberate hijack (reachable via Unwrap since #167) the deferred gz.Close() fails with http.ErrHijacked; that is a takeover, not a truncation, so it is carved out of the "response likely truncated" log — the only server-side signal of the real corruption class stays trustworthy. Pins, all over real connections: WriteHeader and bodyless variants in the TestGzip_*StripsStaleContentLength family (real client gunzip of the wire bytes), hijack log silence behind a middleware-return barrier (client response and srv.Close cannot order the deferred Close on a hijacked conn). Closes #175. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…status close; honest HEAD (#175) The ErrHijacked close-log carve-out silenced real corruption: when handler output preceded the hijack (write buffered in flate, or a flushed prefix on the wire) the abandoned gzip stream left the client with unexpected EOF while the server stayed silent. The carve-out's "nothing was owed" premise only holds when nothing went through the gzip layer — gzipResponseWriter now latches wroteGzip in Write/FlushError (WriteHeader writes no gzip bytes and does not count), and a hijack after output logs the distinct "gzip stream abandoned by hijack after output" message. Folded-in scope (maintainer ruling 2026-06-07, detonates at #134): a committed bodyless status (204/304, stdlib bodyAllowedForStatus semantics) with nothing through the gzip layer made the deferred gz.Close fail with ErrBodyNotAllowed and the close log cry "truncated" falsely per conditional- GET hit. The middleware now skips the doomed empty-stream close writes when no stream started, and strips the eagerly-set Content-Encoding at the bodyless commit (stdlib writeNotModified precedent). HEAD with a bodyless handler gets the same skip so net/http no longer mints Content-Length: 23 from the empty stream — the HEAD makes no length claim, Content-Encoding stays to mirror the GET twin. New pins: loud hijack-after-write, loud hijack-after-flush, loud genuine close failure (the only test keeping the close log alive at all), quiet 204, quiet 304, HEAD no-length-claim, and the first-Write stale-CL strip e2e sibling that survives the #135 gateway deletion. Comments reworded to the two-shape truncation truth (overrun cut vs short close — gz.Close succeeds on the compressible shape, fully silent server-side). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ut (#175) Round-2 fixup: wroteGzip conflated "handler ATTEMPTED gzip output" with "gzip bytes REACHED the wire", and the quiet hijack arm ignored the status latch. A countingWriter now sits between gzip.Writer and the real ResponseWriter; every close-log claim keys on bytesOut (bytes net/http accepted) plus the status latch. - commit-then-hijack: Hijack flushes committed headers, so the client holds a gzip-advertising 200 with no stream — was silent, now its own loud line. Quiet only on pristine takeover (status==0 && bytesOut==0). - stray Write after a clean hijack: rejected wholesale, wire intact — was "client received a truncated body", now quiet (stdlib logs the misuse). - write/flush behind committed 204/304: zero bytes accepted, wire clean — was "(response likely truncated)" per #134 conditional-GET hit; flush now quiet, body write gets an accurate body-behind-bodyless line (wroteBody). - genuine close failure with bytesOut==0 no longer claims truncation. Comment precision: hijacker bytes land AFTER whatever net/http already flushed (never "exactly what the hijacker wrote"); expanding-shape overrun docs scoped to flate-buffered bodies (large incompressible ones get ErrContentLength from the handler's own Write); writeNotModified precedent scoped to 304 — the 204 CE strip is by analogy; the CE Del admits it takes a handler-set Content-Encoding too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…carve-out (#175, #181) The wire-truth hijack-verdict machinery (50e9fb6) had no caller — no handler hijacks today — and each review round surfaced deeper edge cases in it. Maintainer ruling 2026-06-07: descope it; the proper per-shape hijack faithfulness is tracked in #181, latent until a hijacking handler exists. Removed: - the countingWriter / bytesOut interposition between gzip.Writer and the real ResponseWriter (50e9fb6). - the wroteBody latch (existed only to distinguish hijack/bodyless shapes). - the three distinct hijack/bodyless loud lines — "committed and flushed before hijack…", "gzip stream abandoned by hijack after output…", and "gzip body written behind bodyless %d…" — and their pins (HijackAfterCommit, HijackAfterWrite, HijackAfterFlush, StrayWriteAfterCleanHijack, BodyWriteBehindNoContent). Replaced with one coarse, honest carve-out: on a deferred gz.Close() failure, errors.Is(err, http.ErrHijacked) suppresses the "likely truncated" log (a takeover is not a truncation). It is deliberately coarse — it cannot tell a clean takeover from a hijack that abandoned committed output; that precision is #181. One pin survives: a clean no-output hijack does not log "likely truncated" (HijackKeepsCloseLogQuiet). The Unwrap/Hijack doc now points to #181 for the faithfulness gap instead of claiming the middleware handles it. Kept untouched in behavior: the stale-Content-Length strips on every commit path (WriteHeader gated !informational, Write, FlushError, deferred-close Del) and their e2e pins; the bodyless-status (204/304) close-log quieting + CE strip and the honest HEAD no-length-claim, with their pins; the genuine-close-failure loud pin (the carve-out's narrowness guard, survived the simplification). Net -418 lines vs 50e9fb6. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Closes #175.
The fix (the #134 blocker):
gzipResponseWriterstripped a handler-set uncompressedContent-LengthonWrite/FlushErrorbut not on explicitWriteHeader— net/http latches the length there, committing the stale uncompressed length over the longer compressed stream (clientunexpected EOF, handler sees nil, only the deferred-Close log hints). A fourth variant (bodyless handler) commits via the deferredgz.Close()'s direct writes, bypassing the wrapper entirely. Now stripped on all four commit paths (WriteHeadergated!informationalto compose with #165). Latent today; #134'sServeContent/FileServermounts set Content-Length, so it goes live at cutover.Bodyless quieting (#134-relevant): a handler committing 204/304 behind gzip made the deferred close cry "response likely truncated" falsely on every conditional-GET cache hit; HEAD minted a bogus
Content-Length: 23(empty-gzip-stream length). Both fixed — bodyless/HEAD-no-output skip the doomed close writes, CE stripped on bodyless commit (304 analogy towriteNotModified).Hijack carve-out — deliberately coarse. A single
errors.Is(err, http.ErrHijacked)suppresses the truncation log (a takeover is not a truncation). It does not distinguish a clean takeover from one that abandoned committed output — that faithfulness is tracked in #181, latent until a hijacking handler exists (none today). An earlier branch built precise per-shape wire-truth logging; three adversarial review rounds kept finding deeper edge cases in a zero-caller path, so it was descoped (net −418 lines from that peak).Heavily reviewed: 10 review agents + 3 silent-failure re-hunts (two found real probe-reproduced holes, fixed; the descope ended the loop). All stdlib claims verified against go1.26.4. Merged latest develop (#172+#173); gate green incl.
-raceand a-count=5TestGzip flake check.🤖 Generated with Claude Code