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]
# Changelog All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] ### Added - `libfossil version` prints a single, stable, machine-parseable build identifier -- module version (or a `-ldflags -X github.com/danmestas/libfossil/cli.buildVersion=...` override for release builds), Go toolchain version, and platform -- and exits 0. Previously `version` was not a recognized command at all and fell through to the unrecognized-argument path. There is no global `--version` flag: `repo extract` already owns `--version` as a command-scoped flag (the source version to extract), and kong's global flags are visible in every subcommand's context, so a root-level `--version` would collide with it. - `libfossil repo serve` CLI command wires the already-public `Repo.ServeHTTP` xfer HTTP server into the command surface. A repository created or cloned with this tool can now be served to a peer -- including a stock `fossil clone` -- without writing Go. Binds to `127.0.0.1:8080` by default; override with `--addr host:port`. Process interrupt (Ctrl-C/SIGTERM) cancels the same context `ServeHTTP` already shuts down on; no new shutdown mechanism was introduced. ### Fixed - A clone from a libfossil server could fail with a card-syntax error (`decode card 16303: empty line after split`) naming a card the response never contained. The message decoder tried three body framings in order and treated any decompression failure as "not this framing", so a well-formed compressed body that exceeded the decoder's own size bound fell through to the uncompressed branch and the still-compressed bytes were parsed as card text — thousands of nonsense cards until one split into no fields. Worse than the misleading error, an oversize or truncated compressed body was *accepted*: the garbage cards decoded without error and the artifacts they carried failed their hash check downstream. A body recognizable as zlib is now decompressed or reported, never reparsed. The bound is also raised to 64 MiB, since it sat below what this implementation's own server emits in a single clone round — two libfossil peers could not reliably clone from each other even though each could clone from fossil. Two compile-time guards constrain the bound: it must clear the server's clone *batch budget* twice over, and it must stay below the size at which a length prefix this implementation writes could itself be read as a zlib header. Neither guard certifies a whole round — the budget bounds what precedes an artifact, not the artifact, so a large artifact that does not lead its round can still overrun the bound (#109). (#104) ### Changed - **Breaking:** `StatusOpts`, `MergeOpts`, and `CheckoutOpts` have been removed. No function anywhere accepted any of the three, and nothing constructed one: `Checkout.Status()` takes zero arguments, `Repo.Merge` takes positional string arguments, and `CheckoutOpts` had no construction site at all. A public type with no call site documents a capability that does not exist — a consumer reading `MergeOpts{Strategy: ...}` in the docs could reasonably conclude a strategy-selecting merge API exists; it never did. `Repo.Merge` and `Checkout.Status` keep their current signatures — an options-struct refactor for `Merge` was considered and declined. `CheckoutOpts.Force` was also a third same-named `Force` field in this package, alongside the already-removed `UpdateOpts.Force` and the real, fully-wired `ExtractOpts.Force`; removing it also removes that readability hazard. `ExtractOpts.Force` is unrelated and unaffected. - **Breaking:** `CloneOpts.ProjectCode` and `CloneOpts.ServerCode` have been removed. Neither was ever wired to anything — `Clone` accepted both fields but never forwarded them to the internal clone path, so setting either had no effect and gave no error. The internal clone options struct has no matching fields at all, so wiring them would mean designing semantics first (is a caller-supplied code a validation assertion or an identity override?) and no caller has asked for that. `CloneResult.ProjectCode` and `CloneResult.ServerCode` are unrelated and unaffected — they continue to report both, populated from the remote via the clone protocol negotiation, which is the only place they meaningfully originate. - **Breaking:** `UpdateOpts.Force` has been removed. It was never wired to anything — `Checkout.Update` accepted the field but never forwarded it to the internal update path, so setting it had no effect and gave no error. Deleting it is honest about what the API actually does; real forcing semantics for `Update` can be designed and added later as a new, deliberately-wired field if a caller needs them. `ExtractOpts.Force` is unrelated and unaffected — it is fully wired and unchanged. - **Breaking:** `Checkout.Update` now returns `(UpdateResult, error)` instead of a bare `error`. The internal 3-way merge already tracked which files were written, removed, and left with conflict markers; the old signature discarded all of it, so a caller could not tell a clean update from one that silently wrote `<<<<<<<` conflict markers into working-tree files. `UpdateResult.Conflicted` lists the paths that were merged but not cleanly — this is a successful update (`err == nil`), not an error; a genuine failure still returns a non-nil `error` with a zero-value `UpdateResult`. `Checkout.Extract` is unchanged. - **Breaking:** `Repo.Timeline` now enumerates the repository's `event` table newest-first (every event kind by default, or a single kind via `TimelineOpts.Type`), matching canonical `fossil timeline`. The previous behavior — a first-parent walk from a required start rid — is preserved under a new, honestly-named method, `Repo.Ancestry(LogOpts)`. The old `Repo.Timeline(LogOpts)` was actually an ancestry walk masquerading as an enumeration: it never visited a second parent or a sibling branch head, so it silently omitted any check-in that wasn't a first-parent ancestor of the given start rid. There is no deprecated shim; callers of the old `Timeline` should switch to `Ancestry` if they want the walk, or adopt the new `Timeline(TimelineOpts)` if they want a full enumeration. - `LogEntry` gains a `Kind` field (`EventKind`) identifying which of `event.type`'s six kinds (`ci`, `e`, `f`, `g`, `t`, `w`) an entry is. `Parents` is only populated for `Kind == EventKindCheckin`. - **Breaking:** `Repo.Timeline` orders by `(mtime DESC, rid DESC)`, a total order with rid as a true tie-break at exact mtime equality — a deliberate improvement over canonical fossil's bare `mtime DESC` with no tie-break, which can repeat or skip rows at a page boundary. Pagination uses a new opaque `Cursor` type: take one from a returned `LogEntry.Cursor` and pass it back as `TimelineOpts.After` to resume immediately after that entry. **`TimelineOpts.Before time.Time` and `TimelineOpts.After FslID` are deleted, not deprecated** — callers constructing either field will fail to compile. `Cursor`'s representation is intentionally hidden — it can only be obtained from a `LogEntry`, never built from a timestamp and a rid by hand, because a hand-built cursor derived from a rounded `time.Time` is not guaranteed to match its row exactly, which is what reintroduces skipped or duplicated rows at a page boundary in the first place. - **Breaking:** `LogEntry` no longer has a `Cursor` field, and `Repo.Timeline` now returns `[]TimelineEntry` instead of `[]LogEntry`. `LogEntry` is `Repo.Ancestry`'s result type; its cursor was always the zero value, which is structurally identical to a legitimate Timeline first-page call — so feeding an `Ancestry` entry's cursor into `TimelineOpts.After` silently paginated from page one forever with no error, and documenting the field as invalid on `Ancestry` entries did nothing to stop it. `TimelineEntry` (which embeds `LogEntry` and adds `Cursor Cursor`) is now `Repo.Timeline`'s result type, so the cursor lives only on the type that has one to give. Callers that read `entry.Cursor`, `entry.UUID`, `entry.User`, etc. off a `Repo.Timeline` result need no source change — those fields are still reachable through `TimelineEntry`'s embedded `LogEntry`. Callers that stored a `Repo.Timeline` result as `[]LogEntry` (rather than letting `:=` infer the type, or using `[]TimelineEntry`) will fail to compile. ## [0.1.0] - 2026-04-20 Initial open-source release of `libfossil`, a pure-Go implementation of the Fossil SCM that reads and writes the same `.fossil` SQLite repository format. ### Added - Repository lifecycle: create new repos and clone from existing ones. - Working-tree operations: checkout and checkin. - Timeline traversal over commits and events. - Merge and rebase primitives. - Diff and annotate (blame) over tracked content. - Manifest parsing and content-addressed blob storage. - Sync protocol client/server for pulling and pushing between repos. - Observer interfaces for sync and checkout, allowing external hooks into both network sync events and working-tree state transitions. - SQLite driver abstraction with support for both `modernc.org/sqlite` (pure Go) and `ncruces/go-sqlite3` (cgo-free, wasm-based) backends. - Deterministic simulation test harness with BUGGIFY-style fault injection for exercising concurrency and failure paths. - OpenTelemetry observer provided as a separate submodule to keep the core dependency footprint small. - `wasip1/wasm` build target for running `libfossil` under WASI runtimes.