Skip to content

docx/xlsx: refuse packages whose parts unpack past 1 GiB (#851) - #1849

Merged
yeroo merged 3 commits into
mainfrom
issue-851-leftovers-from-451-yppx-no-size
Nov 2, 2026
Merged

yeroo merged 3 commits into
mainfrom
issue-851-leftovers-from-451-yppx-no-size

Conversation

@yeroo

@yeroo yeroo commented Nov 2, 2026 •

Copy link
Copy Markdown
Owner

Closes #851

What and why

Opening a .docx or .xlsx extracted every ZIP part with no package-wide limit, so a small crafted file of highly compressible parts could make opening allocate gigabytes. #451 fixed this for .yppx; this applies the same "select the parts, sum their declared sizes, refuse before extracting anything" budget to Word and Excel.

  • opccore::zip::declared_bytes (moved from projcore::yppx, saturating sum).
  • docxcore::package::load_package: MAX_PACKAGE_UNCOMPRESSED = 1 GiB; over it is LoadError::TooLarge { total, budget } ("document too large: its parts unpack to N MiB, over the 1024 MiB limit").
  • gridcore::xlsx: the same budget, checked in part_entries, so load_xlsx and load_xlsx_repair both refuse before extracting (repair never files an oversized part as damaged); XlsxError::TooLarge.
  • Suite: both errors show in words; an oversized .docx is not sent to Recover (it is not damaged). wordcomshim shows it in words too.
  • gridcore::legacy::open_workbook_ex reads mimetype for the ODS sniff only when it declares <= 256 bytes, so it cannot inflate a huge entry ahead of the budget (pre-existing, found in review).
  • The budget bounds extracted bytes, not peak memory; the docs say so.

Tests

Boundary tests (budget = total loads, total-1 refused) for docx, xlsx and xlsx repair; a part declaring 4 GiB is TooLarge (was CorruptPart); directory entries are not budgeted; a too-large docx is not recovered in the suite (convert_tests::a_too_large_docx_is_not_recovered); a long mimetype is not sniffed (fails with the guard reverted).

Local whole suite on the head: see the merge note. On this laptop the two known convert_child flakes (a_child_that_dies_is_a_load_error_not_a_crash, a_child_over_its_memory_limit_is_stopped) fail under the full run; this change does not touch convert_child.rs, so per the machine's rule the local suite counts as passing provided CI is green.

Review record

  • r1 (revmux, claude-only profile; the default profile's model was unavailable): 1 Major (oversized .docx went to Recover / Debug text) fixed; 1 pre-existing (mimetype pre-read) fixed.
  • r2: 2 minor (mimetype test did not discriminate; wrong recovery comment) fixed; 1 pre-existing (cp1252 transcoding peak memory) — doc fixed here, the code change deferred as its own issue.
  • review stopped: round 2 had no Major; 1 minor finding(s) in xlsx open: cp1252 part transcoding can allocate several times the 1 GiB package budget #1850

🤖 Generated with Claude Code

https://claude.ai/code/session_01HSDL3pEpKYBsRq3sk9rL8v

yeroo and others added 3 commits November 3, 2026 01:13
Opening a .docx or .xlsx extracted every part with no package-wide
limit, so a small file of compressible parts declaring large sizes could
allocate gigabytes. Sum the declared sizes of the parts that will be
extracted and refuse with a new TooLarge error before extracting any,
as #451 did for .yppx. For .xlsx the check sits in part_entries, so
load_xlsx_repair refuses too instead of taking the part for damaged.
declared_bytes moves to opccore::zip for all three loaders. The suite's
sheet open shows TooLarge in words, not its Debug form.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSDL3qKNvyrNNa8EviBU8i
…ead (#851 r1)

The suite's document load showed TooLarge in Debug form and then ran
text recovery on it, so an undamaged but oversized .docx opened as
"recovered text from a damaged file"; it now keeps the load error in
words. wordcomshim says it in words too. open_workbook_ex read the
`mimetype` entry for the ODS sniff before load_xlsx's budget, up to its
declared 4 GiB; it now reads it only when it declares 256 bytes or less.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSDL3qKNvyrNNa8EviBU8i
The mimetype test now holds a long but matching ODS mark in a real xlsx,
which the old pre-read took for ODS: it must open as .xlsx. The budget
docs say they bound the extracted bytes, not peak memory, and the
suite's recovery docs drop a wrong reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSDL3qKNvyrNNa8EviBU8i
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yeroo
yeroo merged commit b5ee0b1 into main Nov 2, 2026
14 checks passed
@yeroo
yeroo deleted the issue-851-leftovers-from-451-yppx-no-size branch November 2, 2026 23:52
@yeroo

yeroo commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Merged automatically (auto-merge is on for this checkout): review stopped: round 2 had no Major; 1 minor finding(s) in #1850; merge rounds: none; whole suite on 8e2c92a: 11609 passed, 2 failed — the two known local convert_child flakes (a_child_that_dies_is_a_load_error_not_a_crash, a_child_over_its_memory_limit_is_stopped, "the conversion took too long"); this change does not touch convert_child.rs and CI (14 checks, Windows included) is green, so per this machine's rule the local suite was counted as passing; merge-check ok.

Follow-ups: #1850 (own issue, minor, review r2, pre-existing: cp1252 transcoding peak memory beyond the package budget). No disputed findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Leftovers from #451: yppx: no size budget when decompressing package parts on open

2 participants