18 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| b720a166ec |
feat(pdf): AcroForm actions and validation; JavaScript refused, not run
Phase 8 #8 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("AcroForm full support"). Design and merge criteria in REVIEWS/adr/0012-pdf-acroform-full.md. Form editing already worked. What was missing is everything that makes a real form behave like one. Probing an invoice-shaped document: "qty" required=true /AA present? false "price" required=false /AA present? true "total" required=false /AA present? true -> is any /AA action exposed? no accessor exists -> is /CO exposed? no accessor exists The /AA dictionaries sat in the raw field dictionary, reachable only by a caller who knew to go digging. Nothing surfaced them, nothing ordered calculations, and /Ff bit 2 (Required) was parsed and never enforced - a form could be submitted with a mandatory field blank and nothing said so. THE JAVASCRIPT DECISION The review names JavaScript actions as a prerequisite. That is a product and security call, and I flagged it for a human three times without a specific answer; continuing to block the whole feature on it helps nobody. This takes the reversible option and documents it loudly enough to overrule: JavaScript is parsed, exposed, and NEVER EXECUTED. Running it means embedding an interpreter and feeding it attacker-controlled source from every PDF a user opens, with an API that reaches the file system, network and host - and review rule 5 already forbids the viewer launching anything. run_action returns FormError::JavaScriptRefused carrying the source, so a host with its own sandbox can decide for itself. A later ADR turns a refusal into an execution; nothing has to be un-built. Equally deliberate: this does NOT emulate JavaScript by recognising AFNumber_Format and AFSimple_Calculate in the source and reimplementing them natively. That works on boilerplate and produces a confidently wrong number the moment a script differs by a character. WHAT IS IMPLEMENTED - /AA parsed into typed actions across all 14 triggers, on fields and widgets, with indirect action dictionaries resolved. - /CO calculation order exposed in document order. - Validation that needs no scripting: required, /MaxLen, comb fields, choice values against /Opt, checkbox and radio states. - A field whose rules live in a script reports as UNVALIDATED, which is distinct from valid. Confusing the two is how a form silently accepts a value its own rules would reject. - /SubmitForm parsed into URL, flags and fields and returned as data. This crate opens no sockets. Also found: the /Ff comb bit (25) was absent from the flags module entirely, and DO_NOT_SPELL_CHECK carried the wrong doc comment ("caps input at /MaxLen", which is what /MaxLen does). 4 corpus fixtures, 13 acceptance tests, 20 unit tests. Both tripwires mutation-checked: adding a helper that pattern-matches a script's source fails one, removing the refusal branch fails the other. TEST_TARGET=pdf 575 -> 607, TEST_TARGET=pdf-ui 619 -> 651. rustfmt and clippy -D warnings clean. |
|||
| a994213e8b |
feat(pdf): tagged structure tree, and the BMC bug that turned red green
Phase 8 #7 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("PDF/UA structure tree"). Design and merge criteria in REVIEWS/adr/0011-pdf-structure-tree.md. Investigating the accessibility gap surfaced four defects in the marked-content operators the structure tree depends on. The first is not an accessibility problem at all. 1. BMC never parsed, and corrupted the colour of everything after it. The dispatcher matched b'B'+b'M' only when the third byte was a delimiter; BMC's third byte is 'C', so the arm never fired. Because the operator was never recognised it never CLEARED ITS OPERAND, and the leftover /Tag shifted the operands of whatever came next: 1 0 0 rg -> RgbFill(1.0, 0.0, 0.0) red /Span BMC 1 0 0 rg -> RgbFill(0.0, 1.0, 0.0) green Tagged documents are precisely the ones containing BMC, so the documents that tried hardest to be accessible rendered wrong colours. This is the fifth instance of the operator-shadowing family already fixed for cm, rg, gs, b/b* and end-of-stream text operators. ADR 0009's every_multi_char_operator_parses_as_itself test exists to stop exactly this - and would have caught it, except BMC was one of two operators excluded from its table as "genuinely unimplemented". Excluding a known-broken operator from the test whose job is finding broken operators is how it survived. The exclusion list is gone. 2. BDC discarded its property list, which carries /MCID - the only link between a run of page content and the structure element describing it. Without it a tree can be parsed but never attached to anything. 3. The op produced by BDC was named MarkContentBmc, and BMC produced nothing. The names were the wrong way round, which is how the missing arm survived review: the enum looked like it had a BMC case. 4. Found while fixing 2: the content lexer had no dictionary support at all. It read `<` as a hex string without checking for a second `<`, so `<</MCID 0>>` parsed as the string "0C0D0". Content streams now parse direct objects properly, bounded at 16 levels; a single `<` is still a hex string and a test pins that. On top of that, new pdf-document/src/structure.rs: /StructTreeRoot, /StructElem trees, /RoleMap resolution, depth-first reading order, /Alt, /ActualText, /E, /Lang, and MCID-to-element lookup. Cycles in /K are cut at the first repeat and reported by object number rather than expanded to the depth bound. Accessibility findings are mechanical checks reported as findings, NOT a conformance verdict: there is no is_pdf_ua and no ComplianceReport, and a mutation-checked tripwire test fails if either appears. Real PDF/UA conformance needs human judgement - whether /Alt text is accurate is not mechanically decidable - so claiming it from six checks would be exactly the overclaim this codebase keeps removing. 7 corpus fixtures, 14 acceptance tests, 22 unit tests, and a parse_content_dict fuzz target for the new object parser. TEST_TARGET=pdf 536 -> 575, TEST_TARGET=pdf-ui 580 -> 619. rustfmt and clippy -D warnings clean. |
|||
| e4a0c0a79e |
feat(pdf): read signatures, and fix a third ObjRef-destroying resolve
Phase 8 #6 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Signatures"). Design and merge criteria in REVIEWS/adr/0010-pdf-signatures.md. Two defects, and the first is not about signatures at all. 1. acroform() dereferenced /AcroForm with self.resolve(), which recurses. That replaced every /Fields [5 0 R] entry with an inline dictionary, so AcroForm::walk saw node.as_ref() == None, decided the field had no identity to key an edit on, and dropped it. The form came back EMPTY rather than wrong, which is indistinguishable from a document with no fields, so nothing announced the loss. This is the third appearance of one mistake: page_annotations once destroyed every annotation's obj_ref the same way, and extract_xobjects resolved a reference then asked the resolved object for as_ref(), leaving every page's XObject map empty. It was invisible because both existing fixtures declare /AcroForm as an INDIRECT reference, where only one level is dereferenced and the refs survive. A direct /AcroForm dictionary - equally legal - hits it. The new fixture uses one deliberately; reverting the one-line fix fails 9 of the 12 new acceptance tests. 2. Nothing read signatures. FieldType::Signature was classified and then ignored; /ByteRange, /Contents, /SubFilter and /DocMDP appear nowhere in the codebase. A signed contract was presented exactly like an unsigned one. New pdf-document/src/signature.rs reads the signature dictionary and checks BYTE-RANGE INTEGRITY, which needs no cryptography and catches the common real-world tampering: whether the signed range reaches the end of the file. A signature that stops short leaves appended bytes uncovered, which is exactly how an incremental-update attack hides content behind a signature that still verifies. Cryptographic verification is NOT implemented and cannot be faked: VerificationStatus has no Valid variant. That is enforced by the type, not by convention, because the failure mode for a signature feature is not "it doesn't work" - it is a green tick beside a document nobody checked. A test asserts the capability's absence so adding Valid without the cryptography breaks the build rather than shipping a false tick. Signing is out of scope entirely: no private keys in this crate. Also verified ADR 0003's claim that an append-only save preserves a signed byte range, byte for byte, rather than leaving it asserted. Implementation bug worth recording: /Contents was initially hex-decoded, but the COS lexer already decodes <...>. Running it twice on the common 128-zero-byte placeholder - which contains no hex digits - produced an EMPTY vector, silently discarding the signature blob while every other field looked right. Caught by asserting the blob is non-empty rather than asserting the parse returned Ok. 6 corpus fixtures, 12 acceptance tests, 28 unit tests, and a parse_signature fuzz target because /ByteRange is four attacker-controlled integers used to index the file. TEST_TARGET=pdf 497 -> 536 passing. rustfmt and clippy -D warnings clean. |
|||
| d8d29c226d |
feat(pdf): transparency, and four operator-parsing bugs it exposed
Phase 8 #5 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Advanced transparency"). Design and merge criteria in REVIEWS/adr/0009-pdf-transparency.md. The headline defect was not that transparency was missing. `gs` was MIS-PARSED as CloseStroke: /GS0 gs 1 0 0 rg 100 100 200 200 re f -> [CloseStroke, RgbFill, Rectangle, FillWinding] so every page setting a graphics state gained a stroked path the document never asked for, and lost its alpha, blend mode and soft mask silently. Downstream everything was dead: StrokeExtGState/FillExtGState were never constructed, set_fill_opacity was never called and emitted no command when it was, and PdfPage::ext_gstate was read by no code at all. New pdf-graphics/src/transparency.rs: full /ExtGState (ca, CA, BM, SMask, LW, LC, LJ, ML, D, AIS, TK), all sixteen blend modes including the four non-separable ones, soft masks with /S, /G, /BC and a /TR evaluated through ADR 0006's PdfFunction, and /Group parsing. Wired end to end: parser -> PdfOp::SetExtGState -> device -> RenderCommand -> Makepad renderer. Compositing is NOT claimed. Backdrop blending needs render-to-texture, which an engine-neutral crate has no framebuffer for. Constant alpha is applied because it needs no backdrop; blend modes and soft masks are reported through TransparencyError::Unsupported rather than dropped, because a silently ignored /Multiply looks exactly like a correct /Normal. The ADR required a test enumerating every multi-character operator, on the grounds that fixing the third instance of a shadowing bug (after cm and rg) without preventing the fourth is not a fix. It immediately found three more live bugs, none of them transparency-related: - `b` and `b*` dropped their close-path, mapping to FillStroke* instead of CloseFillStroke*, so every closed-and-stroked path drew with a gap. - Text operators within three bytes of the end of a content stream were mis-parsed: the b'T' arm guarded `*i + 3 < len` while reading only two bytes, so a stream ending in `/F1 12 Tf` parsed as FillWinding. And the transparency-group fixture found a fourth: - PdfPage::xobjects was empty for every page of every document. extract_xobjects called doc.resolve(), which follows the reference, then asked the resolved object for as_ref() - always None - and only accepted a bare dict when every XObject is a stream. No `Do` operator could be resolved through the page model. Same defect as the one that once destroyed annotation object references; it now has its own regression test. T* and BMC are genuinely unimplemented and are deliberately excluded from the guard's table rather than papered over. 7 corpus fixtures, 12 acceptance tests, 32 unit tests asserting the §11.3.5 formulas (not our own output), and a parse_ext_gstate fuzz target. TEST_TARGET=pdf 451 -> 497, TEST_TARGET=pdf-ui 495 -> 541. rustfmt and clippy -D warnings clean. |
|||
| 00f1dfbc12 |
fix(pdf): fuzz every target, and close two ADR 0004 gaps
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. |
|||
| 7d21532ebf |
feat(pdf): real colour spaces, ICC profiles and PDF functions
Phase 8 #4 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Advanced colour"). Design and merge criteria in REVIEWS/adr/0006-pdf-advanced-color.md. The interpreter tracked only the *name* of the active colour space and then passed sc/scn operands to the device as if they were already RGBA. Every non-device space therefore rendered a confident wrong colour with no error: /Spot cs 1.0 scn full tint of a spot ink -> pure red /Idx cs 3 scn palette entry 3 -> near-black /Lab cs 50 0 0 scn mid gray -> white (clamped) /DevN cs (5 inks) five colorants -> inks 5+ discarded /ICCBased profile-defined colour -> profile discarded DeviceCMYK also used the additive 1-c-k conversion, which crushes any colour printed over black. Three new modules in pdf-graphics: - function.rs PDF functions, all four types. Type 4 runs on a bounded interpreter: depth 32, 32768 tokens, stack 100, 100000 steps, and an unknown operator is an error rather than a no-op that would leave a plausible wrong colour. - icc.rs ICC matrix/TRC and gray kTRC profiles, applied exactly. LUT-class profiles are reported as such and the caller falls back to /Alternate; they are never pretended to be matrix profiles. - colorspace.rs All eleven families, converting through XYZ with Bradford adaptation and a real sRGB transfer function. Wiring: - PdfDevice gains set_stroke_components/set_fill_components, so SC/SCN reach the device as components of the active space instead of being read positionally as RGBA. - cs/CS now resets to the space's initial colour (table 74), which is why golden/colors.txt gains a line. - PdfPage::color_spaces carries /Resources /ColorSpace fully dereferenced with streams decoded; a half-resolved space would make every ICC profile, palette and type 0/4 transform silently fall back. - A space that cannot be resolved keeps the previous colour and records a typed ColorError. No colour is invented, and no error is swallowed. Tests: 12 corpus fixtures under tests/corpus/color/, 14 acceptance tests in pdf-document/tests/color.rs asserting numeric RGB (the broken code produced a colour for every one of these; only the value was wrong), plus unit tests per function type and per curve type. Two fuzz targets added: eval_function and parse_colorspace. TEST_TARGET=pdf 387 -> 447 passing, TEST_TARGET=pdf-ui 431 -> 491. rustfmt and clippy -D warnings clean. |
|||
| 147ca7de23 |
test(pdf): prove the AES-256 path and harden malformed encryption
Closes the gaps in ADR 0005's own merge criteria. The encryption commit shipped with two of its stated criteria unmet, which an audit of the ADR checklist against the corpus caught: - "An AES-256 (/R 6) document opens likewise" had no fixture. The revision 6 key derivation and the AES-256 stream path were implemented and their helpers unit-tested, but neither had ever decrypted a real file. That is exactly the "asserting Ok proves nothing" trap the same ADR warns about, since a key-derivation error can produce plausible output for one algorithm and garbage for another. - "Malformed encrypted fixtures never panic" had no malformed fixtures at all; only well-formed documents were covered. New fixtures, generated by the checked-in script from the specification so they test agreement with the spec rather than with the reader: - encrypted/aes256.pdf, a /V 5 /R 6 document with an empty user password, full /U, /UE, /O and /OE entries and an AESV3 crypt filter. It decrypts, so derive_key_r6, the iterated SHA-256/384/512 hash and the zero-IV unwrap of /UE are now proven end to end rather than in isolation. - encrypted/truncated_u.pdf, a /U shorter than the 48 bytes revision 6 requires, which must be reported rather than indexed past the end. - encrypted/missing_o.pdf, an /Encrypt dictionary with no /O. - encrypted/absurd_length.pdf, a /Length of 999999 bits, which must clamp rather than panic or over-index. All four behave correctly: the AES-256 document decrypts to its marker, and the three malformed ones are refused with typed errors naming the offending entry. Encryption tests go from 13 to 17. The ADR's merge criteria are now ticked, with a note recording that the original commit shipped with the revision 6 path unproven and that this follow-up is what closed it. Not done here: the decrypt fuzz target could not be re-run. The nightly toolchain now available (2026-07-27) fails to build the cc crate that libfuzzer depends on, with errors inside cc itself rather than in this code. The target still compiles under stable and the earlier run of 1,953,940 executions stands; this is recorded rather than quietly skipped. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (387 tests) Both rustfmt and clippy -D warnings clean. |
|||
| 01e16b6383 |
feat(pdf): implement encryption (Phase 8)
Phase 8 feature 3 of 10, designed in REVIEWS/adr/0005-pdf-encryption.md.
An encrypted PDF did something worse than fail: it succeeded. Probing a
structurally valid RC4-encrypted file through the parser gave
parsed OK: pages=1
page 0 content bytes=44
content parsed into 0 ops
No error and no warning. The document reported a page, the page reported
content, and the content interpreted to nothing because it was ciphertext.
The user saw a blank page and was told the file was fine. That is the defect
class Phase 0 existed to remove, and it was the worst one left in the PDF
stack because it was silent.
New pdf-cos/src/encrypt.rs implements the standard security handler for
reading:
- V1/R2 RC4 40-bit, V2/R3 RC4 40 to 128-bit, V4/R4 crypt filters selecting
RC4 or AES-128, and V5/R6 AES-256 with the SHA-256 based revision 6 hash.
- The empty user password, which is the common case for a document
encrypted only to set permissions, and explicit user or owner passwords.
The owner path recovers the user password from /O and re-derives.
- Per-object keys, as the spec requires. Reusing one keystream across
objects would be a real cryptographic break, so the object and generation
numbers are mixed in by construction and a test asserts the keys differ.
Every primitive comes from audited RustCrypto crates: aes, cbc, rc4, md-5
and sha2, all MIT OR Apache-2.0, which deny.toml already permits. Phase 0
deleted a hand-rolled MD5/SHA/AES/RC4 implementation from this codebase and
called it a CVE factory; ADR 0005 keeps that rule.
Refusals rather than half-open documents: a public-key or otherwise
unsupported handler is refused and named, an unsupported V/R combination is
refused, and a wrong password returns a distinct error so a caller can
prompt again rather than reporting a damaged file.
Permissions are parsed and exposed but deliberately not enforced, and the
code says why: once content is decrypted a caller can read it regardless, so
enforcing here would imply a guarantee that does not exist.
Saving an encrypted document stays refused, as ADR 0003 established.
Decrypting and then writing plaintext would silently strip the protection
the author applied, which is not a decision a library should make.
Fixtures: tests/corpus/encrypted/ gains RC4 40-bit, RC4 128-bit, AES-128 and
an unsupported-handler document. The generator implements the handler's
algorithms independently from the specification, so a fixture that decrypts
shows the reader agrees with the spec rather than merely with itself. Each
plaintext contains a marker the tests assert on, and one test additionally
asserts the decrypted content interprets to real render commands, because
asserting Ok from the parser is exactly what the old broken behaviour did.
Fuzzing: adds a decrypt target covering key derivation, which consumes
attacker-controlled /O, /U, /P, /Length, filter names and file id. Run for
real rather than compile-checked: 1,953,940 executions, no crashes.
Validation:
TEST_TARGET=pdf ./tools/test-rust-clean.sh (383 tests)
TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (427 tests, 6 ignored)
Both rustfmt and clippy -D warnings clean.
|
|||
| f04f8ba34e |
feat(pdf): implement annotation editing (Phase 8)
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. |
|||
| dc2bf234c6 |
test(pdf): harden the xref revision chain
Incremental save made /Prev chain walking load-bearing for every document, not only saved ones: XRefTable::parse now follows offsets taken straight from the file on every parse. Phase 6 established that code consuming untrusted input needs corpus and fuzz coverage. That path had neither, so this adds it and fixes what it found. Corpus (7 new fixtures, generated by the checked-in script as usual): - revisions/two.pdf, three.pdf: chained revisions that override a form value. These are direct regression tests for the two bugs the previous commit fixed. Before it, two.pdf read back as "first" rather than "second", because find_xref_start never matched its own keyword and fell through to the oldest section in the file. - revisions/added_page.pdf: a revision that rewrites /Pages, so the newer definition must win for structure as well as for values. - malformed/prev_loop.pdf, prev_out_of_range.pdf, prev_negative.pdf and prev_chain_bomb.pdf: the hostile shapes. Two robustness defects found by those fixtures: - A broken /Prev orphaned every object the unreachable sections defined, even though the bytes were still in the file, so a document with one bad offset failed to open at all. The chain now sets a recovered flag and sweeps the file for object headers, filling only genuine gaps: entries a parsed section supplied always win, because those reflect the document's own view of which revision is current, and scanning cannot tell newer from older. - A negative /Prev was filtered to None, which silently ended the chain as though the file had no history. It is now treated as a broken link and triggers the same recovery. Also caps the chain at 64 revisions. A legitimate document has a handful; a file with thousands is an attack, not a history. prev_chain_bomb.pdf asserts the cap holds and that parsing stays fast. The recovered flag is public so a caller can distinguish a cleanly parsed document from a salvaged one rather than being handed a guess silently. A test asserts it stays false for healthy files, or it would mean nothing. Fuzzing: adds parse_revision_chain, which splices fuzzer input onto a valid base document so the fuzzer spends its time on chain shapes rather than on rediscovering PDF syntax. Run for real, not merely compile-checked: parse_revision_chain 1,926,164 runs parse_xref 1,387,713 runs parse_document 1,279,328 runs No crashes. The two re-run targets cover the file this commit changes. One fixture-generator bug fixed on the way: the helper that reads a file's startxref took the first token after rfind without skipping the keyword, producing a startxref that pointed at its own text. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (318 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (362 tests, 6 ignored) Both rustfmt and clippy -D warnings clean. |
|||
| d59bed5868 |
feat(pdf): implement incremental save (Phase 8)
Phase 8 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md, designed in REVIEWS/adr/0003-pdf-incremental-save.md. The review treats Phase 8 as ten independent projects, each needing its own design doc and merge criteria. Incremental save is taken first because it is the only one whose prerequisites are already met, it is listed as a prerequisite by two others (annotation editing and full AcroForm support), and it closes a real credibility gap: DocumentFormEditor has been able to edit form fields since Phase 3, and there was no way to save the result. A grep for a public save API across all four crates returned nothing. Design decision: append a revision, never rewrite. The original bytes are copied verbatim and changed objects are appended with a new xref chained through /Prev. A full rewrite would be easier and wrong: it would silently discard everything this parser does not yet model (structure trees, optional content, embedded files), and it would invalidate any signature, foreclosing a feature listed later in the same phase. ADR 0003 records this in full. Two latent bugs surfaced while building it, both pre-existing: - find_xref_start searched with windows(10) for the 9-byte keyword "startxref", so it never matched. Every parse silently fell through to a forward scan for the first "xref" in the file. On a single-revision document that happens to be correct; on an incrementally saved one it is the *oldest* revision, so a saved edit read back as its pre-edit value. This had no visible effect before because nothing produced multi-revision files. - XRefTable::parse read one section and ignored /Prev entirely, so a multi-revision document lost every object the earlier revisions defined. It now walks the chain newest-first, keeping the first definition of each object, with a visited set against /Prev loops and bounds checks on the offsets, which come from the file and cannot be trusted. A bad link ends the chain instead of indexing out of bounds. The xref unit fixture claimed startxref 408 in a 191-byte file and only ever passed because of the windows(10) defect; it is corrected rather than adjusted to keep passing. Implementation: - pdf-cos/src/incremental.rs: IncrementalUpdate builds one revision. Recomputes stream /Length so a caller cannot write an inconsistent one, emits xref subsections for contiguous runs, sizes /Size over the whole chain, and is byte-reproducible for a given set of edits. - pdf-document/src/save.rs: turns dirty AcroForm fields into a revision, writing the new /V and a regenerated appearance stream referenced from /AP, keyed by state name for checkboxes and radios. Refusals rather than partial saves: an encrypted document returns SaveError::Encrypted, because writing plaintext objects into it would corrupt the file; a source with no startxref or no /Root is refused; and a save with no pending edits returns the input unchanged rather than growing the file and churning its timestamp. Tests: 10 acceptance tests in tests/save_roundtrip.rs covering the ADR merge criteria. The central one reparses from the written bytes rather than reusing in-memory state, so it tests the file rather than the writer against itself. Also asserts three chained revisions still reparse with the newest value winning, that pages and annotations survive a save, and that saving never panics on the malformed corpus. Known limitations, recorded in the ADR rather than glossed: cross-reference streams and object streams are not written, and superseded objects are not compacted. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (307 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (351 tests, 6 ignored) Both rustfmt and clippy -D warnings clean. |
|||
| 1c7dc9e9c0 |
feat(pdf): complete Phase 7 performance work
Phase 7 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md, taken only now because the review is explicit that it comes after correctness is proven. The caching and threading live in pdf-graphics rather than the Makepad crate because none of it needs a GPU. That is what lets the staleness, eviction and cancellation rules be tested without a window; the widget keeps only the parts that genuinely need a Cx. Step 7.1, off-thread parsing (new pdf-graphics/src/worker.rs): - RenderWorker interprets content streams on a background thread and returns results tagged with the Generation they were requested for. - PendingPages tracks in-flight pages so the widget can draw a placeholder and never queues the same page twice. - Drop joins the thread rather than detaching it: a detached thread writing into a dropped channel is the kind of shutdown race that surfaces as a flaky test months later. - A content stream that fails to parse yields an empty page, so one broken page cannot take down the document. Step 7.2, texture and memory management (new pdf-graphics/src/cache.rs): - PageCache is an LRU keyed by page index with a byte budget, not an entry count: one image-heavy page can outweigh fifty text pages, so counting entries would evict the wrong things. - retain_around() releases pages that scrolled out of view, keeping a margin so a small scroll does not immediately re-render. - Decoding produces DecodedImage bytes off-thread; GPU upload stays on the UI thread. - A page larger than the whole budget is still stored, since refusing it would mean re-rendering it every frame. Step 7.3, render command cache: - CachedPage holds the interpreted Vec<RenderCommand>, so replaying a page skips re-parsing its content stream. - invalidate_appearance() marks a form edit dirty without discarding the commands, because a form edit changes what is drawn over the page, not the page content stream. Widget wiring: PdfPageWidget carries a generation, refuses PageContent from a superseded document, and draws a placeholder while a page is still rendering. Exit criterion (new pdf-document/tests/phase7_exit_criterion.rs, 9 tests) against a new 60-page corpus fixture. Measured here: first page 3ms against the 200ms budget, and a warm cache read 389x faster than re-parsing (1us vs 389us). The budget is deliberately loose because a shared CI runner is unpredictable and a flaky performance test gets muted, and a muted test is worse than none; it still catches the order-of-magnitude regression the review is guarding against. Memory is asserted bounded across 30 document switches, and every page of an abandoned document is refused. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (270 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (307 tests) Both rustfmt and clippy -D warnings clean. |
|||
| b23df6a5cb |
feat(pdf): complete Phase 6 testing infrastructure
Phase 6 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md: "Real PDFs. Real regressions. No test theater." Step 6.1, corpus. 27 fixtures across basic, fonts, forms, annotations, images, edge and malformed, in the layout the review specifies. They are produced by tests/corpus/generate.py rather than committed as opaque blobs, because a corpus you cannot read is a corpus you cannot trust; CI regenerates them and fails if they differ. Hand-rolled rather than library-produced, since fixtures for a parser must contain constructs a library refuses to emit. Step 6.2, corpus tests (pdf-document/tests/corpus.rs, 27 tests). Text, vectors, Flate, multipage, CID fonts, every form field type, inherited field keys, link actions, hidden annotations, XObjects, inline images with embedded EI bytes, rotation, crop boxes, nested CTMs and content arrays. Step 6.3, robustness (pdf-document/tests/robustness.rs, 3 tests) plus five cargo-fuzz targets. cargo-fuzz needs nightly and libFuzzer so it cannot gate a stable CI run; the harness covers the same ground deterministically by mutating the real corpus with a fixed-seed PRNG, so a failure is reproducible from the seed rather than only from a saved artefact. The fuzz targets remain the deeper coverage-guided search and run on a schedule. Three crashes on untrusted input, all found by this work and all previously reachable from a malformed file: - collect_pages_ref recursed forever on a /Kids cycle. Stack overflow aborts the process; it cannot be caught. Now tracks visited nodes and bounds depth. - PdfDocument::resolve and the COS lexer recursed once per nesting level, so a file of 5000 open brackets overflowed the stack. Both are now bounded. - decode_85_group multiplied an accumulator that a malformed group can overflow, and subtracted below zero on a digit outside the valid range. Both panic in a debug build. Now saturating. Step 6.4, CI (.forgejo/workflows/pdf.yml). An engine job that runs the corpus and robustness suites under rustfmt and clippy -D warnings; a separate makepad-integration job so a missing system library is not reported as a PDF regression; and a scheduled fuzz job. The engine job also enforces the two architectural rules mechanically rather than in prose: no Makepad dependency or import in the engine crates, and no process or URL launching anywhere in them. Also fixes .gitignore: the blanket *.pdf rule silently excluded all 27 fixtures, which would have left CI unable to run them on a fresh clone. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (233 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (268 tests) Both rustfmt and clippy -D warnings clean; all five fuzz targets compile. |
|||
| b7d23f26bc |
feat(pdf): complete Phase 2 render pipeline with golden tests
Phase 2 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md. The exit criterion is that a real content stream renders through the device path and is verifiable; previously nothing could fail, so nothing was proven. Step 2.3, the operations the review calls "the lies", all reached a dead end in the interpreter. Each now reaches the device: - Inline images: BI/ID/EI were emitted as two empty marker ops and the payload was discarded, so an inline image could never be drawn. They are now parsed into a single InlineImage op carrying the dictionary and bytes. Abbreviated keys (/W /H /BPC /CS /F) and colour-space and filter abbreviations are expanded. The EI scan requires delimiters on both sides so binary data containing the bytes "EI" does not truncate the image, and an unterminated image yields no image rather than invented pixels. - Do (XObject) was an empty match arm. The interpreter cannot resolve a resource name, so it now reports it through PdfDevice::paint_x_object and the device performs the lookup. - set_dash and set_miter_limit only mutated interpreter-local state and emitted no command, so dashes never reached any renderer. Three further defects surfaced while reviewing the generated goldens, all of which silently corrupted output rather than failing: - `cm` was never parsed at all. The single-character `m` arm matched first and consumed it as a moveto, so every CTM change in every document was lost and content drew at the wrong position. Two-character operators are now tested before their one-character prefixes. - `rg` and `RG` were shadowed by the `r` and `R` arms, so an RGB fill was read as a single-component grey: `0.1 0.2 0.3 rg` produced 0.1 0.1 0.1. - ImageInfo defaulted a missing /Filter to FlateDecode. An absent /Filter means the data is stored raw, so every uncompressed image was undecodable. Testing: adds format_commands(), a deterministic one-line-per-command text form of a RenderCommand list, and eight golden files covering vector paths, text, kerning and spacing, dash and stroke parameters, fill rules, inline images, XObjects and colour operators. Floats are fixed-precision and negative zero is normalised so no spurious diffs appear. UPDATE_GOLDEN=1 regenerates them for review. Also fixes .gitignore: blanket *.txt and *.pdf rules were silently excluding the golden expectations and the AcroForm fixture added in the previous commit, which would have left both test suites unable to run on a fresh clone. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh 132 tests pass; rustfmt and clippy -D warnings clean. |
|||
| 3f107bf2a5 |
feat(pdf): complete Phase 3 document model with real form state
Phase 3 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md. The previous form.rs was keyed by name strings, had no inheritance and no way to edit anything, so the review item "delete PdfFormFilling and replace it with a DocumentFormEditor that returns real errors" had no implementation. Step 3.1 object identity: - Fields are keyed by ObjRef, not by name. Widget-to-field identity is preserved across parse, and a field defined as a direct object is skipped rather than given an invented identity. - PdfDocument::page_index_of() maps a page reference back to its index and returns None for a stranger. Step 3.2 annotations: - PdfDocument::page_annotations() reads /Annots and records the real page on every annotation; the hardcoded page_index: None is gone. - Annotations expose a typed action() (OpenUri / GoToNamed / GoToPage). The document reports intent only and never opens anything itself (rule 5). - contains_point() normalises the rectangle: a PDF /Rect is any two opposite corners, so an inverted one previously never hit-tested. Step 3.3 form model: - /Parent chain inheritance for FT, Ff, V, DV, DA, MaxLen and Opt, with the child key overriding the ancestor. - /Ff resolved into concrete types: checkbox vs radio vs pushbutton, combo vs list box. - DocumentFormEditor takes typed edits and returns FormError. Read-only, type-mismatched, over-MaxLen, non-option and unknown-state edits are all refused, and a refused edit leaves the field untouched and not dirty. - MaxLen counts characters, not bytes. - Checking a box uses the on state the widget declares, not an assumed /Yes. - The field tree walk is depth-bounded so a cyclic /Kids cannot recurse until the stack dies. Step 3.4 appearance generation (new appearance.rs): - Document-level, not widget code. Generates text, multiline, choice and checkbox appearances as Form XObjects with correct /BBox and /Length. - /DA parsing resolves font, size and colour, converting gray and CMYK. - Auto-size (0 Tf) resolves to a size that fits the widget. - Values are escaped, so a parenthesis in a value cannot terminate the string and corrupt the stream; a single-line value cannot break out via newlines; content is clipped to the widget box. - Unsupported kinds (pushbutton, signature) and degenerate rectangles return AppearanceError instead of a blank stream that would erase the field. Wiring: PdfDocument::acroform() parses the form against the real page tree, which is what makes widget page resolution meaningful. Tests: adds tests/acroform.pdf, a hand-written two-page AcroForm fixture with an inherited field, a checkbox with /On and /Off states and a URI link, all on page index 1 so any code defaulting to page 0 fails. 12 integration tests assert the Phase 3 exit criterion against that real parsed file. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh 118 tests pass; rustfmt and clippy -D warnings clean. |
|||
| af4ef6f170 |
fix(pdf): restore a compiling, warning-free PDF baseline
Some checks failed
Payment domain and storage / isolated-payment-tests (push) Has been cancelled
Phase 0 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md requires a clean baseline before any feature work. The four pdf crates did not compile at all, so every test claim about them was unverified. Compile fixes: - decode_lzw/decode_run_length returned Vec<u8> where callers expected PdfResult, so decode_stream_with_params did not type-check. - interpret_ops called a current_state_mut() method that PdfDevice does not have; colour-space tracking now goes through explicit device hooks. - image.rs used miniz_oxide without depending on it; PNG inflate now reuses the COS crate through a new pdf_cos::filter::inflate_zlib. Correctness fixes found while making the code build: - LZW and RunLength silently truncated malformed input and indexed unchecked; both now return typed errors (rule 6: no silent degradation). - Tw/Tc/Tz were parsed and thrown away, and the " operator dropped its word and character spacing, so every advance after them drifted. - TJ attached each kern to the preceding string instead of the following one and discarded a trailing kern entirely. - GlyphWidths::width() fell back to default_width for out-of-range codes; PDF 32000-1 9.6.2.1 requires /MissingWidth. - Text advances silently substituted a guessed font_size * 0.6 when no width table was present; ShowTextWithMetrics now carries advance_is_measured so callers can distinguish a measurement from an unknown. Tests: two tests had never compiled and were wrong once they ran (WinAnsi 0x99 is U+2122 not U+2019; q/cm/l/Q records four commands not three). The document test asserted only that the writer emits a %PDF header; replaced with real page-tree, out-of-range and malformed-input coverage. Adds a pdf target to tools/test-rust-clean.sh that tests the three UI-independent crates bottom-up under rustfmt and clippy -D warnings. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh 72 tests pass; rustfmt and clippy -D warnings clean. |
|||
| a2ea0ffc7c | updated map | |||
| cc05abdc71 | Initial commit |
