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_DeclaresTheEnvelopeLength drives both verbs against a real server, because httptest.NewRecorder models none of net/http's framing and would pass whatever the writer did. Its two subtests are an envelope past the buffer net/http cannot size and an 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_DeclaresTheFallbackLengthOnAnUnencodableEnvelope and TestWriteError_AnswersAnUnencodableEnvelopeWithAnInternalEnvelope cover the encode-failure arm from package oci_test, the only place it is reachable. The second passes a caller status of 409, not 500, 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 of TestWriteRemoteReadError_SetsNoStoreOnA501Only, and TestRemoteManifestServe_UnhealthyRemote_RefusalShape. The first and the last also assert the value is the count WriteError took off the envelope rather than anything relayed, and in TestRemoteListProxies_UnframeableUpstreamStatusAnswersUnavailable the referrers arm flips from asserting no length to asserting the envelope's own.
  • Verified red against main at 1e482e55e: go test -overlay swapping in main's errors.go fails all seven of those functions, and the subtest an envelope net/http would have sized itself is 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

🤖 Generated with Claude Code

Edited by Sylvia Shen

Merge request reports

Loading
Loading