Three defects found by auditing the ADR merge criteria against the code
rather than against memory.
1. The scheduled fuzz job ran five hardcoded targets. Four had been
added since and were never fuzzed: parse_revision_chain, decrypt,
eval_function and parse_colorspace. eval_function is the sharpest of
those - it executes PostScript taken verbatim from an untrusted file.
The list now comes from `cargo fuzz list` and the job fails rather
than passing vacuously if it comes back empty.
`cargo fuzz list` reads the manifest, so a target file added without
its [[bin]] entry would still be skipped silently. The engine job,
which runs on every push, now checks the two agree. Both directions
of that guard were exercised before committing.
2. ADR 0004 rule 3 promises an annotation whose appearance cannot be
generated "keeps its original /AP and is reported as skipped". The
keeping worked - to_dict clones the source dictionary - but nothing
reported it: SaveReport only tracked skipped appearances for form
fields. A caller who moved a stamp was never told its artwork still
showed the old position. Adds
SaveReport::annotation_appearances_skipped and
appearance_is_generated, which enumerates the out-of-scope types
explicitly so a new AnnotationType fails to compile until classified.
3. SetContents and SetFlags had no round-trip test. Both were
implemented and unit-tested against the in-memory model, but neither
was ever reparsed from written bytes - the assertion ADR 0004 calls
central.
New fixture annotations/stamp.pdf carries real /AP artwork for a Stamp
(undrawable: must be preserved and reported) beside a Square (drawable:
must not be reported), so the reporting cannot pass by reporting
everything. The stamp test was mutation-checked: it fails when the
reporting line is removed.
ADR 0003 and 0004 merge criteria are now ticked. 0003's were genuine
paperwork - every box traced to an existing named test. 0004's were not,
and its ADR now records what was missing rather than implying it always
worked.
TEST_TARGET=pdf 447 -> 451 passing. rustfmt and clippy -D warnings clean.
Phase 8 feature 2 of 10, designed in
REVIEWS/adr/0004-pdf-annotation-editing.md. Its prerequisites are "document
model, appearance generation, incremental save"; the first two landed in
Phase 3 and the third in ADR 0003, so this was the ready one.
Chosen ahead of the other unblocked feature, AcroForm full support, because
the review lists that one as needing JavaScript actions. Running
document-supplied code is a large new dependency and a security surface that
deserves its own ADR and threat review rather than arriving as a side effect
of finishing a form feature.
Annotations were strictly read-only: the module had public fields and
from_dict, and not one mutator or &mut self method. The viewer could report
a click on a link but could not move a highlight, restyle a square or delete
a stamp.
New pdf-document/src/annotation_edit.rs, deliberately the same shape as
DocumentFormEditor so a caller wiring a drag gesture does not have to learn
a second contract:
- AnnotationEdit covers Move, Resize, SetColor, SetInteriorColor,
SetBorderWidth, SetOpacity, SetContents, SetFlags and Delete.
- Every edit is validated before anything changes, so a rejected edit leaves
the annotation untouched. Degenerate and non-finite rectangles, colours
outside 0..1, negative border widths and out-of-range opacities are all
refused with typed errors.
- A degenerate rectangle is refused rather than silently normalised: it
usually means a bug in the UI upstream, and quietly fixing it hides that.
An inverted but valid rectangle is normalised on store, so hit testing and
appearance sizing never see one upside down.
- Read-only annotations refuse edits unless the caller opts in through an
explicit allowing_read_only(), with one exception: clearing the read-only
flag itself is permitted, or a locked annotation could never be unlocked.
- Colour reading converts the grey and CMYK forms of /C to RGB, since the
array length selects the space.
Identity: PdfAnnotation gains an obj_ref, because an index into /Annots is
not stable across a save. Populating it exposed a real bug in
page_annotations: it called self.resolve() on the /Annots array, which
recurses and replaced every entry with its dictionary, destroying the
references. It now resolves only the array itself.
Saving: save_annotation_edits appends a revision through the ADR 0003
writer. Deleting rewrites the page dictionary so the reference leaves
/Annots, because an object that stops existing while the page still points
at it produces a file other readers reject.
Out of scope and recorded in the ADR rather than implied: creating new
annotations, appearance generation for types this crate cannot draw (a
Stamp keeps its existing /AP rather than being blanked), applying
redactions, and rich text.
Tests: 23 unit tests plus 13 corpus acceptance tests covering the ADR merge
criteria. The round trips reparse from the written bytes rather than reusing
in-memory state, assert a deleted annotation is gone from the reparsed
page's /Annots and not merely from the model, that unrelated annotations
survive a deletion, that two saves chain, and that editing then saving never
panics on the malformed corpus.
Validation:
TEST_TARGET=pdf ./tools/test-rust-clean.sh (354 tests)
TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (398 tests, 6 ignored)
Both rustfmt and clippy -D warnings clean.
