Artifact e3adc369baca611196f754533c4e0fc779a2193db359c22e840c2fbb6b788a2d:

• File CHANGELOG.md — part of check-in [c892c7a622] at 2026-07-20 03:33:23 on branch trunk — fix: never parse a compressed body as cards (#104) (#111)

  * fix: never parse a compressed body as cards (#104)

  A clone served by libfossil failed in libfossil's own client with

  xfer: decode card 16303: xfer: empty line after split

  on a response that contained 202 cards. The index, the card, and the fault were all fictional.

  xfer.Decode tried three body framings in order -- raw zlib, the §4.1 4-byte-length container, then uncompressed card text -- and treated any decompression failure as "this is not that framing". decompressBounded failed a well-formed container purely because its decompressed length passed a 50 MB bound, so the fallback chain handed the still-compressed bytes to the card parser. The parser read compressed noise as cards for sixteen thousand iterations before one of them split into no fields, and reported that as the fault. Captured at offset 15,084,752 of the failing body, the "card" the parser was reading is DEFLATE output:

  "...\xd9\x34\x50\xe7\xcf\xa0\xf8\xd5\xd5\x87l\xff\t\xde\xfb\xe3..."

  Two changes. Once a body is recognized as a zlib stream, a failure to decode it is reported rather than reinterpreted -- oversize, truncated, or corrupt, the error now names the transport failure instead of an arbitrary card position. Both compressed framings are still attempted, since a length prefix can coincidentally parse as a zlib header, and uncompressed bodies still reach the card parser; only compressed ones no longer can.

  The bound itself was also wrong, and is the reason this was reachable at all. §4.1 places no limit on the decompressed size of the container; the 50 MB was a local memory guard that sat below what this implementation's own server emits in one clone round, so two libfossil peers could not clone from each other even though each could clone from fossil. It is now 64 MiB and coupled to sync.DefaultCloneBatchBytes by a compile-time guard that fails the build, rather than the clone, if the budget is raised past half of it.

  The defect predates the size-bounded clone batching of #100. #100 shrank rounds enough to hide it on the corpus that exposed it; it did not touch the decoder.

  Regression coverage, in two layers. internal/xfer pins the decode behaviour directly: an oversize container, a truncated one, a large in-bounds one, and an uncompressed body. internal/sync adds the configuration no existing test covered -- libfossil serving and libfossil cloning across a real HTTP encode/decode, on a corpus of multi-megabyte incompressible artifacts that crosses the server's per-round budget. Every prior clone test was either served by fossil or wired through MockTransport, which passes *xfer.Message values across untouched and never encodes a wire body at all. Artifact size is the other half of the reproduction: this failed on wapp, whose artifacts are large, and not on pikchr, whose artifacts are small.

  Verified against the wapp corpus: the round trip that failed now completes with 1038 of 1038 artifacts on both ends.

  * fix: make the #104 fixture reach the bound, and pin the bound from both sides

  Review found the flagship regression test passed against pre-fix code. It did. Its corpus was 21.6 MB spread over twelve artifacts, and pagination keeps every round near the 16 MB budget however large such a corpus grows, so the round never came near the 50 MiB bound it was supposed to cross. The 25-line rationale reasoned about crossing DefaultCloneBatchBytes, which is not the quantity that matters and never was; a permanent comment asserting a mechanism that does not exist is worse than no comment.

  The quantity that matters is one artifact. A round is charged against the budget before each artifact and sends whole the artifact that crosses it, so a round carries budget plus one entire artifact of unbounded size. A single artifact larger than bound-minus-budget is the only corpus that can exceed the bound, and it is also the one path by which a round can still exceed the current bound -- so the fixture guards the live risk and reproduces the historical one with the same bytes. It is now one incompressible artifact of 7/8ths of MaxDecompressedBytes, sized against the constant so it tracks it, behind testing.Short.

  Verified against a pre-fix tree with both test files dropped in:

  TestCloneSelfRoundTripOversizeArtifact FAIL Clone: handleFile aad935c2...: UUID mismatch for received file: expected aad935c2..., got 501b4814...

  which is the sharper end of this defect: pre-fix did not merely misreport, it accepted the compressed bytes as cards and produced an artifact whose content did not hash to its name.

  The cheap end-to-end round trip is kept as a separate test and described honestly as a coverage plank -- the first test anywhere here to have libfossil decode a body libfossil produced -- rather than as a guard on anything.

  Second finding, message.go: the claim that format 1 is the "Fossil HTTP sync wire format" is false. Every application/x-fossil body carries the §4.1 length prefix, request side and reply side, both through blob_compress. Format 1 matches nothing canonical emits and is kept as a tolerance, now labelled as one.

  Its safety was also load-bearing on a constant with no stated relationship to it. For a container of decompressed length N, prefix byte 0 is N>>24, and a zlib CMF requires CM==8 in the low nibble, so no container under 128 MiB can present a valid header at offset 0. Raising MaxDecompressedBytes past that point would leave decompressContainer's prefer-what-decodes ordering as the only remaining defense -- a narrower margin than this code was written against, and #98 is exactly the change that might want a different bound. That is now a build failure rather than a trap, matching the guard already on the other side. Verified to fire at 0x08000000 and to permit 0x07FFFFFF.

  Also: the MaxDecompressedBytes doc claimed to stop a hostile body from exhausting an embedding process's memory. It does not -- the transport reads the body whole before Decode sees it. It bounds expansion only, and now says so.

  Test hygiene: the server handshake polls for a live listener instead of sleeping 200ms and dropping ServeHTTP's error, and the 32 MiB and 64 MiB xfer allocations skip under -short like their sync counterpart.

  * fix: correct three claims about what the #104 guards cover

  The fix is unchanged. Three statements about it were wrong, and a comment that misstates a mechanism is worse than no comment because the next reader trusts it.

  The ordering claim was the serious one. decompressContainer was described as "preferring whichever framing actually decodes, so a spurious header match still loses to a real container". It does not. Format 1 returns immediately on success and format 2 is reached only when format 1 fails; the error comparison at the end selects which failure to report when both have already failed, and protects nothing when format 1 spuriously succeeds. Probed: a bare zlib body returns from format 1 with format 2 never attempted.

  That makes the aliasing guard weaker than its comment claimed, and in a direction neither review had reached: the aliasing byte is the top byte of the length the PEER declared, and this decoder never reads that field -- it slices past it and counts bytes actually inflated. No value of MaxDecompressedBytes constrains what a peer declares. A container declaring 0x081D0000 while carrying 46 bytes presents a parsing zlib header at offset 0, measured.

  What actually saves a real container in that case is the stream, not arithmetic: after two prefix bytes are eaten as a header, inflate must still parse the remaining two plus the real stream as DEFLATE and match its Adler-32. Measured `flate: corrupt input` at all three aliasing lengths probed, so format 1 fails, format 2 runs, and the container decodes correctly.

  The guard is kept because it states something true and useful -- Encode writes uint32(len(raw)) as the prefix, so holding the bound below the threshold means no body libfossil emits can carry an aliasing prefix -- but it is now documented as an invariant on our output, not a defence of our input, with the airtight alternatives named and left to their own issues.

  Second, "a single artifact larger than this bound cannot be cloned" is true but not the operative threshold. A round already holding bytes has that much less room, so the ceiling is the bound minus what the round accumulated first. Measured on this branch with a 58,720,256-byte artifact: 8, 12 and 15 MB of filler ahead of it all fail, and 17 MB passes because it exhausts the budget and pushes the artifact into a round of its own. Non-monotonic in the filler, which is worth knowing. That is #109, a defect this fix exposes rather than causes; the fixture's claim to guard it is withdrawn and it now says plainly that it covers the leading-artifact case only.

  Third, the changelog's "pin the bound from both sides" overclaimed. The guards pin the bound against the batch budget and against the aliasing threshold. Neither certifies a round.

  Rebased onto 8b6da9a. The internal/content -race failure reported last round was not a flake and not unrelated -- this branch predated #107, which rewrote available.go. TestIsAvailable_CyclicChainTerminates goes from exceeding its 30s budget to 0.06s across that rebase.

  ---------

  Co-authored-by: Dan Mestas <[email protected]> (user: [email protected], size: 9891) [annotate] [blame] [check-ins using]