Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions rest/cachecontrol.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ func cacheControl(maxAgeSeconds int, next http.Handler) http.Handler {
}

// cacheControlWriter injects the Cache-Control header at commit time — the
// first WriteHeader, Write, or flush — and only when the response is a 200.
// first latching WriteHeader (non-1xx, or the 101 carve-out — see the
// informational predicate), Write, or flush — and only when the
// response is a 200. A forwarded informational WriteHeader (1xx minus 101 —
// rationale on the informational predicate) commits nothing (#165): the
// stamp decision belongs to the final status that follows, exactly as
// net/http's own writer leaves that set uncommitted (101 latches, there and
// here).
// Commit time is the only safe moment: setting the header eagerly would leak
// it onto error responses written later (writeError never clears headers),
// and onto the contract 500 the sentry layer writes after a handler panic.
Expand All @@ -75,6 +81,13 @@ type cacheControlWriter struct {
}

func (w *cacheControlWriter) WriteHeader(code int) {
if informational(code) {
// A non-latching 1xx (the predicate excludes 101) never commits —
// forward and keep the stamp decision for the final status (#165;
// rationale on the informational predicate).
w.ResponseWriter.WriteHeader(code)
return
}
if !w.committed {
w.committed = true
if code == http.StatusOK {
Expand Down Expand Up @@ -104,8 +117,9 @@ func (w *cacheControlWriter) Write(b []byte) (int, error) {
// made is rolled back. Leaving it would poison the live header map and lie
// committed=true: a handler reacting to the failed flush by writing an
// error would commit a non-200 carrying max-age, the exact leak class the
// type doc forbids. The rollback's premise — nothing reached the wire, so
// nothing committed — holds for wrappers that fail before writing anything:
// type doc forbids. The rollback's premise — nothing of the final response
// reached the wire, so nothing committed — holds for wrappers that fail
// before writing anything of it:
// an unflushable wrapper ABOVE gzip, or a chain with no flushable bottom and
// no gzip in between. It is NOT universal: gzip's FlushError (rest/gzip.go)
// pushes its header and sync block downstream BEFORE the delegated flush can
Expand Down
15 changes: 15 additions & 0 deletions rest/cachecontrol_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@ func TestCacheControlWriter_CommitDecision(t *testing.T) {
assert.Empty(t, rr.Header().Get("Cache-Control"))
})

// A non-latching 1xx WriteHeader (the predicate carves out 101) latches
// nothing (#165) — and in particular
// must not stamp: a stamp on the 1xx would sit in the live map and leak
// onto whatever final status follows, here a 500 (the leak class the type
// doc forbids). The stamp-on-the-real-200 direction needs real 1xx wire
// machinery the recorder lacks — informational_internal_test.go's shared
// table covers it on a real server.
t.Run("WriteHeader 103 leaves the decision to the final status", func(t *testing.T) {
w, rr := wrap()
w.WriteHeader(http.StatusEarlyHints)
w.WriteHeader(http.StatusInternalServerError)
assert.Empty(t, rr.Header().Get("Cache-Control"),
"a 500 after a forwarded 103 must not carry the freshness signal")
})

t.Run("second WriteHeader cannot change the first decision", func(t *testing.T) {
w, rr := wrap()
w.WriteHeader(http.StatusInternalServerError)
Expand Down
6 changes: 4 additions & 2 deletions rest/cachecontrol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ func captureHeader(h http.Handler, last *http.Header) http.Handler {
// commitSnapshot clones the header map at commit time — the first
// WriteHeader, Write, or flush (FlushError, for symmetry with the production
// writers) — which is exactly when a real server snapshots headers onto the
// wire. Anything set afterwards is invisible to clients and must stay
// invisible to the battery.
// FINAL response (a first non-latching 1xx WriteHeader is exactly when it
// does NOT — no route in this battery emits any 1xx, so the helper keeps the
// simple first-call rule). Anything set afterwards is invisible to clients and
// must stay invisible to the battery.
type commitSnapshot struct {
http.ResponseWriter
last *http.Header
Expand Down
8 changes: 6 additions & 2 deletions rest/gzip.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import (
//
// Exported because the legacy gateway chain reuses it until cutover deletes
// that stack.
//
// A handler-sent 1xx on a gzip-negotiated request carries Content-Encoding:
// gzip in the interim response — the stdlib sends the live header map per
// RFC 8297 — pre-existing, adjacent to the holes tracked in #175.
func GzipMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") {
Expand Down Expand Up @@ -64,8 +68,8 @@ func (w *gzipResponseWriter) FlushError() error {
// uncompressed Content-Length must go here too — same staleness Write
// handles above; left in place it truncates the compressed stream. An
// explicit WriteHeader still commits it (net/http latches the length at
// WriteHeader) — pre-existing hole, reachable on develop without any
// flush, tracked separately.
// a final WriteHeader) — pre-existing hole, reachable on develop without
// any flush, tracked separately.
w.Header().Del("Content-Length")
if err := w.Writer.Flush(); err != nil {
return err
Expand Down
37 changes: 37 additions & 0 deletions rest/informational.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package rest

import "net/http"

// informational reports whether code is an informational status that
// net/http forwards WITHOUT committing the response.
//
// That set is 1xx minus 101: net/http's own response writer routes
// `code >= 100 && code <= 199 && code != StatusSwitchingProtocols` through
// its informational path, leaving its wroteHeader latch false — but on a 101
// it SETS wroteHeader and commits ("We shouldn't send any further headers
// after 101", per the stdlib comment), dropping everything after from the
// wire — wire-silent only: the server logs each superfluous WriteHeader, and
// Write returns http.ErrBodyNotAllowed.
// This predicate mirrors that exactly, 101 carve-out included: excluding 101
// here means the wrappers latch on it just as the stdlib does, so e.g. a
// post-101 panic re-panics instead of "writing" a contract 500 the stdlib
// would swallow. Every writer wrapper in the new-stack rest chain with
// commit-time state (statusWriter's status capture, commitWriter's commit
// latch, cacheControlWriter's header stamp) must mirror that: forward a
// non-latching 1xx WriteHeader to the delegate and latch nothing, so the
// subsequent final WriteHeader behaves exactly as a first call (#165). The
// legacy gateway's statusRecorder still latches on all 1xx — known, tracked
// in #176, dies at cutover. A new rest-chain wrapper with WriteHeader state
// starts here — and joins the shared table in informational_internal_test.go.
//
// (HTTP/2 would treat all 1xx informationally, but RFC 9113 removes 101 from
// HTTP/2 entirely, and this server is plain HTTP/1.1 — latching on 101 is
// safe on both.)
//
// No handler calls WriteHeader with a 1xx today (net/http's automatic
// Expect:100-continue reply bypasses the wrapper chain entirely); the guard
// exists so the wrappers stay faithful to net/http's commit semantics rather
// than to current traffic.
func informational(code int) bool {
return code >= 100 && code <= 199 && code != http.StatusSwitchingProtocols
}
Loading