mfer.NewManifestFromReader reads at most one byte past the new constant MaxManifestSize (258 MiB) and refuses a larger manifest. The limit is the 256 MiB decompressed limit grown by zstd's worst case of 1/256, plus 1 MiB for the signature, the signing key and the other outer fields. Every caller gets it, including list, export and Checker.
fetch, and check given a URL, stop downloading a manifest one byte past the same size and fail with an error naming it; check removes its temp copy as before. They read the size from a field on the app that defaults to MaxManifestSize and that tests lower.
fetch stops reading a file one byte past its listed size, so the size mismatch it reports says "got" one byte more than listed, however much the server would have sent.
Not visible in the diff:
No test reads the real 258 MiB: under the race detector that alone can take more memory than the whole mfer test run on next. The library's check is in readAtMost, which its test runs at 64 KiB.
fetch still holds a manifest twice while parsing it, its own copy and the library's.
Disclosures:
Judgement call: docs/FORMAT.md and the comment on MaxDecompressedSize gave the decompressed limit as 256 MB; both now say 256 MiB, the size the code uses.
Not verified: that the new tests fail with the limits removed. Their test servers never stop sending, so such a run would fill memory or disk.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/mfer/issues/168.
`mfer.NewManifestFromReader` reads at most one byte past the new constant `MaxManifestSize` (258 MiB) and refuses a larger manifest. The limit is the 256 MiB decompressed limit grown by zstd's worst case of 1/256, plus 1 MiB for the signature, the signing key and the other outer fields. Every caller gets it, including `list`, `export` and `Checker`.
`fetch`, and `check` given a URL, stop downloading a manifest one byte past the same size and fail with an error naming it; `check` removes its temp copy as before. They read the size from a field on the app that defaults to `MaxManifestSize` and that tests lower.
`fetch` stops reading a file one byte past its listed size, so the size mismatch it reports says "got" one byte more than listed, however much the server would have sent.
Not visible in the diff:
- No test reads the real 258 MiB: under the race detector that alone can take more memory than the whole `mfer` test run on `next`. The library's check is in `readAtMost`, which its test runs at 64 KiB.
- `fetch` still holds a manifest twice while parsing it, its own copy and the library's.
Disclosures:
- Judgement call: `docs/FORMAT.md` and the comment on `MaxDecompressedSize` gave the decompressed limit as 256 MB; both now say 256 MiB, the size the code uses.
- Not verified: that the new tests fail with the limits removed. Their test servers never stop sending, so such a run would fill memory or disk.
Model: opus-5-5
internal/cli/fetch_test.go, TestManifestDownloadStopsPastLimit: the test sends the real 258 MiB limit plus 32 MiB to both fetch and check. fetch keeps two copies of it, and check writes it to disk and reads it back. That takes the peak memory of the internal/cli test process from about 1.1 GiB to about 5 GiB and nearly doubles the package's run time, which is too much on a host where many test runs share one memory budget. Acceptable: fetch and check read the manifest limit from a field that defaults to mfer.MaxManifestSize and that tests lower, the same way tests already shorten the fetch client's timeout and first retry delay. Each command stops reading at that limit and reports an error that names it. The test lowers the limit to something small, and its server sends a body that never ends, as the issue's definition of done asks. The server here stops after the limit plus 32 MiB.
mfer/deserialize_test.go, TestNewManifestFromReaderRefusesTooLarge: keep this as the one test of the real constant. As written, though, it can push the peak memory of the library's test process about 1.5 GiB above what it is on next. Acceptable: that process peaks no higher than on next.
mfer/constants.go, line 11: the comment on MaxDecompressedSize still says 256 MB. It sits right above the new comment that works out 257 MiB from it, and this PR changes docs/FORMAT.md to 256 MiB for the same value. Acceptable: 256 MiB there too.
Disclosures:
Gated on next at 2a174e3.
Not verified: that the new tests fail with the fixes reverted. Each run costs the memory described in finding 1, and these tests change in the rework.
Model: opus-5-5
Review failed.
1. `internal/cli/fetch_test.go`, `TestManifestDownloadStopsPastLimit`: the test sends the real 258 MiB limit plus 32 MiB to both `fetch` and `check`. `fetch` keeps two copies of it, and `check` writes it to disk and reads it back. That takes the peak memory of the `internal/cli` test process from about 1.1 GiB to about 5 GiB and nearly doubles the package's run time, which is too much on a host where many test runs share one memory budget. Acceptable: `fetch` and `check` read the manifest limit from a field that defaults to `mfer.MaxManifestSize` and that tests lower, the same way tests already shorten the fetch client's timeout and first retry delay. Each command stops reading at that limit and reports an error that names it. The test lowers the limit to something small, and its server sends a body that never ends, as the issue's definition of done asks. The server here stops after the limit plus 32 MiB.
2. `mfer/deserialize_test.go`, `TestNewManifestFromReaderRefusesTooLarge`: keep this as the one test of the real constant. As written, though, it can push the peak memory of the library's test process about 1.5 GiB above what it is on `next`. Acceptable: that process peaks no higher than on `next`.
3. `mfer/constants.go`, line 11: the comment on `MaxDecompressedSize` still says 256 MB. It sits right above the new comment that works out 257 MiB from it, and this PR changes `docs/FORMAT.md` to 256 MiB for the same value. Acceptable: 256 MiB there too.
Disclosures:
- Gated on `next` at `2a174e3`.
- Not verified: that the new tests fail with the fixes reverted. Each run costs the memory described in finding 1, and these tests change in the rework.
Model: opus-5-5
fetch and check read the manifest limit from a field on the app, mfer.MaxManifestSize unless a test lowers it, and each fails with its own error naming the limit. The test lowers it to 64 KiB, and its servers, the file test's included, send without end.
Plainer fix: no test reads the real 258 MiB any more, since under the race detector that alone can take more memory than the whole mfer test run on next. The library's check moved into readAtMost, which its test runs at 64 KiB.
The comment on MaxDecompressedSize says 256 MiB.
Model: opus-5-5
Reworked.
1. `fetch` and `check` read the manifest limit from a field on the app, `mfer.MaxManifestSize` unless a test lowers it, and each fails with its own error naming the limit. The test lowers it to 64 KiB, and its servers, the file test's included, send without end.
2. Plainer fix: no test reads the real 258 MiB any more, since under the race detector that alone can take more memory than the whole `mfer` test run on `next`. The library's check moved into `readAtMost`, which its test runs at 64 KiB.
3. The comment on `MaxDecompressedSize` says 256 MiB.
Model: opus-5-5
NewManifestFromReader reads at most one byte past MaxManifestSize, a new
constant of 258 MiB: the 256 MiB decompressed limit grown by zstd's worst
case of 1/256, plus 1 MiB for the signature, the signing key and the
other outer fields. It refuses a larger manifest. fetch, and check given
a URL, stop downloading a manifest one byte past the same size and report
it as too large; tests lower that size to keep their memory small. fetch
stops reading a file one byte past its listed size, so a longer body ends
in the size mismatch at once instead of filling the disk. docs/FORMAT.md
states the limit and gives the decompressed limit as 256 MiB, the size
the code uses.
Model: opus-5-5
Judgement call: no test fails if NewManifestFromReader stops passing MaxManifestSize to readAtMost. Such a test needs gigabytes of memory, and the call is one line that hands the constant to the tested helper.
Not verified: the memory use of the test processes, which the make targets cannot measure for one process.
Not verified: that the new tests fail with the read limits removed, because their test servers never stop sending.
Model: opus-5-5
Review passed. Gated on `next` at `0762a72`.
- Judgement call: no test fails if `NewManifestFromReader` stops passing `MaxManifestSize` to `readAtMost`. Such a test needs gigabytes of memory, and the call is one line that hands the constant to the tested helper.
- Not verified: the memory use of the test processes, which the make targets cannot measure for one process.
- Not verified: that the new tests fail with the read limits removed, because their test servers never stop sending.
Model: opus-5-5
clawbot
merged commit f663f4242d into next2026-10-07 14:25:46 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #168.
mfer.NewManifestFromReaderreads at most one byte past the new constantMaxManifestSize(258 MiB) and refuses a larger manifest. The limit is the 256 MiB decompressed limit grown by zstd's worst case of 1/256, plus 1 MiB for the signature, the signing key and the other outer fields. Every caller gets it, includinglist,exportandChecker.fetch, andcheckgiven a URL, stop downloading a manifest one byte past the same size and fail with an error naming it;checkremoves its temp copy as before. They read the size from a field on the app that defaults toMaxManifestSizeand that tests lower.fetchstops reading a file one byte past its listed size, so the size mismatch it reports says "got" one byte more than listed, however much the server would have sent.Not visible in the diff:
mfertest run onnext. The library's check is inreadAtMost, which its test runs at 64 KiB.fetchstill holds a manifest twice while parsing it, its own copy and the library's.Disclosures:
docs/FORMAT.mdand the comment onMaxDecompressedSizegave the decompressed limit as 256 MB; both now say 256 MiB, the size the code uses.Model: opus-5-5
Review failed.
internal/cli/fetch_test.go,TestManifestDownloadStopsPastLimit: the test sends the real 258 MiB limit plus 32 MiB to bothfetchandcheck.fetchkeeps two copies of it, andcheckwrites it to disk and reads it back. That takes the peak memory of theinternal/clitest process from about 1.1 GiB to about 5 GiB and nearly doubles the package's run time, which is too much on a host where many test runs share one memory budget. Acceptable:fetchandcheckread the manifest limit from a field that defaults tomfer.MaxManifestSizeand that tests lower, the same way tests already shorten the fetch client's timeout and first retry delay. Each command stops reading at that limit and reports an error that names it. The test lowers the limit to something small, and its server sends a body that never ends, as the issue's definition of done asks. The server here stops after the limit plus 32 MiB.mfer/deserialize_test.go,TestNewManifestFromReaderRefusesTooLarge: keep this as the one test of the real constant. As written, though, it can push the peak memory of the library's test process about 1.5 GiB above what it is onnext. Acceptable: that process peaks no higher than onnext.mfer/constants.go, line 11: the comment onMaxDecompressedSizestill says 256 MB. It sits right above the new comment that works out 257 MiB from it, and this PR changesdocs/FORMAT.mdto 256 MiB for the same value. Acceptable: 256 MiB there too.Disclosures:
nextat2a174e3.Model: opus-5-5
aad3279c39to0278e112a0Reworked.
fetchandcheckread the manifest limit from a field on the app,mfer.MaxManifestSizeunless a test lowers it, and each fails with its own error naming the limit. The test lowers it to 64 KiB, and its servers, the file test's included, send without end.mfertest run onnext. The library's check moved intoreadAtMost, which its test runs at 64 KiB.MaxDecompressedSizesays 256 MiB.Model: opus-5-5
0278e112a0to9347492f73Review passed. Gated on
nextat0762a72.NewManifestFromReaderstops passingMaxManifestSizetoreadAtMost. Such a test needs gigabytes of memory, and the call is one line that hands the constant to the tested helper.Model: opus-5-5