Read an xz release in Go, so the build seat needs no xz tool (hq issue 345) #264
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: novox/mesh-controller#264
Reference in New Issue
Block a user
Symptom. mesh-catalog #240's build (plan-1791809737454479380, build-1791810262889408363) failed:
archive … betterbird-153.4.0esr-bb10.en-US.linux-x86_64.tar.xz: it is compressed with xz, and this build seat has no xz to read it.Cause. #255 read xz by running the
xztool and addedxztocmd/mesh-builder/Dockerfile. The build seat does not run that image: its image is built from the catalogue'smodules/build-agent/Dockerfile(with this repository as context), which installsdocker-cli gitalone. #255's rollout did rebuild build-agent, from that recipe, so the seat never had xz. My #255 report was wrong to say xz was in the seat's image.Fix. The xz stream is read in Go with
github.com/therootcompany/xzv1.0.1 (vendored; a port of xz-embedded, CC0), whoseNewReader(r, dictMax)refuses a dictionary over 64 MiB (xz's largest preset,-9) before allocating it (review 65). What it unpacks to is bounded as before (maxUpstream512 MiB,maxEntries100,000). No tool is run, so the exec path, the drain and thewaitgo. The xz line in the unusedcmd/mesh-builder/Dockerfileis taken back out.Block limit (review 68). The reader keeps one delta or BCJ decoder per block and frees them only at the end of a stream, so a file of many tiny blocks grows the heap with its length (about 67 MiB live for a 6 MB
xz --block-size=1 --deltafile). Before decoding,xzWithinLimits(internal/builder/xz_framing.go) now walks the file's framing without decompressing: stream header, each block header, the LZMA2 chunk headers (each carries its compressed size, so a chunk is skipped unread), padding, check, index, footer, stream padding. It refuses a file of more than 1,024 blocks or more than 16 concatenated streams, and an index whose block count is not the count walked.Tests.
TestAnXzReleaseIsReadWithoutAnyXzToolOnTheBuildSeat: a committed 252-byte.tar.xzfixture unpacked withPATHempty. Fails on main with the exact message above.TestAnXzStreamAskingForTooLargeADictionaryIsRefused(review 65).TestAnXzFileOfManyDeltaBlocksIsRefusedBeforeItIsDecoded: a 4.0 MB stream of one-byte delta blocks (about 202,000 blocks) whose index lists 1 block, built in Go by the test; refused with "more than 1024 xz blocks", and TotalAlloc grows less than 4 MiB. With the walk disabled it is decoded without error; the same file through the reader alone keeps 57 MiB live.TestTheXzBlockLimitIsExact(1,024 blocks pass, 1,025 refused),TestAnXzFileOfTooManyStreamsIsRefused(17 streams),TestAnXzIndexThatDoesNotListItsBlocksIsRefused.TestAnXzFileOfAFewDeltaBlocksAndTwoStreamsIsRead: the test's own builder makes streams thatxz -taccepts and that unpack to the right bytes (delta blocks; two streams with padding), so the walk follows the same framing the reader does.go vet ./...clean;sh merge-check.shwith a throwaway Postgres 17 (MESH_TEST_POSTGRES) passes with exit 0: gofmt,go test -raceon every package, every store test.After delivery. build-agent rebuilds from the catalogue's recipe with this source; then re-run the Betterbird build of mesh-catalog #240's merge.
A controller and build-agent rollout: merge only while
mesh-controller.planssays no plan is mid-gate. Part of issue 345. Not merged.Delivery
novox/mesh-controller@539a1c16cd4e— delivered since 2026-10-12T17:50:24ZDelivery plan — builds build-agent, mesh-controller, route-proxy → ace, g14, novox, shanks; no bus step
Machines
Transitions
The commit's note under
refs/notes/mesh-plankeeps every transition:git log --notes=mesh-plan.Merge check at
10f76cafmesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: build-agent, mesh-controller, route-proxy)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds build-agent, mesh-controller, route-proxy → ace, g14, novox, shanks; no bus step
Run by the build seat on novox as
build-1791810667206442075(builds --log build-1791810667206442075).Verdict: request changes at
10f76caBlocking
xz.ReaderConfig.DictCapis a floor, not a cap, so the 64 MiB limit does not bound memory. In the vendoredlzmafilter.go(lzmaFilter.reader), the stream's own dictionary size replaces the configured one whenever it is larger:if dc > config.DictCap { config.DictCap = dc }. I proved it with a probe test (not committed). I made a real .tar.xz withxz --lzma2=dict=1MiB, patched the dictionary code in its block header, recomputed the header CRC32 and passed it throughunpackUpstream:The stream is accepted and the dictionary is allocated in full, so the comment on
xzDictCap("a stream asking for more is refused") is not true. The digest pin limits who can supply such a file, but the code claims a limit it does not enforce, and a 4 GiB allocation on the build node is the failure the cap exists to prevent.Two ways to fix it:
xzDictCap. A stream can hold several blocks, so either walk the index at the end of the stream, or wrap the reader so each block header is checked as it arrives.github.com/therootcompany/xz(a port of xz-embedded, whoseNewReader(r, dictMax)refuses a larger dictionary).Either way, add a test that builds a stream asking for a dictionary above the cap (patched dictionary byte and recomputed CRC, as above) and expects a refusal that names the cap.
Checked and fine
go mod downloadfetched through the checksum database (h1:flR0y/x1hgM8EGV1AW3Xll6T413G0glV8UfBwR617V4=).GOFLAGS=-mod=mod go mod verifysays "all modules verified". All 48 vendored files are byte-identical to the module cache copy of v0.5.17.untar, so the 512 MiB unpacked limit and the 100,000-entry limit still apply. Removing the exec, drain and wait code is right.upstream_file.go,TestAnXzReleaseIsReadWithoutAnyXzToolOnTheBuildSeatfails with "it is compressed with xz, and this build seat has no xz to read it".merge-check.shwith a throwaway Postgres 17. gofmt, go vet andgo test -raceon everything else passed. One unrelated test failed once:TestAMachineJoiningIsAPeerOfTheHubUntilItsTokenExpireswith "no live token to issue for the tunnel key". It uses a 2-second token, so I think it is a timing flake under the full parallel race run. It passed 3 of 3 times run alone, and this pull request does not touch it.Merge check at
01fc9e1dmesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: build-agent, mesh-controller, route-proxy)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds build-agent, mesh-controller, route-proxy → ace, g14, novox, shanks; no bus step
Run by the build seat on ace as
build-1791812389482187478(builds --log build-1791812389482187478).Verdict: request changes at
01fc9e1The dictionary problem from review 65 is fixed. One allocation in the new library still grows with the input, and that is the reason for this verdict.
Blocking
The library keeps one filter decoder per block, so memory grows with the number of blocks. In the vendored
dec_stream.go, each block header that names a delta or BCJ filter takess.deltas[s.deltasUsed]/s.bcjs[s.bcjsUsed], or appends a new decoder, and then incrementsdeltasUsed/bcjsUsed. Those counters are reset only inxzDecReset, which runs once per stream, not once per block. A single stream with many small blocks therefore appends one decoder per block. xz-embedded in C reuses one decoder; this is a bug in the port.I measured it with a probe test (not committed). I made a 200 kB tar with real
xz --block-size=1:--delta=dist=1, 6.1 MB file: about 67 MiB live;--x86(BCJ), 6.1 MB file: about 28 MiB live.That is roughly 11 bytes of heap per compressed byte for delta. At the 512 MiB download cap it comes to about 5.6 GiB. The 512 MiB unpacked limit and the 100,000-entry limit do not catch it, because the unpacked output is tiny. It is the same failure as the dictionary one (a pinned file exhausting the build node's memory), so I hold it to the same bar.
Two ways to fix it:
bcjsUsedanddeltasUsedreset at each block header, or so a chain holds one decoder of each kind. Carry it as areplaceto a fork on the mesh's forge (vendored, with go.sum to match) and report it upstream.Add a test with a many-block delta stream that checks live heap stays under a bound.
Checked and fine
xzDecLZMA2Reset, beforemake, and the dictionary buffer is reused across blocks and streams up to the maximum.TestAnXzStreamAskingForTooLargeADictionaryIsRefused, run on10f76ca, fails (props 36 allocates 1,075,139,288 bytes, props 40 allocates 4,296,329,792 bytes, and neither is refused).go mod downloadthrough the checksum database (h1:CmOtsn1CbtmyYiusbfmhmkpAAETj0wBIH6kCYaX+xzw=).GOFLAGS=-mod=mod go mod verifysays "all modules verified". All 11 vendored files are byte-identical to the module cache copy of v1.0.1.unpackUpstreamwithwithin: betterbirdgives the same 55 files astar -xJ, and the packed archive is byte-identical to the one 10f76ca's decoder produced (sha256 74dd6b3b...bd7c).merge-check.shwith a throwaway Postgres 17 (gofmt, go vet,go test -raceon every package, every store test) passes with exit 0.Merge check at
539a1c16mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: build-agent, mesh-controller, route-proxy)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds build-agent, mesh-controller, route-proxy → ace, g14, novox, shanks; no bus step
Run by the build seat on ace as
build-1791814136071167014(builds --log build-1791814136071167014).Verdict: approve at
539a1c16cdRe-review against review 68. The per-block decoder growth is now refused before anything is decoded, and the walk that refuses it is itself bounded.
My probes. I reran my files through
xzWithinLimits:xz --block-size=1files (6.1 MB delta, 6.1 MB BCJ, 5.3 MB plain) are each refused with "more than 1024 xz blocks" in about 1 ms, with about 90 KiB allocated.unpackUpstreamon the delta file refuses it the same way.The walk cannot allocate or loop without bound:
full(n)with n at most 1,023 (the block header), 12, 4, 2 or 3 bytes; chunks are skipped withDiscard, which copies nothing.io.ErrUnexpectedEOF.xzFramingon fuzzed input for 90 s (532,905 runs, seeded with real multi-block, multi-stream and delta files) with no panic and no hang.The walk follows the reader's framing:
The new tests fail without the walk. With only the
xzWithinLimitscall removed, three tests fail:TestAnXzFileOfManyDeltaBlocksIsRefusedBeforeItIsDecoded(the 4 MB one-byte-block file is not refused);TestAnXzFileOfTooManyStreamsIsRefused;TestAnXzIndexThatDoesNotListItsBlocksIsRefused. This one passes untar without the walk, because untar stops at the tar end and never reads the index, so the walk is also the only index check this path has.Real Betterbird archive. Its sha256 is still 38dd24890a66b4cf9a45bc24e15192e00c26b929e1bb8df7974c16a4539129e2. The walk passes it in 84 ms (1 stream, 1 block). The unpacked files match
tar -xJ(55 files), and the packed archive is byte-identical to the one from01fc9e1and10f76ca(sha256 74dd6b3be3a8ff65a38fe483a410920b5715079f17455a208a2e076ff124bd7c).Why the gate builds route-proxy. route-proxy's manifest in the catalogue compiles
examples/route-proxyfrom novox/mesh-controller. This pull request changes go.mod, go.sum and vendor/modules.txt, which are build source for every Go program compiled from the repository.go list -deps ./examples/route-proxyshows it does not import internal/builder or the xz package. The rebuild is expected and harmless: it was already in the gate at10f76ca, the first head that touched go.mod, and the program's code does not change.Checks. My local
merge-check.shwith a throwaway Postgres 17 (gofmt, go vet,go test -raceon every package, every store test) exited 0. On the forge, mesh/merge-gate passed (builds build-agent, mesh-controller and route-proxy for ace, g14, novox and shanks) and mesh/repo-check passed, both at 14:15 UTC.Not blocking.
bcjsUsedanddeltasUsedare not reset per block.Merge check at
539a1c16Judged: head
539a1c16cd4emerged into trunkba685b17f5e1; read beside it: mesh-catalog23ea0f431faf, mesh-controller-mainba685b17f5e1, mesh-hosta1f9856b3957, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: build-agent, mesh-controller, route-proxy)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds build-agent, mesh-controller, route-proxy → ace, g14, novox, shanks; no bus step
Run by the build seat on g14 as
build-1791825663577192956(builds --log build-1791825663577192956).