[youdie006]

The three legacy decoders hand an entry's declared uncompressed size straight to Vec::reserve, so a 200-byte archive that declares 0xFFFF_FFFF_FFFF_FFFF bytes panics out of the public read API:

thread 'main' panicked at library/alloc/src/raw_vec/mod.rs:28:5
capacity overflow

Reached by ZipArchive::by_index(0) then read_to_end. The Stored sibling on an otherwise identical archive returns Err("Invalid checksum"), which is what the template's "all the crate's methods remain panic-free" line asks for.

Clamping rather than rejecting, because the template also says ZIP bombs are not filtered: the decode loops (while dst.len() < uncomp_len) append and grow on their own, so the reserve is only a hint and a large legitimate entry still decodes.

Five sites, all with the same shape: shrink.rs:234, shrink.rs:347, reduce.rs:107, implode.rs:97, implode.rs:214 (line numbers from git show HEAD:).

Verification

Base 75eec5b.

row	new test file
base	4 ran, 3 failed (capacity overflow in Shrink, Reduce, Implode; Stored passes)
this PR	4 ran, 4 passed
revert shrink.rs:234 alone	4 ran, 1 failed (the Shrink case)
revert reduce.rs:107 alone	4 ran, 1 failed (the Reduce case)
MAX_PREALLOC = usize::MAX	4 ran, 3 failed

Each single-site revert is caught by its own decoder's case, so no site is redundant. The Stored case passes in every row, which is what makes the other three a real divergence rather than a broken harness.

cargo test --no-default-features --features legacy-zip: 28 result blocks, 0 failures, and the repo's own tests/legacy_zip.rs round-trips of shrink.zip, reduce.zip and implode.zip still pass. cargo clippy --no-default-features --features legacy-zip --lib -- -D warnings clean; cargo doc --no-deps clean; cargo fmt --check --all clean.

cargo clippy --all-targets -- -D warnings fails, but identically with my changes and on an unmodified tree: tests/data_descriptor.rs:27 trips clippy::bool_assert_comparison on rustc 1.97.0. Not from this change.

Not run here: the Windows and macOS legs, the MSRV 1.88 leg, and --all-features. The change is usize::min on a usize.

Possibly relevant to why this lasted: fuzz/read drives exactly this path, by_index plus io::copy, but fuzz/Cargo.toml pins zip with default-features = false and only a zip-default knob, and legacy-zip is not in default. The fuzzer has never compiled these decoders.

Two things I did not do. MAX_PREALLOC = 1 << 20 is a guess at the right ceiling; deriving it from the compressed length would be just as easy if you prefer that. And ImplodeDecoder::read and ShrinkDecoder::read each reserve and then call a helper that reserves the same value again, which looks redundant but is a separate change.

---

Disclosure: written with the help of Claude (an AI assistant). Every number above is from runs on my machine.

[Copilot]

🟡 Changes recommended

Address the test assertion gap and document or improve the large-entry preallocation behavior.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Prevents legacy ZIP decoders from panicking on enormous declared uncompressed sizes by bounding preallocation.

Changes:

• Adds a 1 MiB preallocation cap.
• Applies it to Shrink, Reduce, and Implode decoders.
• Adds oversized-declaration regression tests.

File summaries

File	Summary
tests/legacy_oversized_declared_size.rs	Adds regression cases; should assert malformed entries return errors.
src/legacy/shrink.rs	Applies bounded preallocation.
src/legacy/reduce.rs	Applies bounded preallocation.
src/legacy/mod.rs	Defines the shared allocation cap, which may regress large-entry performance.
src/legacy/implode.rs	Applies bounded preallocation.

Review details

Suppressed comments (1)

src/legacy/mod.rs:8

• This hard 1 MiB cap means every valid legacy entry larger than 1 MiB now grows the output incrementally and copies it during geometric reallocations, rather than using the exact-size preallocation. That is a potentially substantial regression for large legacy archives, so the claim that clamping costs nothing is inaccurate; please use or document a bounded estimate that preserves more normal-case preallocation (for example, one derived from the compressed input and decoder limits).

/// Upper bound on the speculative pre-allocation made from an entry's declared
/// uncompressed size. That size comes from the archive, so it is not a fact:
/// reserving it outright panics with "capacity overflow" past `isize::MAX` and
/// aborts on allocation failure below it. The decoders append to the buffer as
/// they go, so the reservation is only a hint and clamping it costs nothing.
pub(crate) const MAX_PREALLOC: usize = 1 << 20;

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

No unresolved issues remain, and the supplied assessments support approval.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Pr0methean]

Thanks; this fix looks good.