fix(oci): declare the error envelope length net/http leaves off a chunked response
Why
WriteError wrote the envelope straight into the ResponseWriter and left the
length to net/http, which can only compute one for a response that fits
entirely in the 2 KiB it buffers ahead of the first flush
(bufferBeforeChunkingSize). A larger envelope has flushed before the handler
returned, so net/http no longer knows the total: the GET is framed chunked
with no Content-Length, and the HEAD carries neither header, leaving a
client nothing to size the response from.
That is reachable. newManifestReferenced renders one digest per parent index
into detail.parents, and nothing caps how many parents an image has, so the
409 MANIFEST_REFERENCED grows past the buffer at roughly 28 parents.
Declaring the length also settles a HEAD, which net/http sizes only when the
handler wrote bytes: its auto-length skips a HEAD whose handler wrote none,
because it cannot tell that from a handler that noticed the verb and wrote
nothing deliberately. RFC 9110 § 9.3.2 asks a HEAD for the header fields its
GET would carry.
All 61 non-test call sites in the package share the writer, so the fix is there rather than in any one arm.
Split out of the Step 15 branch because it changes every OCI route, hosted
included, and that is worth reviewing on its own rather than inside a 4,000-line
handler step. It targets main and depends on nothing.
What
It now encodes into a buffer, declares the count, and writes the buffered bytes. The bytes are the encoder's own, trailing newline included, so a small envelope's length is the number net/http would have computed and only a large one's is new.
An envelope that cannot be encoded is answered with 500 and an INTERNAL
envelope, not with the caller's status and an empty body. Either answer is one
the client did not ask for, and only this one carries an OCI error code the
client can key on: callers set the code on their wide event before calling, so a
body-less answer leaves the logs naming a code that never reached the wire, and
the log line is the first place an on-call engineer looks. The status moves with
the body because the envelope that named the caller's status is the one that
failed to encode. Nothing partly encoded goes out, since the buffer is reset
first, and the fallback needs no fallback of its own: NewInternalEnvelope(nil)
is a fixed code, a fixed message, and a nil detail map, and encoding/json
rejects none of those.
That arm is unreachable from inside this package, and the detail map's value
types are what make it so rather than their count. Every value a caller here
puts there is one encoding/json accepts without a marshaler of its own: a
string, an int or int64 (manifest_push.go's retry_after_seconds,
upload.go's expected_offset), or a []string (manifest_delete.go's
parents). ErrorEnvelope and its Detail map are exported, so a caller
outside the package supplying a channel, a func, or a cyclic structure is what
reaches it. The doc comment used to claim the branch was unreachable because
"every caller in this package fills with strings", which those four callers are
not.
The //nolint:errchkjson on the old discarded encode goes, because the primary
encode's error is checked now rather than thrown away. errchkjson is enabled
(linters.default: all) and reports unsafe type any found for a
Detail map[string]any whose encode error is dropped, so the fallback encode
carries one of its own with the reason on the directive line.
Verification
TestWriteError_DeclaresTheEnvelopeLengthdrives both verbs against a real server, becausehttptest.NewRecordermodels none of net/http's framing and would pass whatever the writer did. Its two subtests arean envelope past the buffer net/http cannot sizeandan envelope net/http would have sized itself; only the first fails without the fix, and the second is there to say the fix changed nothing where net/http already sized the response.- The declared value is checked against the
GET's own bytes, not only for presence and cross-verb agreement. Those two hold for any count a writer declares twice, so a length capped at the chunking buffer's size would satisfy them while every large envelope shipped truncated, which is the one case this writer exists for. Decoding the body is the other half: a length that matches a truncated response is caught by the JSON failing to parse (assertLengthSizesTheEnvelope). TestWriteError_DeclaresTheFallbackLengthOnAnUnencodableEnvelopeandTestWriteError_AnswersAnUnencodableEnvelopeWithAnInternalEnvelopecover the encode-failure arm frompackage oci_test, the only place it is reachable. The second passes a caller status of409, not500, because that is what separates the substituted status from one that happens to match.- Closed-header-set assertions gain
Content-Length:TestWriteRemoteReadError_CarriesOnlyTheServiceGeneratedHeaders, both cases ofTestWriteRemoteReadError_SetsNoStoreOnA501Only, andTestRemoteManifestServe_UnhealthyRemote_RefusalShape. The first and the last also assert the value is the countWriteErrortook off the envelope rather than anything relayed, and inTestRemoteListProxies_UnframeableUpstreamStatusAnswersUnavailablethe referrers arm flips from asserting no length to asserting the envelope's own. - Verified red against
mainat1e482e55e:go test -overlayswapping inmain'serrors.gofails all seven of those functions, and the subtestan envelope net/http would have sized itselfis the one that passes.
Comment blocks sit at main's caps
lint:comment-caps landed on main after this branch was cut, and the last
commit compresses this MR's comments to the caps it enforces: 3 lines on an
exported doc comment, 1 on an unexported one, 2 anywhere in a _test.go file.
Three claims the code can no longer hold are above instead. net/http's pre-flush
buffer and the detail.parents growth that crosses it are under Why, the
value types that leave the encode-failure arm reachable only from outside the
package are under What, and why the declared value is asserted against the
body's own bytes is under Verification.
writeRemoteReadError also sets X-Content-Type-Options before the 501 branch
now. An unexported doc comment and the block at the head of its body share that
same 1-line cap, so the why for that call rides the call line as a trailing
comment, where it costs nothing, and the RFC 9110 § 15.6.2 rationale for the
501's Cache-Control sits below the first statement.
e2e scenarios
docs/testing/e2e/oci.md gains e2e.oci.consume.error-envelope-declares-its-length
under Consume: every OCI error response declares Content-Length, and a HEAD's
value equals its GET's, including for an envelope large enough to be framed
chunked.
Please read the conformance job
This changes the error-envelope response for every hosted OCI route the
opencontainers/distribution-spec suite exercises. mise run conformance would
not run locally, because it needs /etc/labkit and /secrets writable and both
are root-owned on my machine, so conformance:oci:s3-garage is the check that
matters here.
Overlap with the S16 Step 14 stack
!1872 (merged) has merged, and this branch is rebased onto it. !1873 (merged) and !1875 are still
open and both now touch remote_serve.go and remote_manifest_test.go, which
this MR touches too, but no hunk of either sits in the regions this MR changes:
writeRemoteReadError and the closed-set assertion in
TestRemoteManifestServe_UnhealthyRemote_RefusalShape. Measured against their
diffs on 2026-08-26, so a rebase on their side can move it. This MR can merge in
any order relative to them.
Related to #288