Advertise Flusher and Hijacker only when the underlying writer has them

[paskal]

Follow-up to #51, two loose ends from reviewing that branch after it merged.

The wrapper advertised capabilities it did not have. gzipResponseWriter implemented Flush and Hijack unconditionally, so a handler's w.(http.Flusher) assertion succeeded regardless of what sat underneath, and the flush that followed silently did nothing when the writer beneath offered no Flush.

This is reachable with two middlewares from this repo: Timeout wraps with http.TimeoutHandler, whose writer implements neither Flusher nor Hijacker, as timeout.go:26 already notes. Composing Timeout with Gzip gave a handler a Flusher that did nothing at all.

The returned writer is now selected from the capabilities of the one it wraps, four small variants over the same core. Unwrap stays on the core, so http.ResponseController keeps working in every case. The new test asserts the advertised set matches, both for a plain server writer and behind Timeout, and fails on the previous code.

Hijack abandoned an open gzip stream. Anything the handler had already written stayed in the compressor and never reached the wire. The stream is finished before the connection changes hands.

Documentation, not a code change: sniffing forces the commit to be deferred, so when a handler calls WriteHeader without a Content-Type, headers it changes before the first Write still reach the client, where net/http would ignore them. That is a deliberate trade for being able to compress handlers that never set a content type. It is now written down on the type and in the README rather than left for the next reader to discover. Handlers that mutate headers after WriteHeader are relying on a no-op, so the deviation is permissive rather than corrupting.

---

Also picked up the 101 gap umputun reported on #51, since it lives in the same WriteHeader/Hijack path this PR already touches.

WriteHeader only committed once a Content-Type was known, so the plain upgrade sequence — WriteHeader(101) with no content type, then Hijack — recorded statusSet and status and never forwarded them. Hijack set hijacked, close returned at that guard, and the client received the raw protocol bytes with no status line at all. The existing TestGzip_SwitchingProtocols passed because it sets a content type.

101 now commits immediately. It can, because it carries no body to sniff and decide already excludes it from compression. The regression test omits Content-Type and reads the raw socket rather than going through the HTTP client, so it sees what actually reaches the wire; without the fix it gets raw-protocol-bytes and no 101 Switching Protocols.

Narrow in practice, as noted — gorilla/websocket writes the 101 itself after hijacking rather than calling WriteHeader — but it was a real difference from net/http.

[coveralls]

Coverage Report for CI Build 32198654843

Coverage decreased (-0.7%) to 96.936%

Details

• Coverage decreased (-0.7%) from the base build.
• Patch coverage: 10 uncovered changes across 1 file (17 of 27 lines covered, 62.96%).
• 2 coverage regressions across 1 file.

Uncovered Changes

File	Changed	Covered	%
gzip.go	27	17	62.96%

Coverage Regressions

2 previously-covered lines in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
benchmarks.go	2	97.98%

---

Coverage Stats

Relevant Lines:	1697
Covered Lines:	1645
Line Coverage:	96.94%
Coverage Strength:	47.37 hits per line

---

💛 - Coveralls

[umputun]

the capability selection is right, and thanks for folding the 101 fix in here, TestGzipSwitchingProtocolsWithoutContentType covers the sequence I described.

one thing left on the error path in hijack, gzip.go:180-189. The gzip writer is closed, pooled and nilled before h.Hijack() runs, and Close's error is discarded:

if w.gz != nil {
    _ = w.gz.Close()
    gzPool.Put(w.gz)
    w.gz = nil
}
conn, rw, err := h.Hijack()

two consequences. A failing Close means truncated gzip output, and a subsequent successful Hijack hides it. And if Hijack fails without taking the connection, hijacked stays false while w.gz is already nil, so a handler that carries on writing goes down the w.ResponseWriter.Write path and appends raw bytes under the Content-Encoding: gzip this response already advertises.

safer ordering is close first and return the error, then hijack, and only put and nil the writer once Hijack has actually succeeded. Leaving the closed writer attached on the failure path means a continued Write errors rather than corrupting the body, and the deferred close still returns it to the pool.

narrow either way, Hijack failing after the type assertion succeeded is mostly a double-hijack, so no rush.

[paskal]

Fixed in #56, both consequences confirmed on the current code before changing it.

The wrapped Close error is returned now, and on a failed Hijack the writer stays attached rather than being released, so a continued write fails with flate: closed writer instead of appending raw bytes under Content-Encoding: gzip. I checked that a closed gzip.Writer really does error and leaks nothing, since that is what makes the failure path safe rather than merely different. The writer is released only once Hijack has taken the connection; the deferred close still pools it otherwise, which is fine because Reset clears the closed flag and a second Close returns nil.

Three test cases, the first two of which fail without the change.