Mutation testing of the PR against main left three mutants of image.rs alive:
the ICCBased arm of components deleted (an ICC image is then removed whole),
and data_elsewhere made false or inverted (a cleared copy then drops, or keeps,
/FFilter and /FDecodeParms whatever /F says).
an_icc_based_image_is_cleared_in_place_through_any_references clears an image
of one component and of three, its profile direct and through a reference to
a reference, and checks the samples. a_cleared_copy_keeps_parameters_for_data_
in_another_file_only_with_one checks a /F string, direct or by reference,
keeps them, and a null /F, a reference to null or to no object, or none, drops
them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
Main (#5569) decodes every image through cos's ImageBudget, which counts the
JBIG2 decoder's own work and the globals it reads, through main's own
restore_shared_jbig2 and image::without_globals for the images that share a
redacted image's /JBIG2Globals too. This branch replaces both with
globals::unshare, so the merge keeps unshare and drops them.
without_dictionary now decodes through ImageBudget::decode instead of
Document::decoded_image_within, which main made private. That closes the gap
the PR noted: the decoder's own work (up to hayro-jbig2's 2^30 steps an image)
was charged one unit of the run budget. apply passes its run's allowance, what
clearing left of it, as main's restore did. Past it apply is refused with
ImagesPastAllowance, as main's restore was. remove_hidden gives unshare an
allowance of its own (sanitize::unshare_unmarked), and past it is refused with
SharedJbig2Budget: "redact fewer pages at a time" means nothing there, and
Optimize's discard runs it too. SharedJbig2Budget's text now says finding and
storing again.
Tests: main's allowance test runs on unshare, with ImageBudget::with_work(0)
refused as ImagesPastAllowance, through unshare_unmarked as SharedJbig2Budget,
and u64::MAX storing one image. Test helpers decode through ImageBudget, since
Document::decoded_image is no longer public.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
A review of ecf028965 measured that decoding an image undoes the filters
of each dictionary it names again when cos doesn't hold their bytes (20
images naming a 32 MiB dictionary: 1.2 s at 2 units each). content()
now remembers what each dictionary decoded to, or its limit when it
couldn't, and an image is charged that before it is decoded.
Names gathered from many entries are held as a set as they are read, so
a million entries naming one chain hold it once. The claim that each
object is fetched once a run for each way it is read says its exception:
a /JBIG2Globals chain named part way is followed again from there.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
A review of fee25bfe4 found three costs the budget didn't count:
- A /JBIG2Globals reference in parameters given directly was fetched
again for every entry of every stream (2,000 entries, 53 KB: 40 s, two
thousand units of four million). What a reference leads to is now
remembered for the run under it, and a chain met part way takes what
is known from there.
- A /Filter name was kept whatever it was, so a 32 MiB name referred to
nine times was held nine times and copied for each image (725 MiB from
2.8 KB). Only a name a decoder knows is kept; any other is no filter,
and a name longer than any decoder's isn't looked at.
- A decode that failed spent nothing, so a dictionary one byte past its
limit escaped the budget. A failure now costs as much as it was allowed
to decode: a dictionary MAX_JBIG2_GLOBALS, an image its allowance.
The README says what the budget counts, and that past it the redaction
is refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
Reviews kept finding ways a small file could make this run long or large:
readable() fetched the same objects again for each image that names a
dictionary (~3.9 s each when two large objects push each other out of
cos's caches), and the memo of names grew with distinct arrays times
their entries (~3.7 GB from 2 MiB). Nothing bounded the run as a whole.
Everything unshare reads now goes through one Run: what a reference
leads to is fetched at most once a run for each way it is read (a size, a
filter, parameters, an entry, a flag, /F, a colour space, an ICC
profile), looked up before it is fetched and kept only as what is read
of it. Objects fetched, names remembered, and what is decoded (a unit for
each 4 KiB) count against RUN_BUDGET (2^22), and past it the run is
refused with SharedJbig2Budget. A scan reads a few objects a page, so a
book of thousands of pages sharing one dictionary stays well within it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
f5aa20f7a keyed what Names remembers by the object references end at,
but followed them all before looking: every hit fetched the object
again. With two large objects that push each other out of cos's caches,
each fetch decoded 17 MiB, and a 66 KiB file took 26 s where it took
0.6 s. Each reference is now looked up before it is fetched, and what
it leads to is remembered under every reference on the way, with the
key of the last: a hit fetches nothing, an alias or a chain only the
references before what is remembered. The test counts fetches.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
`ImageBudget::decode` is now the one public way to decode an image's
CCITT fax or JBIG2 data: `Document::decoded_image` and
`decoded_image_within`, which decoded with no limit on the work, are
test-only in pdfcraft-cos, and pdfcraft-filters exports only
`decode_ccitt_within` and `decode_jbig2_within`, which spend from a
caller's allowance (`decode_jbig2` is gone, `decode_ccitt` private).
So an operation over many images can't decode them without sharing an
allowance; the compiler holds new callers to it. Doctests that must not
compile check both crates; with the old entry points public they fail.
Tests outside the crates decode through an unlimited ImageBudget.
Streams in general (`Stream::decoded`) are left for a follow-up.
Names kept what a value names by its first reference, so streams naming
one array each through an object of their own that refers to it missed
the memo every time: quadratic time, and now memory too (8000 such
streams with 8000 entries, 1.4 MB: 30 s, 504 MiB). It is keyed by the
object the references end at, so another reference, or a chain, finds it
(the same file: 0.29 s, 7 MiB, measured by the review). matches and
added use the same key.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
Three ways a small file could still make a redaction run long or large:
- direct_parms kept /JBIG2Globals as given, so a direct array there was
copied for each entry (427 MiB from 32 KB). It is kept only when it is a
reference, as Document::with_direct_parms keeps it; a direct value is
no stream, so no decoder reads it.
- The entries copied were bounded only by /Filter's length, and the work
repeated for every stream that uses a dictionary: time grew with the
square of the file (117 s from 249 KB). A /Filter longer than any
decoder takes (MAX_CHAIN, and a /Crypt) is read as no filter, so its
parameters aren't read either; no decoder reads such a stream anyway.
- named() read every entry of every stream's parameters, through up to
33 references each, with nothing shared between streams. Names keeps
what each value the file refers to names, for the whole run, so streams
sharing an array or an entry cost it once; unshare adds a shared list,
and finds what it matches, once too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
It reserves room for a translated "Mixed", and in Czech or French that pushed the alignment
button up to 54 pt past the Tools pane. The field is now no wider than SIZE_FIELD, and a longer
translation scrolls in it. The language test checks the size row too.
Conflict with main's rail docking, resolved by keeping both: the rail follows the side chosen
for it (main) at 49 pt with 1 pt padding, and main's dock button stays above the page display
button, while zoom out and in stay side by side under the zoom field.
direct_parms copied every value it followed, and direct() every /Filter
entry, so each reference to one large array copied it again: about 5.5 MB
a reference, 11 GB from a file of about 225 KB. Now, as
Document::with_direct_parms does, a size is a number, /Filter a name or
an array of names, and direct_parms() keeps of each entry the numbers and
flags decoders read (DECODE_PARMS_KEYS, now public in cos) and
/JBIG2Globals as given, for as many entries as there are filters.
Anything else is null.
A kept image whose own /DecodeParms come through a chain is stored again
with them, which 2eddb1bc9 made so: what_stays tests it now.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
Conflict with main's hotbar, resolved by keeping both: the bar keeps main's offset past a hotbar
docked on the left or top, and centres in the margin that starts after it.
Conflicts with main's font menu, size field and accessible names, resolved by keeping both:
the font list keeps main's searchable installed-fonts menu inside its outline; the line spacing
list and number fields keep their names. The size field now reserves room for "Mixed", so
MIN_ROW counts it (62 pt, measured for English), and the spacing columns take their width from
the font list so a longer translation of "Mixed" can't widen them.
content() decoded a dictionary through with_direct_parms, which reads
/DecodeParms and its entries one reference deep, while named() follows
the chain. So a copy of the dictionary whose parameters came through a
reference to a reference decoded to other bytes, wasn't known for a copy,
and stayed in the file with Ok. readable() now gives /DecodeParms, each
entry and each value through followed, /JBIG2Globals aside, so a copy
decodes as a reader that follows references decodes it.
With the copy known, the image naming it has to be stored again, and
PdfCraft decodes that image's symbols with the copy's parameters one
reference deep, so it can't: the run is refused, where it went on with
the copy in the file.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
/F, /Width, /Height, /Filter and its entries, /ColorSpace and an ICC
profile were resolved one reference deep, while /DecodeParms and
/JBIG2Globals follow a chain (followed). So /F through a reference to a
reference to null counted as data in another file and refused the run,
and a /ColorSpace chained to /DeviceRGB wasn't known to have three
components, so a JBIG2 image naming it was stored as one bit, where main
refused it. All now go through followed, which a ring ends.
image::components serves clear too, so an image whose colour space is
given that way is now cleared, where it was removed whole; clear's
/Decode for a stencil mask, read one reference deep, follows the chain
as well, so a chained [1 0] isn't missed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
Found while testing crates/ui-egui/src/panels/page_selection_ui.rs:24-27 (parse, `number` closure)
Co-authored-by: Claude Haiku 5.5 <noreply@anthropic.com>
* A rebuilt xref finds an object whose header names another generation (#5580)
`poppler-85140-0.pdf` is 340 bytes with no cross-reference table, and the
header of its one page reads `3 18446744073709551616 obj` while the page tree
points at `3 0 R`. Before #427 the digits wrapped and 2^64 wrapped to exactly
0, so the header read as `3 0 obj` and the file opened by accident. Once an
overflowing integer became a real, the object was unreachable and the file
stopped opening at all, which `cargo xtask check` reports as a regression.
The generation is not really the point. `3 99999 obj` with the same page tree
is refused in exactly the same way on main -- the control for the new test
fails on that row first, with "page 1 does not exist" -- so this is a
mismatched generation in a rebuilt table, not an integer overflow. A table
rebuilt by scanning the file has no revision history to tell two generations of
an object apart, and a file that needs rebuilding references everything as
`N 0 R` whatever its headers say.
So, in the reconstruction path only:
- a generation number that cannot be read is taken as 0, the value such a
file's references use (`read_object_header`). A generation is at most
65,535 (ISO 32000-2 §7.5.4), so there is nothing to lose by it;
- every scanned object is also filed under generation 0, with `or_insert` so
a real `N 0 obj` is never displaced by the alias;
- the object is read back behind the same lenient header, since reading it
parses the header again and the strict identifier check rejected it
(`read_repaired_body`, gated on `repaired`).
Reading numbers anywhere else is unchanged, and a table read from the file's
own xref keeps the exact identifier match. The scan already files objects found
in object streams under generation 0, and cos's table parser already takes an
unreadable generation as 0, so this follows what the code does elsewhere.
The test covers generations 0, 99999 and 2^64 with no table and no `startxref`,
and asserts the page is found at its own size. Its fixture is built in the test
rather than taken from the corpus, so it holds without the pinned checkout.
* The generation-0 stand-in is one key, not a second object
Two consequences of filing a scanned object under generation 0 as well,
both from review.
`XRef::objects` walks the table's keys, so an object with a stand-in came out
once per key: a document whose page tree cannot be read is recovered by looking
for objects that are pages, and it reported the page twice. Enumeration now
yields one object per location, after the sort it already does. Both keys stay
for lookups; only this walk is deduplicated.
And `or_insert` never replaces, so a later `3 1 obj` moved the exact `3 1` key
to the later body while the `3 0` stand-in stayed on the earlier one -- `3 0 R`
and `3 1 R` would then read different objects. The scan now tracks which
generation-0 keys are stand-ins: a real `N 0 obj` takes the key and keeps it,
and a later header for the same number replaces a stand-in.
The tests check both through what comes out: page 2 must not exist for a
single-page file recovered without a page tree, and with two `3 1 obj` headers
the page comes back at the later body's 300x150 rather than the earlier
200x100.
* M2: the repair scan keeps its place on a non-header, and an object stream's first member is enumerated (#5580)
- read_object_header's lenient path works on a clone, so `42 3 0 obj` no longer costs the
header after the stray number (CodeRabbit, minor).
- XRef::objects deduplicates by location and object number: an object stream and its first
member share (offset, 0) and are two objects (CodeRabbit, minor).
- vendor/README.md: hayro-syntax patch (34) and its tests.
Both new tests fail with their fix reverted.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A /F that is null, by reference to null or to no object, is as if absent,
so an image with one is stored again, not refused as data in another file.
The check is image::data_elsewhere.
A copy stored again kept /FFilter and /FDecodeParms, which named now
reads, so the dictionary stayed through the copy and the run refused,
blaming something else. A cleared copy kept those and /DP. Both drop them,
the cleared copy only when its data isn't in another file.
A dictionary that names itself isn't counted as something that uses one
when the run is refused, as it already wasn't when it goes on.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
/FDecodeParms are the parameters of data in another file, which a reader
may draw instead of the stream's own. A dictionary that only a removed
image named there stayed in the file with Ok. It is now read with
/DecodeParms and /DP, and an image whose data is in another file counts
as one that can't be stored without the dictionary: readers draw that
file, not a copy stored here.
images_restored counts what was stored again. A dictionary that names
itself as its own globals (malformed) leaves with what used it but can't
be stored, and was counted.
The table's /Mask route gets a row in the routes test, as it was only
measured.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
A kept image that named the dictionary added every dictionary it names to
what had to leave, in one pass. An image using only one of those, earlier
in the file, had been passed over, so it kept that dictionary and the
redaction refused, saying something else referred to the data. In the
other order it was stored again. Now only the names that matched have to
leave, so another dictionary stays with what uses it, in either order.
The variant's doc says what is counted: something that names the data,
JBIG2 or not.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
When nothing that names the dictionary is stuck and something else refers
to it (a private key, say), the refusal said 1 item(s) couldn't be stored
without it. It now counts 0 and says something else refers to the data.
remove_hidden's refusal gets a test: the comment has gone by then, and the
text doesn't say nothing was changed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
* M7: make page-mark replacement transactional (#5657)
* ROADMAP: drop the header line (the log entry stays)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* Fix Matrix::invert rejecting valid matrices with a small scale
Found while testing crates/content/src/lib.rs:732-735 (Matrix::invert)
Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
* Matrix::invert: a matrix whose inverse overflows has none
A subnormal determinant passes the relative check, and dividing by it
overflows (CodeRabbit on #5703).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Haiku 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
* fix(ui): don't check recent files on the UI thread at startup (#5682)
Restoring settings dropped recent files whose files no longer exist by
asking the file system about each one on the UI thread, before the first
frame. On a network mount that has stopped answering (davfs2 in the
report) that call blocks in the kernel, so the window never appears.
Settings now keep every recent entry when they load. Once the window is
up, a worker checks the restored paths and each frame drops the ones it
has found gone. An entry it hasn't reached, or can't tell about (the
mount answers with an error), stays listed. A worker stuck on a dead
mount stays stuck, but nothing waits for it. This follows the pinned
folders' worker.
Fixes#5682
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Recent files: a file opened while the check is pending stays listed
The worker's answer that a restored path is gone is out of date once the
user opens that path; keep the new entry (CodeRabbit on #5692).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
- Replace cluttered settings list with clean, modular card containers
- Integrate interactive real-time search box flush with card boundary
- Combine page display and continuous scrolling into a unified segmented pill
- Enable smooth scrolling with dedicated animation duration
- Enhance vertical scrollbar spacing and layout geometry
- Update unit and integration tests across layout, appearance, and theme
- Credit @Combiler01 in contributors/people.toml
apply works on a copy, so nothing changes when it refuses, but remove_hidden
isn't transactional: a direct caller's document is part changed by then. The
message said nothing was changed for both. The app's callers work on copies.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY
Redaction starts from the images a run replaces or removes and the forms
and soft masks under them, so a dictionary used by something that left
another way stays, with apply returning Ok: a stamp or widget appearance, a
Type 3 glyph, an /Alternates entry, a /Mask, or an image with /Filter by
reference or /DecodeParms through a reference to a reference. A
byte-identical copy stays too, which is what pdfseparate then pdfunite
leaves, and Remove Hidden Information never takes a dictionary out.
Compare what a full save would keep before and after the run instead, the
same walk write_full makes, now cos::reachable. What left names the
dictionaries, in any entry of its parameters; what stays that names one, or
a copy, is stored again without it; and the walk is taken once more, so a
dictionary still kept refuses the run, whatever keeps it. An image with
/Width, /Height or /Filter by reference is read, and is one bit in the copy,
as it was when drawn. /Alternates stays with what is stored again, and a
stream that isn't JBIG2 refuses rather than being stored.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY