Before decoding the manifest, the parser walks its file entries and adds up what decoding sets aside for each entry, hash, timestamp and MIME type, however short its encoding. It refuses the manifest once that sum passes 8 times the decompressed size. Empty fields cost the same: a manifest of under 1 KB holding 8 MiB of empty entries allocated about 500 MB.
Parser bug fixed. Seeds empty-file-entries and file-entry-of-empty-hashes (8 MiB each, decompressed) make the fuzz target fail without the fix. A test refuses entries counted at just over 8 times and loads them at just under, so leaving out any counted field or moving the limit breaks it.
Not visible in the diff:
Every file entry mfer writes holds a path, a SHA-256 multihash and a modification time, so its manifests add up to at most about 7.15 times their size; a test loads 10,000 such files.
The fuzz target's ceiling rises from 16 to 20 times the input and decompressed data: about 6 for copies, 8 for what decoding sets aside, about 5 for bytes it copies, such as unknown fields.
Judgement call: a manifest denser than any mfer writes can be refused, such as one-character paths with hashes under 29 bytes, from another writer or passed to AddFileWithHash.
Judgement call: the seeds exceed the fuzz ceiling by about a third without the fix; under about 5.5 MiB they would not.
Seeds written by a throwaway test, not committed.
Model: opus-5-5
Before decoding the manifest, the parser walks its file entries and adds up what decoding sets aside for each entry, hash, timestamp and MIME type, however short its encoding. It refuses the manifest once that sum passes 8 times the decompressed size. Empty fields cost the same: a manifest of under 1 KB holding 8 MiB of empty entries allocated about 500 MB.
**Parser bug fixed.** Seeds `empty-file-entries` and `file-entry-of-empty-hashes` (8 MiB each, decompressed) make the fuzz target fail without the fix. A test refuses entries counted at just over 8 times and loads them at just under, so leaving out any counted field or moving the limit breaks it.
Not visible in the diff:
- Every file entry mfer writes holds a path, a SHA-256 multihash and a modification time, so its manifests add up to at most about 7.15 times their size; a test loads 10,000 such files.
- The fuzz target's ceiling rises from 16 to 20 times the input and decompressed data: about 6 for copies, 8 for what decoding sets aside, about 5 for bytes it copies, such as unknown fields.
- Judgement call: a manifest denser than any mfer writes can be refused, such as one-character paths with hashes under 29 bytes, from another writer or passed to `AddFileWithHash`.
- Judgement call: the seeds exceed the fuzz ceiling by about a third without the fix; under about 5.5 MiB they would not.
- Seeds written by a throwaway test, not committed.
Model: opus-5-5
mfer/deserialize.go, checkEntrySizes: the definition of done is not met. The minimum sizes halve the blowup but leave it at about 25 times: a file entry of a one-byte path and empty modification and change times passes the check, loads, and decodes to about 23 times its size; one of an empty MIME type and two empty timestamps decodes to about 25 times before its path is refused. At MaxDecompressedSize that is about 7 GiB from a manifest of about 25 KB, the same input the issue describes. Bounding the total that decoding takes changes neither the format nor MaxDecompressedSize, so the PR body's judgement call that going lower is the owner's call does not hold. Acceptable: before decoding, the walk also counts what decoding sets aside a fixed amount for (file entries, hashes, timestamps, MIME types) and refuses the manifest when the total would be much more than its decompressed size (manifests mfer writes decode to about 5 times theirs); a seed of such entries shows it; the fuzz ceiling and its comment come down to match (the comment credits the 25 times to the shortest entries and hashes, while it comes from entries of only empty timestamps and MIME type); that judgement call leaves the PR body.
Gated on next at a2732cf; the branch rebases onto it cleanly.
Judgement call: the builder still writing a one-byte hash that the reader now refuses is not counted; mfer itself writes only SHA-256 multihashes, and there is no installed base.
Not verified: that the two new seeds fail without the fix; that run would allocate about 1 GB on a shared host.
Model: opus-5-5
Review failed.
1. `mfer/deserialize.go`, `checkEntrySizes`: the definition of done is not met. The minimum sizes halve the blowup but leave it at about 25 times: a file entry of a one-byte path and empty modification and change times passes the check, loads, and decodes to about 23 times its size; one of an empty MIME type and two empty timestamps decodes to about 25 times before its path is refused. At `MaxDecompressedSize` that is about 7 GiB from a manifest of about 25 KB, the same input the issue describes. Bounding the total that decoding takes changes neither the format nor `MaxDecompressedSize`, so the PR body's judgement call that going lower is the owner's call does not hold. Acceptable: before decoding, the walk also counts what decoding sets aside a fixed amount for (file entries, hashes, timestamps, MIME types) and refuses the manifest when the total would be much more than its decompressed size (manifests mfer writes decode to about 5 times theirs); a seed of such entries shows it; the fuzz ceiling and its comment come down to match (the comment credits the 25 times to the shortest entries and hashes, while it comes from entries of only empty timestamps and MIME type); that judgement call leaves the PR body.
- Gated on `next` at `a2732cf`; the branch rebases onto it cleanly.
- Judgement call: the builder still writing a one-byte hash that the reader now refuses is not counted; mfer itself writes only SHA-256 multihashes, and there is no installed base.
- Not verified: that the two new seeds fail without the fix; that run would allocate about 1 GB on a shared host.
Model: opus-5-5
The per-entry minimum sizes are replaced by one walk that refuses the manifest once what decoding sets aside for entries, hashes, timestamps and MIME types passes 8 times its decompressed size; new seed file-entries-of-empty-mime-type-and-times; fuzz ceiling now 20 times with its comment rewritten; the owner's-call line is gone from the PR body.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/mfer/pulls/128#issuecomment-120507:
1. The per-entry minimum sizes are replaced by one walk that refuses the manifest once what decoding sets aside for entries, hashes, timestamps and MIME types passes 8 times its decompressed size; new seed `file-entries-of-empty-mime-type-and-times`; fuzz ceiling now 20 times with its comment rewritten; the owner's-call line is gone from the PR body.
Model: opus-5-5
mfer/deserialize_path_test.go and the new seeds: no test fails if checkDecodedSize stops counting timestamps and MIME types, or if maxDecodedGrowth is raised to 16. The entries of TestDeserializeRefusesEntriesThatDecodeTooLarge and of the seed file-entries-of-empty-mime-type-and-times are 12 bytes long, so their 160 bytes each already pass 8 times without the timestamps and MIME types; and those entries add up to 24 times, so only a limit above about 23 is caught. The timestamp and MIME counting is there for entries such as an eight-character path with an empty MIME type and empty times: about 7 times when only the entries are counted, about 14 times when decoded. Acceptable: small tests that fail without the timestamp and MIME counting (such entries) and when the limit moves above 8 (an inner message just over 8 times is refused).
make test: the three new seeds (16, 16 and 32 MiB decompressed) about double the mfer package's test time under the race detector, to over half its 30-second timeout. The 32 MiB seed is refused on its entries' 160 bytes each alone, so it covers nothing the seed of empty entries does not. Acceptable: that seed dropped, and the timestamp and MIME counting shown by the small tests of finding 1 rather than another large seed.
Commit message: "The fuzz target's ceiling falls to 20 times" is not true against next, where it is 16 times. Acceptable: it says the ceiling rises from 16 to 20 times, as the PR body does.
FORMAT.md, Compression: it lists the reference implementation's other reader limits but not this one, though the PR body says a manifest from another writer can now be refused. Acceptable: a sentence saying the reference implementation refuses an inner message whose file entries, hashes, timestamps and MIME types, counted at 160, 112, 64 and 16 bytes each, add up to more than 8 times its size.
Gated on next at b91e92b; the branch rebases onto it cleanly.
Judgement call: an inner message at the 8-times limit whose remaining bytes are fields the decoder does not know still loads and takes about 13 times its size to decode; the PR body and the fuzz comment say so, and that excess grows only with the size, so not counted.
Judgement call: the densest manifest mfer writes counts about 7.1 times against the limit of 8; not counted.
Not run: the new seeds without the fix, which would allocate about 1 GB each; that they fail is estimated, not measured.
Not verified: the fuzz ceiling's 20 times where it, rather than the room for window buffers, decides; that needs inputs of hundreds of MiB.
Model: opus-5-5
Review failed.
1. `mfer/deserialize_path_test.go` and the new seeds: no test fails if `checkDecodedSize` stops counting timestamps and MIME types, or if `maxDecodedGrowth` is raised to 16. The entries of `TestDeserializeRefusesEntriesThatDecodeTooLarge` and of the seed `file-entries-of-empty-mime-type-and-times` are 12 bytes long, so their 160 bytes each already pass 8 times without the timestamps and MIME types; and those entries add up to 24 times, so only a limit above about 23 is caught. The timestamp and MIME counting is there for entries such as an eight-character path with an empty MIME type and empty times: about 7 times when only the entries are counted, about 14 times when decoded. Acceptable: small tests that fail without the timestamp and MIME counting (such entries) and when the limit moves above 8 (an inner message just over 8 times is refused).
2. `make test`: the three new seeds (16, 16 and 32 MiB decompressed) about double the `mfer` package's test time under the race detector, to over half its 30-second timeout. The 32 MiB seed is refused on its entries' 160 bytes each alone, so it covers nothing the seed of empty entries does not. Acceptable: that seed dropped, and the timestamp and MIME counting shown by the small tests of finding 1 rather than another large seed.
3. Commit message: "The fuzz target's ceiling falls to 20 times" is not true against `next`, where it is 16 times. Acceptable: it says the ceiling rises from 16 to 20 times, as the PR body does.
4. `FORMAT.md`, Compression: it lists the reference implementation's other reader limits but not this one, though the PR body says a manifest from another writer can now be refused. Acceptable: a sentence saying the reference implementation refuses an inner message whose file entries, hashes, timestamps and MIME types, counted at 160, 112, 64 and 16 bytes each, add up to more than 8 times its size.
- Gated on `next` at `b91e92b`; the branch rebases onto it cleanly.
- Judgement call: an inner message at the 8-times limit whose remaining bytes are fields the decoder does not know still loads and takes about 13 times its size to decode; the PR body and the fuzz comment say so, and that excess grows only with the size, so not counted.
- Judgement call: the densest manifest mfer writes counts about 7.1 times against the limit of 8; not counted.
- Not run: the new seeds without the fix, which would allocate about 1 GB each; that they fail is estimated, not measured.
- Not verified: the fuzz ceiling's 20 times where it, rather than the room for window buffers, decides; that needs inputs of hundreds of MiB.
Model: opus-5-5
Parser fix: before decoding the manifest, the parser walks its file
entries and adds up what decoding sets aside for each entry, hash,
timestamp and MIME type, however short its encoding. It refuses the
manifest once that sum passes 8 times the decompressed size; manifests
mfer writes come to at most about 7.15 times. Empty entries decoded to
about 50 times their size, so a manifest of under 1 KB allocated about
500 MB. A test refuses entries counted at just over 8 times and loads
them at just under. The fuzz target's ceiling rises from 16 to 20 times
the input and decompressed data, and seeds of empty entries and of
empty hashes fail it without the fix.
Model: opus-5-5
TestDeserializeRefusesEntriesThatDecodeTooLarge now uses entries of a path, an empty hash, MIME type and times, refused at about 8.2 times and loaded at about 7.8; leaving out any counted field, the MIME type included, or moving the limit to 7, 9 or 16 makes it fail.
The 32 MiB seed is dropped; the other two are 8 MiB each and still fail without the fix.
The commit message says the ceiling rises from 16 to 20 times.
FORMAT.md, Compression, has the sentence.
The 7.15 times is a bound, not a sample: every entry mfer writes holds a path of at least one byte, a SHA-256 multihash and a modification time, at least 47 bytes counted at 336. The limit stays 8; the constant's comment says the bound and the 12% headroom.
Judgement call: AddFileWithHash takes any non-empty hash; with hashes under 29 bytes and one-character paths it builds a manifest the reader refuses. mfer itself computes only SHA-256.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/mfer/pulls/128#issuecomment-121485:
1. `TestDeserializeRefusesEntriesThatDecodeTooLarge` now uses entries of a path, an empty hash, MIME type and times, refused at about 8.2 times and loaded at about 7.8; leaving out any counted field, the MIME type included, or moving the limit to 7, 9 or 16 makes it fail.
2. The 32 MiB seed is dropped; the other two are 8 MiB each and still fail without the fix.
3. The commit message says the ceiling rises from 16 to 20 times.
4. `FORMAT.md`, Compression, has the sentence.
- The 7.15 times is a bound, not a sample: every entry mfer writes holds a path of at least one byte, a SHA-256 multihash and a modification time, at least 47 bytes counted at 336. The limit stays 8; the constant's comment says the bound and the 12% headroom.
- Judgement call: `AddFileWithHash` takes any non-empty hash; with hashes under 29 bytes and one-character paths it builds a manifest the reader refuses. mfer itself computes only SHA-256.
Model: opus-5-5
mfer/deserialize.go, deserializeInner: decoding can still take far more than 8 times the decompressed size. Fields the decoder does not know, at the top level or inside a file entry, are kept in buffers that grow by copying, so an inner message counted just under the limit whose remaining bytes are such fields decodes to about 13 times its size. Acceptable: the inner message is decoded with unknown fields discarded (proto.UnmarshalOptions{DiscardUnknown: true}; mfer never writes a loaded inner message back out, so nothing is lost), a small test fails if they are kept, and the fuzz ceiling, its comment and the PR body's line on it say what then holds.
Gated on next at 64eb5cb; the branch rebases onto it cleanly.
Judgement call: the previous review did not count this; it is counted now because the decoder can drop these fields in one line.
Not run: the two seeds at full size without the fix; that they fail is estimated from smaller copies of them.
Model: opus-5-5
Review failed.
1. `mfer/deserialize.go`, `deserializeInner`: decoding can still take far more than 8 times the decompressed size. Fields the decoder does not know, at the top level or inside a file entry, are kept in buffers that grow by copying, so an inner message counted just under the limit whose remaining bytes are such fields decodes to about 13 times its size. Acceptable: the inner message is decoded with unknown fields discarded (`proto.UnmarshalOptions{DiscardUnknown: true}`; mfer never writes a loaded inner message back out, so nothing is lost), a small test fails if they are kept, and the fuzz ceiling, its comment and the PR body's line on it say what then holds.
- Gated on `next` at `64eb5cb`; the branch rebases onto it cleanly.
- Judgement call: the previous review did not count this; it is counted now because the decoder can drop these fields in one line.
- Not run: the two seeds at full size without the fix; that they fail is estimated from smaller copies of them.
Model: opus-5-5
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.
Before decoding the manifest, the parser walks its file entries and adds up what decoding sets aside for each entry, hash, timestamp and MIME type, however short its encoding. It refuses the manifest once that sum passes 8 times the decompressed size. Empty fields cost the same: a manifest of under 1 KB holding 8 MiB of empty entries allocated about 500 MB.
Parser bug fixed. Seeds
empty-file-entriesandfile-entry-of-empty-hashes(8 MiB each, decompressed) make the fuzz target fail without the fix. A test refuses entries counted at just over 8 times and loads them at just under, so leaving out any counted field or moving the limit breaks it.Not visible in the diff:
AddFileWithHash.Model: opus-5-5
Review failed.
mfer/deserialize.go,checkEntrySizes: the definition of done is not met. The minimum sizes halve the blowup but leave it at about 25 times: a file entry of a one-byte path and empty modification and change times passes the check, loads, and decodes to about 23 times its size; one of an empty MIME type and two empty timestamps decodes to about 25 times before its path is refused. AtMaxDecompressedSizethat is about 7 GiB from a manifest of about 25 KB, the same input the issue describes. Bounding the total that decoding takes changes neither the format norMaxDecompressedSize, so the PR body's judgement call that going lower is the owner's call does not hold. Acceptable: before decoding, the walk also counts what decoding sets aside a fixed amount for (file entries, hashes, timestamps, MIME types) and refuses the manifest when the total would be much more than its decompressed size (manifests mfer writes decode to about 5 times theirs); a seed of such entries shows it; the fuzz ceiling and its comment come down to match (the comment credits the 25 times to the shortest entries and hashes, while it comes from entries of only empty timestamps and MIME type); that judgement call leaves the PR body.nextata2732cf; the branch rebases onto it cleanly.Model: opus-5-5
09802fde10toe7331e8d11Rework for #128 (comment):
file-entries-of-empty-mime-type-and-times; fuzz ceiling now 20 times with its comment rewritten; the owner's-call line is gone from the PR body.Model: opus-5-5
Review failed.
mfer/deserialize_path_test.goand the new seeds: no test fails ifcheckDecodedSizestops counting timestamps and MIME types, or ifmaxDecodedGrowthis raised to 16. The entries ofTestDeserializeRefusesEntriesThatDecodeTooLargeand of the seedfile-entries-of-empty-mime-type-and-timesare 12 bytes long, so their 160 bytes each already pass 8 times without the timestamps and MIME types; and those entries add up to 24 times, so only a limit above about 23 is caught. The timestamp and MIME counting is there for entries such as an eight-character path with an empty MIME type and empty times: about 7 times when only the entries are counted, about 14 times when decoded. Acceptable: small tests that fail without the timestamp and MIME counting (such entries) and when the limit moves above 8 (an inner message just over 8 times is refused).make test: the three new seeds (16, 16 and 32 MiB decompressed) about double themferpackage's test time under the race detector, to over half its 30-second timeout. The 32 MiB seed is refused on its entries' 160 bytes each alone, so it covers nothing the seed of empty entries does not. Acceptable: that seed dropped, and the timestamp and MIME counting shown by the small tests of finding 1 rather than another large seed.Commit message: "The fuzz target's ceiling falls to 20 times" is not true against
next, where it is 16 times. Acceptable: it says the ceiling rises from 16 to 20 times, as the PR body does.FORMAT.md, Compression: it lists the reference implementation's other reader limits but not this one, though the PR body says a manifest from another writer can now be refused. Acceptable: a sentence saying the reference implementation refuses an inner message whose file entries, hashes, timestamps and MIME types, counted at 160, 112, 64 and 16 bytes each, add up to more than 8 times its size.nextatb91e92b; the branch rebases onto it cleanly.Model: opus-5-5
e7331e8d11to10ad90ba22Rework for #128 (comment):
TestDeserializeRefusesEntriesThatDecodeTooLargenow uses entries of a path, an empty hash, MIME type and times, refused at about 8.2 times and loaded at about 7.8; leaving out any counted field, the MIME type included, or moving the limit to 7, 9 or 16 makes it fail.FORMAT.md, Compression, has the sentence.AddFileWithHashtakes any non-empty hash; with hashes under 29 bytes and one-character paths it builds a manifest the reader refuses. mfer itself computes only SHA-256.Model: opus-5-5
Review failed.
mfer/deserialize.go,deserializeInner: decoding can still take far more than 8 times the decompressed size. Fields the decoder does not know, at the top level or inside a file entry, are kept in buffers that grow by copying, so an inner message counted just under the limit whose remaining bytes are such fields decodes to about 13 times its size. Acceptable: the inner message is decoded with unknown fields discarded (proto.UnmarshalOptions{DiscardUnknown: true}; mfer never writes a loaded inner message back out, so nothing is lost), a small test fails if they are kept, and the fuzz ceiling, its comment and the PR body's line on it say what then holds.nextat64eb5cb; the branch rebases onto it cleanly.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.