andodeki
00f1dfbc12
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.
7 KiB
ADR 0003: PDF incremental save — append revisions, never rewrite
- Status: Accepted
- Date: 2026-07-28
- Review item: Phase 8,
DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md - Supersedes: nothing
- Related:
REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.mdphases 3, 4 and 7
Context
Phases 0–7 are complete. The viewer parses documents, renders them, and edits
form fields through DocumentFormEditor with real appearance regeneration.
None of that can be saved. There is no public API anywhere in the four PDF crates that writes an edited document back out:
$ grep -rn "pub fn" pdf-document/src/*.rs | grep -iE "save|write|serial"
(no matches)
PdfWriter and PdfDocBuilder can only author a whole new file from
scratch, which is how the test fixtures are produced. They cannot represent
"this existing document, plus a change".
So the product currently ships an editor whose edits are discarded. That is the same class of defect Phase 0 was created to remove — an API that claims a capability it does not deliver — and it makes the Phase 3 and Phase 4 work demonstrably useful only inside a single session.
Phase 8 lists ten independent features. Incremental save is chosen first for three reasons:
- Its prerequisites are already met. The review lists "writer/serializer, byte-preserving tests, signature preservation". The writer exists, the test infrastructure exists (Phase 6), and there are no signatures to preserve yet precisely because signing is not implemented.
- It unblocks two other Phase 8 features. Both annotation editing and AcroForm full support list incremental save as a prerequisite. Neither can start until this exists.
- Nothing else closes the credibility gap. Encryption and advanced colour add new capability; this one makes existing capability real.
Decision
Implement saving as an incremental update — the original bytes are copied verbatim and a new revision is appended — rather than as a full rewrite.
A PDF incremental update appends only the changed objects, a new
cross-reference section, and a trailer whose /Prev points at the previous
xref offset. Readers follow the chain backwards, so unchanged objects resolve
to their original bytes.
Why append rather than rewrite
A full rewrite is simpler to implement and wrong for this product:
- It destroys anything the parser does not understand. This parser does not yet model optional content groups, structure trees, embedded files or digital signatures. A rewrite would silently drop all of them. An append physically cannot: the original bytes are still there.
- It invalidates every existing signature. A signature covers a byte range of the file. Appending preserves that range; rewriting does not. Signing is not implemented today, but a save format that structurally precludes it is a dead end.
- It is unauditable. With an append, the previous revision is still in the file and the diff is the appended bytes. A reviewer can see exactly what an edit changed.
The cost is file growth on repeated saves. That is the correct trade: a larger file is recoverable, a destroyed one is not.
Scope of this ADR
In scope:
- Append a revision containing modified objects.
- Update form field values (
/V) and appearance streams (/AP). - Emit a valid xref section chained through
/Prev. - Preserve the original bytes byte-for-byte.
- Round-trip: the saved file reparses and observes the edit.
Explicitly out of scope, to be separate work:
- Cross-reference streams (
/Type /XRef). Only classic xref tables are written. A document that uses xref streams can still be saved, because a classic table appended to it remains readable, but this is recorded as a known limitation rather than glossed over. - Object streams (
/ObjStm) for the appended objects. - Encryption. Saving an encrypted document is refused, not attempted: writing plaintext objects into an encrypted file would silently corrupt it and could leak content that the document's own security policy protects.
- Signature creation or re-signing.
- Compaction or garbage collection of superseded objects.
Non-negotiable rules for this feature
These extend the review's existing rules and are enforced by tests:
- The original bytes are a prefix of the saved file. Asserted directly, not by inspection.
- A save with no pending edits is a no-op, returning the input unchanged. Appending an empty revision would grow the file for nothing and churn its modification date.
- An unsupported document is refused with a typed error, never saved partially. Encryption is the first such case.
- Every appended object is reachable. A revision that writes an object no trailer points at is a leak and a bug.
- Round-trip is the acceptance test. Save, reparse from bytes, and assert the edit is observable. Anything less tests the writer against itself.
Merge criteria
This feature merges when all of the following hold:
DocumentFormEditoredits can be saved and reparsed, with the new value observable through the normalacroform()path.- The original document's bytes are an exact prefix of the output.
- Saving with no edits returns the input unchanged.
- Saving an encrypted document returns
SaveError::Encrypted. - Appearance streams regenerated in Phase 3 are written as real stream objects and referenced from their widgets.
- Corpus round-trip: every
forms/fixture survives edit → save → reparse. - A saved file survives a second edit-and-save cycle, producing a chain of three revisions that still reparses.
- Malformed and truncated corpus fixtures either save correctly or return a typed error; none panic.
TEST_TARGET=pdf ./tools/test-rust-clean.shpasses, rustfmt and clippy-D warningsclean.
All criteria met. These boxes went unticked when the feature shipped; they
were audited afterwards against the code and each maps to a named test:
byte-prefix to save_roundtrip.rs and incremental.rs
the_original_bytes_are_a_prefix_of_the_output, the no-op save to
report.unchanged, appearance streams to
a_regenerated_appearance_is_written_as_a_real_stream, and the corpus
round trip to every_form_fixture_round_trips.
Consequences
Positive. Form editing becomes a real feature. Annotation editing and full AcroForm support are unblocked. The document model gains a serialisation path that a future signature implementation can build on without redesign.
Negative. Files grow on every save. The parser must handle its own output, which means the xref chain code is now load-bearing in both directions. Documents using xref streams get a hybrid file — valid, but inelegant — until that limitation is lifted.
Risk. The most likely defect is an offset error in the appended xref, which produces a file that opens in this viewer but not in Acrobat. The mitigation is a round-trip test that reparses from bytes rather than reusing in-memory state, plus byte-prefix assertions that catch accidental rewrites.