✨ feat(fixtures): pin the content-encoding decode policy to a shared corpus - #295
Conversation
|
| Filename | Overview |
|---|---|
| pkg/capture/contentencoding_corpus_test.go | Adds the corpus loader, recipe builder, reference oracle, digest verification, and named bidirectional policy-coverage gate; the previously broad coverage checks are now tied to dedicated cases. |
| fixtures/content-encoding/cases/gzip-concatenated-members.json | Pins complete decoding of concatenated gzip members using independently compressed plaintext chunks. |
| fixtures/content-encoding/cases/zstd-concatenated-frames.json | Independently pins complete decoding of concatenated zstd frames. |
| fixtures/content-encoding/cases/salvage-refused-when-nothing-produced.json | Uses a format-derived absolute prefix instead of a compressor-relative truncation ratio, addressing the prior portability concern. |
| fixtures/content-encoding/README.md | Documents the language-neutral fixture schema, member construction, decode outcomes, failure taxonomy, sealing, and coverage contracts. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
C[Shared JSON corpus] --> B[Build body recipe]
B --> G[Go DecodeContentEncoding oracle]
B --> R[Rust client decoder]
G --> P[Assert shared decode policy]
R --> P
C --> D[DIGEST freshness seal]
C --> V[Named policy coverage gate]
Reviews (3): Last reviewed commit: "✨ feat(fixtures): pin that a concatenate..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Deploying docs-tapes-dev with
|
| Latest commit: |
35c79be
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9255f71a.tapes-dev.pages.dev |
| Branch Preview URL: | https://jason-capture-policy-fixture.tapes-dev.pages.dev |
|
@greptileai review Both findings addressed. Salvage case portability. The truncation is now an absolute Coverage predicates. The gate is now a declared table of {rule, the case name that pins it, predicate}, with the predicate run against that named case rather than searched across the corpus. All three failure modes verified: deleting a pinning case, editing it so it no longer pins its rule, and adding a case no rule names — the last closes the table in the other direction so it cannot fall behind the corpus. Worth recording on your DIGEST question: the seal alone would not have caught a deletion. It is recomputed from whatever cases exist, so delete-and-reseal is a legal two-line green diff. It is the named-case gate, not the seal, that stops that. |
|
@greptileai review One more case pair on this branch, from a defect found reviewing the client that consumes this corpus. Its gzip binding stopped after the first member of a concatenated stream and reported success, while
The body spec gained a |
…corpus The envelope and thread corpora make *header* contracts executable. This does the same one layer in, for a *policy*: which content-codings a captured body may use, how stacked layers compose, how much output is allowed, and what a corrupt or half-arrived stream is worth. That policy is implemented independently in Go (pkg/capture/contentencoding.go, the reference) and in Rust (tapesctl's client decoder). Their agreement was a point-in-time claim by whoever last read both files, and it had already decayed once: the client dropped every zstd request body — all of Codex/pi's traffic — while the gateway route decoded the same bytes fine, with nothing red. Bodies are recipes (a plaintext, the layers to apply, an optional truncation) rather than committed bytes. Compressed output is not stable across implementations, so pinning it would assert compressor identity instead of the policy, and would make review a diff of base64 blobs. Byte-exact base64 is kept only where the bytes are the assertion: two hand-built zstd frames differing solely in their window descriptor, two deliberately corrupt frames, and the empty body. Three cases record a decision rather than a settled rule, in a `contested` block that travels with the case into every vendored copy: - the zstd bound is asserted as the window only. Go also passes a max-memory bound and the Rust binding cannot; with the window already at the output cap, no constructible frame distinguishes them, so a case for max-memory would be asserting a language binding's API rather than the policy. - a bodiless request whose headers still claim an encoding errors at the decoder, and must never reach it. Not-calling-the-decoder is recorded as a caller precondition, which a proxy that returns early satisfies. - the same empty body under zstd currently succeeds with zero bytes. That is recorded as observed and flagged as a suspected bug rather than promoted to a rule — encoding the intended answer would make the case fail on the very implementation it describes. The failure taxonomy is three classes because both implementations already distinguish exactly three; corrupt-body and window-refused stay merged under `undecodable`, with the difference in unasserted prose, rather than inventing a distinction neither side carries. This is not extproc's DropResponseDecode metrics label, which still collapses all three and is untouched. related to PCC-1126
… produced output The case that pins the second half of the salvage rule — a stream that ended early AND produced nothing is refused, not salvaged — truncated its zstd body at half its encoded length. Whether a complete block lands inside that half is a property of the compressor, not of the decoder: its block splitting and its level decide it. A conformant decoder handed another compressor's stream could legitimately produce output there, correctly report a salvage, and fail a corpus whose whole premise is that it asserts policy rather than compressor identity. The cut is now an absolute prefix derived from the container format instead: 9 bytes, being the 4-byte magic plus the smallest frame header the format allows plus a block header. Byte 10 is the earliest position at which any encoder's first byte of block content can appear, and a block that has delivered no content can have produced no output whatever its type — so "nothing was producible" is true for every encoder rather than for the one that built the fixture. A larger frame header only pushes that byte further out. The error is unchanged, including its unasserted detail, because an unfinished frame reports the same early end wherever it was cut. That needs a truncation form the schema did not have. drop_tail_bytes fixes what is missing and keep_head_ratio fixes a fraction; neither can express a cut point that comes from the format rather than from one encoder's output length, so keep_head_bytes is added beside them and the three are documented by what each holds fixed across compressors. The coverage gate is rewritten in the same pass, for a failure of the same shape. It asserted properties over the whole corpus — "some case decodes gzip" — which any neighbour sharing the shape satisfies: the alias case, a cap case, a stacked case. Deleting the dedicated case therefore left the gate green while the rule it pinned was pinned nowhere, and the seal does not catch that either, since a digest recomputed from the surviving cases makes delete-and-reseal a legal two-line diff. Each rule now names the case that pins it, and the predicate runs against that case, so a deletion fails and so does gutting the case in place. The table is closed in the other direction too: a case no rule names fails, which keeps it from falling behind the corpus it describes. related to PCC-1126
…st member A gzip stream is a series of members (RFC 1952 2.2) and zstd frames may likewise be concatenated. A streaming compressor that flushes and restarts mid-body emits exactly that shape, so it is ordinary request traffic rather than an edge case. It is worth a case because of how it fails. A decoder that stops at the first member returns *success*, reports no truncation, and hands back a prefix, so the capture is silently short and the loss reappears far downstream as a parse error with nothing pointing at the decoder. Every other way these decoders can lose bytes is loud; this one is not. The two implementations of this policy did not agree on it. Go's compress/gzip reader continues into later members unless Multistream(false) is set; the Rust client's flate2::read::GzDecoder stops at the first, and only MultiGzDecoder continues. Both sides already read every zstd frame, which is pinned here too rather than left implicit: the two codings go through unrelated libraries, so agreement on one is not evidence about the other, and an unpinned rule is one a binding swap can quietly drop. Expressing this needed a `members` count in the build recipe, alongside `truncate`. It splits the *plaintext* rather than the encoded stream, so the member boundary sits at the same logical offset whichever compressor built the fixture and the case can still assert equality with the whole plaintext -- an assertion a split of the encoded bytes could not make, since it would be cutting at an offset only one encoder's output has. related to PCC-1126
2c4607c to
35c79be
Compare
1 participant
Capture policy has two implementations — the Go one here, and a Rust one in the client.
Their agreement has been a point-in-time claim by whoever last read both files, and that
decays silently. It decayed: the client dropped every compressed request body while this
path decoded it fine, and nothing failed until someone ran a session and noticed the rows
were missing.
This adds a shared corpus for content-encoding decode, so the two implementations are
gated against the same cases instead of against each other's source. It generalises the
pattern already used for the envelope fixtures.
Format
Cases specify a body either as a recipe — a plaintext, a list of encodings to apply,
an optional truncation — or as literal base64 where the bytes are the assertion.
Recipes are the default because compressed output is not stable across implementations:
Go's
compress/gzipand Rust'sflate2do not emit identical bytes, and neither do thetwo zstd libraries. Pinning compressed bytes would assert compressor identity, which is
not the policy and which no second implementation could satisfy. The cap cases decode to
32 MiB, which is not committable either way.
25 cases
identity (5) · supported codings (4) · stacked layers (2) · salvage (4) · limits (4) ·
errors (6)
Three questions the corpus forced into the open
zstd bounding. Two 14-byte frames differing only in the window descriptor show the
window bound is observable from outside the decoder, and both implementations place it at
log2(32 MiB). That agreement is now a gate. Maximum-memory is asserted nowhere — with the
window already at the output cap, no constructible frame distinguishes a decoder that sets
it, so a case would assert a library's API surface rather than the policy.
Empty bodies. The case pins the decoder. "Don't call the decoder for a bodiless
request" is recorded as a caller precondition that no decoder test asserts, which makes an
early return in a caller conformant rather than divergent.
Failure classes. Three — unsupported, oversize, undecodable — because both
implementations already distinguish exactly three. Corrupt input and a refused window stay
merged, with the difference in an explicitly unasserted field. Splitting them is a change
to both implementations first and the corpus second.
It found something immediately
DecodeContentEncoding([]byte{}, "gzip")errors while([]byte{}, "zstd")returns successwith zero bytes — identical input, opposite outcomes, decided by which decoder the header
named. That is documented here as a case rather than quietly fixed, and repaired in the
follow-up.
Gates
An oracle over every case, a digest seal so a stale vendored copy fails, and coverage rules
so deleting the last case for a behaviour fails loudly. All three are mutation-tested.
Two failures on a full
go test ./...reproduce on a clean tree and are unrelated: theOpenAPI contract seal, and the Postgres suite, which needs the database
make testprovisions.
related to PCC-1126