* fix(cos): a clone that edits an object stream or a /Length stops reading what other clones cached
Clones share the object cache and the parsed object streams. #5802 keeps
results that read a clone's edits out of those shared maps, but a clone
could still read entries cached before its edit, by another clone or by
itself: after replacing object stream 4, a member of it still read as the
file's copy, and a stream or object stream framed by an indirect /Length
kept the file's framing after the length was edited.
Each cached load now records the other objects it read (a member's object
stream, an indirect /Length, and what those read in turn), and a document
whose edits include one of them reads the object again. Documents with no
edits read the shared entries as before. This also replaces forgetting a
stream's cached member failures on every edit, which the same check covers
for the editing document without costing other clones their entries.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(cos): count a reference read as null in what a load read
A reference whose generation doesn't match the file reads as null, but an
edit that sets that generation makes it match. A load that read null there
(an object stream's /DecodeParms by reference, say) now counts the object
as read, so a clone that sets it decodes the stream again instead of
taking the one another clone cached. Reads keep a shared slice, and the
read bound has its own test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(optimize): edit the object stream without breaking it
space_audit_shares_an_object_stream_..._edited_members set the stream's
/First to 999999 and still expected its members to read, which only held
while a stale shared-cache entry answered for them. Mark the stream with
an unrelated key instead: it is still edited, so its members are sized
on their own, and they stay readable.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(cos): keep an object stream decoded by edited parameters out of the shared cache
Whether an object stream is the loading document's own was settled
before its indirect /DecodeParms were read. A clone that edited them
decoded the stream by its edit and put the result in the cache its
clones share, so the document it came from read the clone's data.
The check now runs after decoding.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: KenM76 <KenM76>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
## What this changes
`read_xref_chain_materialized` in `crates/cos/src/document/reader.rs` keeps a running total of the rows it merges across the cross-reference chain. Past `MAX_XREF_OBJECTS` (8,388,607) it returns `CosError::Syntax` ("cross-reference chain lists more than 8388607 entries"). The open path then rebuilds the index from object headers and logs the repair, as it already does when a single section decodes past that cap. Before this, the per-section cap and the 10,000-section chain limit multiplied, so the merged map had no overall bound.
Each section adds `sect.len() + odd.len()`: its entries plus the rows it set aside, including those from a hybrid `/XRefStm`. Set-aside rows are kept for the whole chain, and a section can set aside nearly everything it lists. The count is of rows merged, not the map's size, because a chain that relists the same objects costs as much to merge. A section's rows never outnumber what the compact reader counts for it, and that reader stops at 2,000,000 rows. So no chain the compact reader accepts is refused here.
The limit itself lives in `read_xref_chain_materialized_with_limit` (`pub(super)`). The existing entry point, `read_xref_chain_materialized`, calls it with `MAX_XREF_OBJECTS`, so tests can reach the limit in a few rows. Both functions have doc comments.
Fixes#5082
## How you verified it
Three regression tests in `crates/cos/src/document/tests.rs`. The two small-limit tests were suggested by @Harrison-Booth in review.
1. `a_cross_reference_chain_past_the_total_cap_is_refused_and_the_file_reconstructed` runs end to end at the production cap. It builds a synthetic file:
- a catalog, a page tree with one page, and a chain of three cross-reference streams;
- the newest stream locates objects 0 to 6;
- two older Flate-compressed streams each list 4,200,000 rows (`/W [1 0 0]`, disjoint `/Index` ranges). Each is half the per-section cap and too large for the compact reader. Their rows are in use at offset 0, so they are set aside, which keeps the test at a few bytes a row.
It asserts that `Document::open` logs the cap with "rebuilding from object headers" and then "rebuilt cross-reference index ...". The rebuilt document must keep `/Root 1 0 R` and the `/Page`. `read_xref_chain_materialized` must return `CosError::Syntax` at the oldest stream's offset.
2. `the_chain_row_limit_counts_disjoint_and_relisted_rows` uses three classic tables of 2, 1 and 1 rows. A limit of 4 passes, and a limit of 3 refuses the chain at the oldest table. This holds when the sections list different objects, and when one relists an object a newer section has: that is a map of three entries but four rows merged.
3. `the_chain_row_limit_counts_hybrid_rows_set_aside` uses a hybrid file whose table puts object 3 at an offset while its `/XRefStm` puts it in an object stream. It merges six rows and sets object 3 aside. A limit of 7 passes, and a limit of 6 refuses it.
Seen failing, each by reverting part of the fix in the working tree:
| Revert | Test 1 | Test 2 | Test 3 |
|---|---|---|---|
| no chain check at all | fails (empty repair log) | fails | fails |
| count without `sect.len()` | passes | fails | fails |
| count without `odd.len()` (state before 780764cb) | fails | passes | fails |
| fix restored | passes | passes | passes |
All three run in about a second together.
Commands, all with `-j 2` and a separate `CARGO_TARGET_DIR`:
- `cargo fmt -p pdfcraft-cos --check`: clean
- `cargo clippy -p pdfcraft-cos -j 2 --all-targets --locked -- -D warnings`: clean
- `cargo test -p pdfcraft-cos -j 2 --locked`: 95 unit tests and 39 integration tests pass (2 ignored)
- [x] `cargo fmt --check`
- [x] clippy with `-D warnings` on the crates I changed
- [x] tests pass on the crates I changed
- [x] n/a — `cargo xtask layers` / `cargo xtask assets`: no crate boundary or asset touched
## Never-crash
- [x] No new `unwrap`/`expect`/`panic!`/`unreachable!`/`todo!` outside tests (the offset conversion uses `unwrap_or(usize::MAX)`)
- [x] Checked arithmetic: `saturating_add` per section, `checked_add` for the running total; the change bounds an allocation sized by input
- [x] No `unsafe`
- [x] The unbounded allocation this fixes has a small synthetic regression test
## Provenance
- [x] n/a — no assets of any kind
- [x] n/a — no font files
- [x] No GPL/AGPL code copied in
- [x] Behaviour comes from ISO 32000 (§7.5.8 cross-reference streams, the Annex C object limit) and the crate's existing escalation path
- [x] n/a — no new assets, so no `ATTRIBUTION.toml` entry
## Security
- [x] n/a — no file paths or tool dispatch touched (`--root` confinement unaffected)
- [x] n/a — MCP and control channel untouched
- [x] No new outbound network access
## Parity and reach
- [x] n/a — hardening of the cross-reference reader, not a parity feature; no `parity/` entry changes
- [x] n/a — no new behaviour to expose; every `automation` tool that opens a document gets the cap
- [x] n/a — no documented behaviour changes
## Anything a reviewer should look at first
- The check runs after each section is read. So one section, already under the per-section cap, is decoded before the chain is refused. Peak memory stays bounded by the cap plus one section.
- The row limit is a parameter of a `pub(super)` function so tests can set it small. Production always passes `MAX_XREF_OBJECTS`, and the error text names whichever limit applied.
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
* **Bug Fixes**
* Documents with cross-reference chains that exceed the supported total size now report a syntax error at the section where the limit is exceeded.
* The limit applies across the entire chain, including repeated object numbers and set-aside entries, so individually valid sections cannot exceed the total cap.
* When opening an affected document, the index can be rebuilt from object headers, allowing available catalog and page content to be recovered.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
main fails to compile (E0592): the merge of #6046 added Stream::decoded_strict_within next to the one already there. Both decode the whole chain bounded by max and map the error to CosError::Filter; the earlier one stays, with #6046's test.
CodeRabbit's review of 674a20899:
- Each budgeted read of a font's stream went through
`Document::with_direct_parms`, which copies the whole stream when its
/DecodeParms is given by reference: an unrelated value in the
dictionary, however large, was copied again for every read.
`DecodeBudget::decode_stages_within` now takes the document and reads
through `Document::decoding_view`, as `decode_strict` and
`decode_with_damage` do: the data is shared, and only /Filter and the
parameters made direct are copied. `Reads::decode` and the editor's
program read use it, a Type 3 font's glyph descriptions read through
`decode_with_damage`, and so does Repair Thai maps' program read.
- A Type 3 font's glyph descriptions resolved one description's
parameters a font before the spent allowance refused it. They now ask
`DecodeBudget::refuses` first, as `decode_stages_within` does for the
others: once the allowance is spent a stream with data is refused, and
noted, before its parameters are resolved.
- `DecodeBudget::refused` counts the decodes refused for want of the
allowance, where `remaining() == 0` can't tell a refusal from an
allowance spent to the byte.
Test: `reading_a_fonts_streams_copies_no_dictionary` (`pdfcraft-fonts`,
a test binary of its own): a font whose program and ToUnicode hold an
unrelated array of a million numbers and /DecodeParms by reference is
read eight times by the editor's program read and the metrics read,
and the process's peak grows by less than half of what one array
holds. That a Type 3 font refuses before resolving parameters changes
no result, only the time it takes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
Editor, export and redaction must not read or rewrite text that the page view and Find never see, which a mismatched cap allows on large streams.
Lower MAX_DECODED to 256 MiB to match the renderer cap, so every Stream::decoded-based content reader stops at the same byte and oversized streams error instead of exposing later text.
Test: decoded_stops_at_renderer_cap_of_256_mib failed before the change and passes after.
Co-authored-by: eliets2 <287273517+eliets2@users.noreply.github.com>
Mutation testing of 063f8629e missed eight mutants in
`DecodeBudget::decode_stages_within`: nothing pinned what a read
stopped at a stage's limit, at the stages' total or at what is left of
the allowance charges, which limit it reports, or where one case gives
way to the next.
`a_stage_past_its_limit_charges_what_came_before_and_the_limit` reads
each way, and each boundary on both sides: a byte more room than the hex
stage and the limit (charged them, a byte left), exactly that (past the
allowance: all of it spent, its limit reported), a hex stage making
exactly the limit so the Flate stage reaches the stages' total with more
left (charged the total), with exactly the total left of a larger
allowance and with less (spent, the allowance's limit reported), and,
with nothing left, a stream with data that decodes to nothing (refused)
and an empty one (read).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
The Resources review of 8eead8ecd. The program read and the metrics reads
passed their limit times the number of filter stages as one total, which
any one stage could use: a FontFile2 of Flate making 128 MiB of
whitespace, under seven ASCIIHex stages (a 130 KB file), made all of it
before being refused, about 0.1 s a font where `main`'s bound stopped it
at 16 MiB. `edit`'s `matching_thai_outline` reads such a program for each
of a page's fonts with no total, and every `Metrics::from_dict` up to
three times.
Now each stage is held to the limit, as a decode without an allowance
holds it, and the stages together to the limit for each of them, so every
stream the stage scaling was for still reads: a full `/CIDToGIDMap` or a
12 MiB program behind ASCIIHex and Flate. The two bounds take an entry
point of their own: `pdfcraft_filters::decode_tolerant_counted_each`,
which `run` already supports, and `DecodeBudget::decode_stages_within`,
which `Reads::decode` and the editor's program read use. A stage past the
limit is charged what the stages before it decoded and the limit; past
the stages' total, or past what is left, it is charged as
`decode_within` charges. The program, the last stage, is so within the
limit without a check of its own.
Tests: `each_stage_is_held_to_its_own_bound_within_the_total` in
`pdfcraft-filters` (a Flate stage stopped at its bound hands the next
stage nothing), `each_stage_is_held_to_the_limit` in `pdfcraft-cos`, and
in `pdfcraft-fonts` `an_editors_program_read_holds_each_stage_to_its_limit`
and `each_stage_of_a_metrics_stream_is_held_to_its_limit`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
One conflict, in redact sanitize.rs. Main's #6021 (930b44131) split `remove_hidden` in two:
- it now counts only what the chosen categories need (`scan_only`);
- it hands the removal to `remove_counted`, which tests also call with `every_page`.
This branch had added one line to the old body: `kept_before`, what a full save keeps before
anything is removed. Main's side is taken whole, and that line is now the first thing
`remove_counted` does after finding the catalog, before any change to the document. So every
caller of `remove_counted` has it. `unshare_unmarked` stays last, after `unlink_undrawn`, and
merged cleanly there. The only early return after a change is `NothingToApply` when nothing
was removed and nothing unlinked.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
Conflicts, and how they were resolved:
- cos writer.rs and lib.rs: main now numbers a full save in `full_save_order` and exports
`reachable_objects`, the object numbers a full save writes, for the pattern guard. This
branch had added the same walk as `walk` and `reachable`. Main's is kept, with its alias
handling, and this branch's is dropped: `globals::kept` is now `reachable_objects`, so the
JBIG2 cleanup and the pattern guard read one walk. `kept` returns the walk's error (a save
that can't number its objects), which redaction and Remove Hidden Information pass on. The
cos test of what is reachable, and two redaction tests, read `reachable_objects` instead.
The exports keep this branch's `DECODE_PARMS_KEYS` and main's `TextChars`,
`civil_from_days` and `new_file_id`.
- redact lib.rs: main unlinks pruned patterns from `/Pages` nodes after `unlink_doomed`; this
branch replaced `restore_shared_jbig2` with `globals::unshare`. Both stay, the unlink first:
an image a pruned pattern's cell drew leaves the file with it, and is a user of a dictionary
that left too.
- redact sanitize.rs: main's `strip_outline_actions` and this branch's `unshare_unmarked` both
stay. At the end of `remove_hidden`, `unshare_unmarked` runs after main's `unlink_undrawn`,
so an image unlinked for being drawn nowhere takes a shared dictionary with it, and is
followed by main's `require_full_save_with_new_id`.
- redact image.rs: `MAX_PIXELS` is `pub(crate)` on both sides; the comment names
`globals::without_dictionary`, where this branch moved `without_globals`.
Tests: `an_undrawn_image_unlinked_takes_a_shared_jbig2_dictionary_too` (Remove Hidden
Information, through `unlink_undrawn`) and
`an_image_a_pruned_patterns_cell_drew_takes_a_shared_jbig2_dictionary_too` (a redaction,
through `unlink_pruned_patterns`) pin the order of the two new routes before `unshare`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
The hybrid update test now checks that the update's page and catalog
win and that the repair log says why, and that the linked file keeps
both revisions; the save test checks that the reopened file keeps the
update's page and catalog.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA
Main now reports unreadable objects from the parse of every object in
Document::reconstruct, so the pre-check that skipped that parse is gone.
The single-pass /XRef positions and the rarest-byte find stay.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
full_save_order retained up to 64 MiB of parsed objects for the write pass, which
reachable_objects then dropped unused. It now asks for no retention.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Overlapping unterminated dictionaries could each be read up to 1 MiB. A shared
budget (the file length) is now taken off on every path; a spent budget keeps the
object a candidate, as before.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The renderer (hayro-syntax (44)) counted the sections its chain reached, and
a hybrid section's /XRefStm stream (ISO 32000-2 §7.5.8.4) counted as one, so
a hybrid update without /Prev was rebuilt by cos but not by the renderer. It
now asks what cos asks: whether the newest section names an earlier one
through /Prev. The /XRefStm stream belongs to the same update.
Tests: a synthetic hybrid fixture (pdfcraft-testkit), with and without
/Prev, in cos and render.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA
An update whose cross-reference section has no /Prev (ISO 32000-2 §7.5.6,
§7.5.8.2) cuts the earlier revisions, and the page tree with them, off the
chain. The signed PDFBOX-2720 and PDFBOX-2942 files have this shape: the
document model found no page tree and the renderer showed 2 blank pages
found by scanning for page dictionaries.
When the chain reaches only the newest section, the file holds another
startxref and the catalog's /Pages can't be read, both readers now rebuild
the table from the file's objects, as for a damaged file:
- cos: the existing reconstruction (newest in-file definition wins), a
repair-log line saying why, and `Document::rebuilt_unlinked_update`.
- hayro-syntax (44): `root_xref` repairs the table before the page tree is
read; `Pdf::rebuilt_unlinked_update` says so.
- render: an open warning, and the lazy structure reader accepts such a
rebuilt file, so links, fields and bookmarks are read too.
A save rewrites the file in full (main's rule for reconstructed files).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA
- cos: a filter chain whose first stage stops at damage and whose
second reads all it was given is damaged.
- optimize: a page damaged partway stops resampling for the whole
document, so another page drawing the same image whole doesn't get
it resampled.
- redact: after redacting content with /DecodeParms by reference, no
stream in the saved file holds the text, read with its parameters.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
`DecodeBudget::decode_strict` read a stream's `/DecodeParms` only when
direct, so content behind a predictor whose parameters are an indirect
object decoded with the predictor's tag bytes left in. Redaction then found
no glyph under a mark, left the text in the file and reported the mark
applied. ISO 32000-2 lets those parameters be indirect.
`decode_strict` takes the document and reads through `decoding_view`, as
`decode_with_damage` does. Its other readers read the parameters too:
Remove Hidden Information's hidden text and layers, the audit's covered-text
check, and edit's backdrop scan.
The accessibility_mark_tag_artifact description says that content which is,
or may be, in a stream damaged partway is refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
`decode_with_damage` and `decode_contents_with_damage` read a stream's
`/DecodeParms` only when direct, as `Stream::decoded` does, so content behind
a predictor whose parameters, or a number in them, are an indirect object
decoded with the predictor's tag bytes left in, and was taken as whole. Edit
then listed and wrote back the wrong bytes, and pruning dropped an image such
a form draws. ISO 32000-2 lets those values be indirect.
- `decode_with_damage` takes the document and reads the parameters through
it, and so does `decode_contents_with_damage`, and `decode_contents` with
it: printing, accessibility's reading of a page and a page copied as a form
read them too. Code that calls `decode` on a stream itself doesn't yet.
- It reads through `Document::decoding_view`: `with_direct_parms`, but a copy
of the stream holds only its `/Filter` and the parameters, its data shared,
so a stream a page's `/Contents` lists again and again isn't copied whole
for each.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
Dylan M. Taylor
Michael Higgins
BFlat
Grzegorz MartensandClaude Opus 5.5
eliets2andeliets2
mtlcl410
quincy-jc
jonathanwxh-cell
mtlcl410andClaude Opus 5.5
FredApps
vangel76andClaude Sonnet 5.5
FredApps