27 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| a82c8f7ff7 |
feat(pdf): Unicode-aware search and layout-aware reading order
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
nigig-build (CAD) / cad-engine-coverage (push) Has been cancelled
nigig-build (CAD) / doc-workspace-coverage (push) Has been cancelled
nigig-build (CAD) / cad-widget-coverage (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
spreadsheet / engine-coverage (push) Has been cancelled
spreadsheet / ui-controller-coverage (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / coverage (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
email / gates (push) Has been cancelled
email / email-domain (push) Has been cancelled
email / nigig-email (push) Has been cancelled
email / supply-chain (push) Has been cancelled
nigig-map / test (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
sms / gates (push) Has been cancelled
Phase 8 bullets one and two. Probing the existing code first, as the
workflow requires, found five defects rather than the one the plan names:
SPLIT MATCH 'Hello': 0 hits
plain_text: "Hello"
PRECOMPOSED 'café': 0 hits
COLUMNS plain_text: "LeftTopRightTop\nLeftBotRightBot"
OUT OF ORDER plain_text: "second\nfirst"
`PageText::find` searched one run at a time and documented that as a known
limitation. It is a limitation from inside the code and a broken feature
from outside it: a writer starts a new run wherever it adjusts kerning, so
an ordinary word arrives as two runs, and the find bar says a word plainly
visible on the page is not there.
`search.rs` indexes the page as one flattened string with a map back to
(run, character), so a cross-run match is found and highlighted with one
rectangle per run — never a merged box, which across a line break covers
half the paragraph.
The separator between two runs is a geometric question with three answers:
abutting runs join with nothing (one word, split by kerning), separated
runs with a space, and a different line or column with a newline. The
newline matters as much as the empty join: joining lines with a space lets
"one Right" match across a column gutter, text that appears nowhere.
Whether two runs share a column is *asked* of the layout analysis rather
than re-derived, or the extracted text and the searched text disagree about
where a column ends — the original defect wearing a different hat.
NFD, never NFC: composition needs the next character, so an NFC fold
applied per character composes nothing and the two spellings of an accent
stay different. That was a real bug in the first draft. And case *folding*,
not lowercasing — Rust lowercases ß to ß, so "Strasse" never found
"Straße".
Columns are detected before lines, because two columns share their
baselines; that is what makes them columns. Bands are separated by a gutter
rather than by bare non-overlap, since two abutting runs on a line do not
overlap either.
Also fixed, found by running the gates rather than by looking: a stream
reader trimmed a trailing CR before `endstream` as if it were the writer's
separator. Binary data ends in CR about one time in 256, and when it did
the reader returned a stream one byte short — no longer AES-block-aligned,
so decryption produced garbage and Flate failed. Roughly one encrypted
document in 250 was silently corrupt on read. The test failed once under
coverage, passed five times in isolation, and failed 2 in 40 when actually
counted. A /Length consistent with the file is now the authority; both
stream readers are fixed and a test reads one file through each.
1477 tests pass (was 1426), coverage 88.37%, all floors met, external
readers pass. 10 mutations across the two modules, all killed.
ADR 0034.
|
|||
| d4e3e9a443 |
feat(pdf): encryption on save — AES-128 and AES-256 (Phase 6, part one)
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
nigig-build (CAD) / cad-engine-coverage (push) Has been cancelled
nigig-build (CAD) / doc-workspace-coverage (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
doc-engine / engine (push) Successful in 21s
doc-engine / coverage (push) Successful in 31s
doc-engine / consumer (push) Failing after 16m57s
email / gates (push) Has been cancelled
email / email-domain (push) Has been cancelled
email / nigig-email (push) Has been cancelled
email / supply-chain (push) Has been cancelled
nigig-map / test (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
ADR 0024. This reverses ADR 0005's "never write encryption", and the reason it is safe to reverse is that the facts changed underneath it. A crate that only reads cannot produce weak ciphertext, so refusing to write any was free. Now that Phase 4 creates documents and Phase 5 edits them, the refusal does something worse than protect nobody: open a password-protected file, change one annotation, save, and the output is plaintext. No error, no warning — the protection is silently dropped. That is this project's recurring failure mode in the one place where the consequence is a breach. The principle survives in a narrower form: no hand-rolled crypto, and no weak cipher offered as an option. RC4 stays readable because files use it and is not writable — EncryptionAlgorithm has no RC4 variant, so the refusal is a type, not a runtime check someone can route around. The encryptor is the literal inverse of the decryptor and imports its primitives rather than restating them; two implementations of one algorithm drift, and here they drift towards "decrypts to garbage". Every unit test round-trips through the existing Decryptor. Encryption sits at one choke point: PdfWriter holds the Encryptor and write_object_at encrypts everything passing through. Not per call site — there are twenty-two of those in PdfDocBuilder, and one stream written in the clear inside an encrypted document is not a partial failure, it is a leak that no reader will report because the file is otherwise valid. The /Encrypt dictionary is the single deliberate exemption: it holds the salts a reader needs before it has a key, so encrypting it bricks the file. Verified against implementations we share no code with, now gated in CI: ok qpdf opens it with the password ok it really is AES-256 ok the wrong password is refused ok poppler decrypts the content ok no plaintext in the encrypted file Four mutations, all killed — two only after the tests were strengthened, and both misses are the interesting part: A fixed IV survived two_saves_of_one_document_are_not_byte_identical, because the AES-256 file key is fresh per save and that alone makes the output differ. The property actually needed is narrower: one encryptor, identical plaintext, different bytes. In CBC a repeated IV under one key leaks that two plaintexts are equal. A wrong /Length survived because our own reader recovers by scanning for endstream — a robustness fix from ADR 0023. An independent reader that trusts /Length reads a truncated stream and decrypts garbage. A lenient reader hides a broken writer, which is why the external gate exists. The /Length test itself had a bug first: it searched a from_utf8_lossy view and reported a stream declaring 80 bytes holding 156. Ciphertext is not UTF-8; the replacement characters shifted every offset. Unencrypted output stays byte-reproducible; encrypted output cannot be, and a test asserts that loss rather than leaving it implicit. pdf: 1220 passed (was 1187). pdf-ui: green. Coverage 88.21%, encrypt_write.rs at 96.5%. Signing is NOT started. It needs the trust-anchor decision ADR 0010 deferred: VerificationStatus::Valid is unreachable by construction, and making sign -> verify pass is a policy change, not an implementation detail. The plan's Phase 6 status now says so. |
|||
| 2ba1837055 |
fix(pdf): main was red — three clippy errors broke the engine build
The PDF engine did not compile under `-D warnings`, which is what CI's
engine job runs, so that job could not have passed on any of the last
eleven PDF commits. The tests themselves were fine (1187 passing); the
build was not.
Two lints in the new jpx.rs, one in tests/filters.rs. One root cause each:
jpx.rs:1279,1290 needless_range_loop on the inverse component
transform. The lint's suggestion does not work here:
each pass reads and writes three component planes at
the same index, and `components.iter_mut()` cannot
express three simultaneous mutable borrows of one Vec.
Allowed locally with the reason written down, rather
than restructuring correct code to satisfy a lint that
has misread it.
filters.rs:383 vec_init_then_push, where the lint is simply right.
No behaviour change. Verified after the fix, on a clean checkout of
|
|||
| 41c43df0e5 |
feat(pdf): redaction and object compaction — and three reader defects
Some checks failed
email.yml / feat(pdf): redaction and object compaction — and three reader defects (push) Failing after 0s
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
Phase 5's last two functional items. Together they are what makes a redaction real, which is why they are one commit: redaction removes the content, compaction removes the revision that still holds it. **Redaction removes operators; it does not draw rectangles.** The famous failure is painting black over text and shipping it — the text is still there, and `pdftotext` prints it. This module draws nothing. It removes the text-showing operators whose position falls inside a rectangle, and the test that matters is that the text can no longer be extracted. Positioning needs the text matrix, so the module tracks `Tm`/`Td`/`TD`/`T*` and the CTM through `q`/`Q`/`cm`. It cannot reach the graphics layer — the crate boundary again — so it treats a showing operator's origin as its position and removes the whole run. That is coarse in the *safe* direction: removing more than asked loses content the user can see is missing; removing less leaves the secret in the file. What it refuses to claim is as important. Images are removed entirely rather than cropped. Metadata and attachments are untouched. And an incremental redaction leaves the original text in the earlier revision — the report says so via `earlier_revisions_retain_content` rather than implying the job is done. **Compaction finishes it.** The output is built from the object graph reachable from `/Root`, so dead objects, superseded revisions and the bytes behind a redaction are not copied — they are simply never written. A signed document is refused unless `allow_signed` is set, because compaction destroys the revision a signature covers and would leave every signature unverifiable with no warning. The end-to-end test is the point: redact, compact, then search the output bytes for the secret. It is gone. **Three reader defects, all found by writing the tests.** - **`PdfWriter` wrote dictionary keys unordered.** `PdfDict` is a HashMap and Rust seeds its hasher per process, so *every generated PDF differed run to run*. Found by compaction's idempotence test — compacting an already-compact file produced the same objects at the same offsets with their keys shuffled. Verified fixed by running four separate processes and getting a byte-identical file. Same defect as the one fixed in `content_edit::write_dict`; this one affected every file this codebase has ever written. - **A short `/Length` silently truncated a stream.** The reader guarded against a `/Length` running past the buffer but trusted one that was too small, cutting the stream at the wrong place and losing the rest with no error. Short lengths are common in hand-edited files. `endstream` is now the authority when the two disagree — but only when it is *further* on, so binary data containing the word `endstream` is still bounded by its declared length. - **Two stream readers disagreed by one byte.** `read_object_at` did not trim the EOL before `endstream` while `find_endstream` did, so a write-read-write cycle grew every stream by a newline. A test fixture had encoded the bug: it declared `/Length 9` for eight bytes of content and asserted the newline came back as data. Both corrected — the newline is syntax (§7.3.8.1), not content. Verified by mutation. Ten defects across the two modules, all caught: redaction covers instead of removes 16 fail CTM ignored 1 fail Q does not restore the CTM 1 fail operands kept when operator removed 12 fail revision warning always false 1 fail signature guard removed 1 fail reachability keeps everything 2 fail dropped reference left dangling 1 fail unresolvable object kept as reachable 1 fail writer dictionary order unsorted 1 fail short-/Length fix reverted 1 fail /Length not rewritten on compaction 1 fail One mutation survived and deleted code rather than adding a test: a `continue` skipping `/Length` in the compaction loop was dead, because the `set` after the loop overwrites it either way. Removed rather than left as untested defence with a reassuring comment — the same call ADR 0017 made about the visited-set guard. A second mutation moved a test rather than a fixture: a stale `/Length` can no longer reach `renumber` through a file, because the reader now repairs it first, so that branch is tested directly instead. Engine suite 1108 -> 1158. External readers still pass. Phase 5 remaining: outline, page label and struct-tree editing. |
|||
| 8701f5df51 |
feat(pdf): image embedding and header/footer stamping — Phase 4 complete
Some checks failed
email.yml / feat(pdf): image embedding and header/footer stamping — Phase 4 complete (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
The last gap in Phase 4: dart-pdf's header_footer_test, image_stamp_test and image_pdf_test had no counterpart here. What was missing is worth stating precisely, because it is the shape of bug ADR 0017 exists to catch. ContentWriter::draw_image has emitted `q w 0 0 h x y cm /Name Do Q` since Phase 2, and was tested. But nothing in the stack could *create* the image XObject that /Name resolves to. So every Do operator ever written named a resource that did not exist, no document could contain a raster image, and nothing anywhere returned an error. The writing half was present, the reading half faithfully reported the content stream, and the image was simply never there. stamp.rs adds: image XObject embedding, header/footer banners with left/centre/right alignment, image stamp content, and stream composition. A JPEG is embedded as-is with /DCTDecode — PDF's image model is the same DCT data the file already holds, so re-encoding would lose quality for nothing — and its geometry is read from its own SOF marker rather than trusted from the caller, because a /Width that disagrees with the codestream renders as diagonal garbage in every viewer. Raw samples embed as Flate. Embedding an image then adding the page that draws it exposed a live defect in PdfDocBuilder. add_object derived its number from `3 + 2 * pages.len()`, so every add_page after an add_object silently shifted a number already handed out. Embedding an image and then adding its page — the natural order, since the page's content stream has to name the image — produced a page whose /XObject entry pointed at the page object itself: 3 0 obj <</Type /Page ... /XObject <</Im0 3 0 R>>>> The file parsed. The reference resolved. The resource was the page. This is the same positional-numbering defect already fixed once for fonts, one layer out — the comment above first_extra_object_number describes the font version, where /ToUnicode pointed at the descriptor and /FontFile2 at the Type0 wrapper. Both come from deriving object numbers from collections that are still growing. Fixed at the root: the page count is frozen when the first extra number is issued, and pages added afterwards are allocated past the fixed block instead of colliding with it. Non-contiguous page numbers are legal — /Kids is an explicit array — and 952 tests confirm nothing depended on the order. The integration tests parse the generated file back with PdfDocument and assert the image appears in `page.xobjects` with subtype Image, that its /Width and /Height match the SOF marker, and that the header and footer baselines are at opposite ends of the page. Reading the resource back is the assertion that matters: a substring check for "/Im0 Do" passed throughout the entire period when no image could be embedded at all. Verified by mutation, five injected defects, each confirmed red: numbering fix reverted 4 fail JPEG width/height transposed 5 fail header positioned from bottom 3 fail sample-count check removed 1 fail attach_image_to_page a no-op 5 fail One test needed correcting rather than the code: three assertions grepped the output for operators, which are Flate-compressed by default, so they were asserting against compressed bytes. They now disable compression explicitly — the structure is identical either way, and the alternative was a test of miniz_oxide. Engine suite 920 -> 952. Coverage 86.16% -> 86.40%; stamp.rs at 94.64% with a floor at 90. Phase 4 is complete and the plan records it, including the numbering defect, since a status table that lists only features would not have told the next reader why the object numbers look the way they do. |
|||
| 674b2be66d |
feat(pdf): JPEG 2000 decoding — Phase 3 complete, all three codecs
Some checks failed
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
email.yml / feat(pdf): JPEG 2000 decoding — Phase 3 complete, all three codecs (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
The last codec ADR 0015 deferred. The plan recorded the blocker as a dependency decision, not an algorithm: openjpeg would add a C dependency that breaks the Android cross-compile. This is pure Rust and adds no dependency at all. It shares the MQ arithmetic decoder with JBIG2 — T.800 and T.88 specify the same coder — so the previous tranche paid for most of this one. Context::with_state moved onto the shared type because JPEG 2000 starts three of its nineteen contexts away from state 0 and JBIG2 starts all of them at 0. Implemented: codestream and JP2 container parsing, packet headers with tag trees and the bit-stuffing rule, EBCOT tier-1 (all three passes, four zero-coding context tables, run-length mode), both 5/3 reversible and 9/7 irreversible wavelets, RCT and ICT, arbitrary decomposition levels, and multiple components. Refused by name: multiple tiles, custom precinct partitions, code-block style options, COC/QCC/RGN/POC overrides, subsampled components. Each error says which feature the file needs. This matters more here than anywhere else in the stack, because a JPEG 2000 decoder that quietly skips something does not fail — it returns a slightly soft or banded image that looks entirely fine. That property also dictates how this is tested. Fixtures are produced by OpenJPEG via Pillow and compared **exactly**, sample for sample: the fixtures are lossless 5/3 so no tolerance is needed, and a tolerance is where a subtly wrong decoder hides. Four images — grayscale raw codestream, the same in a JP2 container, a larger one whose tag trees actually branch, and RGB. A generator script is checked in beside them so CI can prove the fixtures still match what produced them. Verified by mutation. The first round was misleading and is worth recording, because it is the same lesson as ADR 0017: DC level shift dropped 3 fail 5/3 lifting rounding changed 2 fail RCT sign flipped PASSED <- survived RCT components swapped PASSED <- survived cleanup run-length disabled PASSED <- survived sign-context XOR dropped PASSED <- survived Four mutations survived because Pillow writes MCT=0 by default, so the RGB fixture coded its three components independently and never reached the colour transform at all. The RCT branch was completely untested while appearing covered — an untested branch that looks tested is worse than one that looks missing. Added rgb8_mct.j2k with mct=1; all four now fail. The header bit-stuffing mutation is caught by the unit test rather than the round-trip. Two real defects found while writing the tests: - A corrupt marker length in a tile-part header walked the read cursor past the codestream and panicked on a slice. Found by the corruption sweep, not by review. The sweep now truncates at every length and flips every byte of a real file, and asserts only that nothing panics. - The 9/7 flat-signal test initially asserted an amplitude I had derived from my own arithmetic. That is a test agreeing with the code by construction. It now asserts flatness — a ripple means the lifting or the edge extension is wrong — and the amplitude is pinned by the OpenJPEG round-trips instead, which use pixels this code did not produce. Also removed two dead fields and an unused parameter that clippy found: Subband::x0/y0 are always zero in the single-tile case this supports, and dead state implying multi-tile support exists is worse than no state. JPX decodes on the image path, like JBIG2, because the codestream carries its own geometry; it stays in REFUSED_CODECS with a reason string saying where it is decoded rather than that it is missing. Engine suite 866 -> 920. Coverage 85.66% -> 86.16%; jpx.rs at 93.72% with a floor at 88. Phase 3 is complete: CCITT, JBIG2 and JPX all land, and the plan is updated to say so and to record how the two gating questions — JBIG2's CVE record and JPX's C dependency — were actually answered. |
|||
| 81e846ae35 |
feat(pdf): JBIG2 generic-region decoding, and the bitonal image path
Some checks failed
email.yml / feat(pdf): JBIG2 generic-region decoding, and the bitonal image path (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
The second codec Phase 3 deferred. ADR 0015 made a threat review the precondition for implementing JBIG2 rather than an effort estimate, so the review's conclusion is encoded in what this does and does not do. What is implemented: the MQ arithmetic decoder (T.88 Annex E), generic region decoding with templates 0-3 and AT pixels, TPGDON typical prediction, and MMR-coded regions. Segment header and region info parsing, and page composition. What is refused, by name: symbol dictionary, text region, halftone region, refinement region, and a non-empty /JBIG2Globals. Those are the segment types that carry the composition machinery, and shipping them means shipping an interpreter over untrusted input — it is what FORCEDENTRY built its computer out of. A file needing them gets a typed error naming the segment type, exactly as the whole codec used to. The MQ coder itself is pure arithmetic with no file-controlled addressing, which is why it is safe to run and the composition parts are not. Every bound is checked against the declared region size before a buffer is indexed: region dimensions against MAX_DIMENSION and a pixel budget before allocation, segment lengths against the remaining stream, and the region's declared position against the page before a single pixel is written. That last one is the format's actual exploit surface and it has its own test saying so. MMR regions delegate to ccitt.rs rather than carrying a second G4 decoder, so the two cannot drift apart. A test decodes the same coded bits through both paths and requires identical pixels — that is what catches an inverted convention, and JBIG2 is natively 1=black where PDF is 0=black, so the inversion is real and easy to get backwards. JBIG2 is decoded on the image path, not in the filter facade, because it needs /Width and /Height from the image dictionary. It therefore stays in REFUSED_CODECS with a reason string that says where it *is* decoded, so a host showing that string does not tell a user the codec is missing when it is not. CCITT moved the other way for the same reason inverted: it derives its dimensions from /DecodeParms, so it decodes in the facade. Wiring both into ImageInfo::decode_to_rgba surfaced a defect in the parallel-array rule that the CCITT tranche had not reached. For /Filter [/FlateDecode /CCITTFaxDecode] the /DecodeParms array has one entry per filter, and the obvious implementation takes arr[0] — handing the Flate parameters to the fax decoder. ccitt_parms_of finds CCITT's own index instead. This is the same bug ADR 0015 records for the old chain code, in a new place. A declared-but-unresolved /JBIG2Globals returns None rather than decoding without it. Decoding anyway yields a blank or partial image that every caller reads as a success — the declared-versus-delivered failure of ADR 0017. Verified by mutation, six injected defects, each confirmed red: compose bounds check removed 1 fails pack() stops inverting 4 fails (both suites) globals silently ignored 1 fails refused segments silently skipped 1 fails declared-globals check dropped 1 fails ccitt_parms_of always takes slot 0 1 fails 29 unit tests and 12 integration tests, asserting pictures rather than buffer lengths. ADR 0016's stub JPEG decoder returned a correctly sized black rectangle and passed everything that checked a length; these say which colour they expect. Engine suite 825 -> 866. Coverage 85.15% -> 85.66%; jbig2.rs at 94.84% with a floor at 90, and image.rs 31.76% -> 44.77% so its floor rises 28 -> 40. JPX remains refused and is the next tranche. |
|||
| b26e6a1f14 |
feat(pdf): CCITT G3/G4 decoding — the codec Phase 3 deferred
Some checks failed
email.yml / feat(pdf): CCITT G3/G4 decoding — the codec Phase 3 deferred (push) Failing after 0s
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
ADR 0015 refused CCITTFaxDecode by name and recorded it as the recommended next codec: well specified, no arithmetic coding, no C dependency. This implements it. T.4 and T.6, all three schemes selected by /K: G3 1D modified Huffman, G4 two-dimensional, and G3 mixed with a tag bit after each EOL. Both run-length code books, makeup and extended makeup codes, and the pass/horizontal/vertical mode codes. /Columns, /Rows, /BlackIs1 and /EncodedByteAlign are honoured; /Columns and /Rows are bounds-checked before anything is sized from them, because both are attacker-controlled in a hostile file. It decodes to real pixels, so unlike DCTDecode it belongs in the filter facade rather than the image path: the generic filter contract promises decoded bytes and this can honestly keep that promise. Removed from REFUSED_CODECS, added to SUPPORTED_FILTERS — the registry now describes what the crate actually does. Both existing data-driven registry tests pick this up without editing. Three defects were found by writing the tests rather than by reading the code: - A zero-length run recorded no transition. That is exactly how a row beginning with black is coded — a white run of zero, then the black run — so every such row came out with its colours shifted by one run: "####...." decoded as "....####". - Decoding stopped at bits_left() == 0, but encoders pad the final row to a byte boundary. The padding was fed to the decoder as though it were a code, failed to match, and lost the whole image. Now a trailing all-zero tail is recognised as padding, which is unambiguous because every code book needs a 1 bit. - A row of zero-length runs did not advance the pixel position and looped forever. Found by mutation, not by review. Bounded by the column count: a hang is a worse failure than an error. Verified by mutation, five injected defects, each confirmed to turn the suite red: a0 starts at 0 not -1 1 fails pack_row fills black 13 fails find_b1 parity dropped 1 fails short-/Rows check removed 1 fails read_run returns 0 2 fails Two of those did not fail on the first attempt and changed the tests: - a0 = 0 survived, because no fixture placed a colour change at column 0 — the one position where the off-by-one is visible. Added group4_codes_a_change_at_column_zero. - read_run returning 0 survived because the new run bound also errors, so an assertion of merely "some CCITT error" could not tell the two mechanisms apart. The assertions now name the specific failure. 30 unit tests in the codec, asserting decoded pictures rather than byte counts, plus 6 integration tests through the filter facade covering the chain case, truncation and the spec defaults. The facade test asserts output != input: ADR 0015 records DCTDecode "succeeding" by returning its own compressed input, and a test that only asserted Ok passed against that bug. Engine suite 796 -> 825. Coverage 84.82% -> 85.15%; ccitt.rs at 92.57% with a floor at 88. JBIG2 and JPX remain refused and are the next two tranches. |
|||
| 7d6fc4cbbe |
feat(pdf): document creation — outlines, forms, attachments, font subsetting
Phase 4 of NIGIG_PDF_FEATURE_PARITY_PLAN.md. ADR 0019. Almost none of it existed: Outlines, PageLabels, EmbeddedFiles and ViewerPreferences appeared nowhere in the workspace, in any crate. What did exist was a builder whose central method was pub fn add_page_with_content(&mut self, _width: f64, _height: f64, ...) which accepted a page size and discarded it. Asking for 200x400 and 300x500 gave two US Letter pages, because no /MediaBox was written at all. The test asserted the output contained the string "/Type /Page", which it did. Two more defects sat in the object writer, both producing files our own parser rejects: dictionary keys were written unescaped (a key with a space reparses as "expected number"), and f64::NAN was emitted as the literal token NaN, so one non-finite value anywhere made the document unreadable. Added: outline trees with the open/closed state in the sign of /Count, /PageLabels as a number tree with real roman and A..Z/AA..ZZ numbering, named destinations, attachments with file specs, /Info, XMP, viewer preferences, page mode and layout; AcroForm creation for text, checkbox, radio, choice and signature fields with generated appearances; and TrueType subsetting - DejaVu Sans goes from 759,720 bytes to 4,348 for twelve characters. cmap is deliberately not rebuilt: the subset is embedded as a CID font with Identity-H, so the content stream addresses glyphs by id and /ToUnicode serves extraction. A cmap disagreeing with the content stream is worse than none. CFF is refused by name rather than emitting a font with no glyphs. Nine real bugs, every one found by running the output through an independent tool rather than by reading the code: 1 page size discarded reading a generated file back 2 dict keys unescaped probing the writer 3 NaN written as a keyword probing the writer 4 subset zeroed the lsb fontTools outline compare 5 hmtx indexed by new gid fontTools outline compare 6 name table format read as count BaseFont came out "Embedded" 7 add_font shifted numbers already handed out 8 trees allocated over font numbers - object 29 written twice 9 widgets missing /F Print, /P and appearance /Resources 7 and 8 are the instructive pair: every reference resolved and every object existed, each simply named the wrong thing. pypdf reported correct field values from a file PDFium rendered blank. 9 is the one only a renderer could find - /F defaults to non-printable, and a form XObject naming a font its /Resources does not declare is discarded whole. Verified by three independent implementations: fontTools (0 outline mismatches of 12 against the source font), pypdf (metadata, page sizes, outline with resolved page numbers, all five fields, attachment byte-for-byte, labels ['i','1']) and PDFium, which renders both pages correctly. cargo run -p nigig-pdf-graphics --example generate_sample regenerates the sample. Fourteen mutations. Three survived and each exposed a weak test: the key test used an attachment name (written as a string, never a key), nothing read the outline open state, and /P could not be witnessed because page_index is supplied by the reader, which already knows the page. All three now killed. pdf: 789 passed (was 730). pdf-ui: 775. Coverage 85.17%. |
|||
| 6d2e3fb696 |
fix(pdf): repair the tree a hand-resolved merge left red
Commit
|
|||
| 258fa3259e | Merge origin/main: resolve xref/document conflicts, add makepad_table | |||
| fb95b25a67 |
fix(pdf): LZW was broken outright; refuse image codecs instead of faking them
Phase 3 of NIGIG_PDF_FEATURE_PARITY_PLAN.md, lossless half. Design and
merge criteria in REVIEWS/adr/0015-pdf-filters-and-codecs.md.
LZW DID NOT WORK
Fed the worked example from PDF 32000-1 section 7.4.4.2:
LZW default : Err("LZW previous code out of range")
decode_lzw seeded a 256-entry dictionary but set next_code = 258, because
256 and 257 are the clear and EOI codes. New entries were appended with
table.push, landing at index 256 - so the counter and the real index were
permanently two apart and every dictionary reference resolved to the wrong
entry. Any PDF using LZW was affected, which is a whole class of older
files.
Also in the same area:
- /EarlyChange was ignored. It selects when the code width grows; a file
setting 0 decoded to GARBAGE rather than failing, which is worse.
- Predictors were applied to Flate only, though /Predictor is equally legal
on LZWDecode.
TWO MORE BUGS FOUND WHILE IMPLEMENTING
decode_stream read /Filter as a single NAME and fell through to
"unsupported filter" for an array. The document layer calls decode_stream,
so every chained stream in every document failed to decode - including the
common [/ASCII85Decode /FlateDecode]. It now delegates to
decode_stream_with_params, leaving one decoding path.
decode_flate_with_predictor inflated its own input, so calling it from a
chain decompressed already-decompressed bytes. Split into apply_predictor,
which works on decoded data.
IMAGE CODECS: REFUSED, NOT FAKED
DCTDecode and JPXDecode previously returned their COMPRESSED bytes as
though decoded:
"DCTDecode" | "JPXDecode" | "Crypt" => data,
A caller received a Vec<u8> that looked like image data, was not, and
produced garbage pixels rather than an error. CCITTFaxDecode, JBIG2Decode,
JPXDecode and DCTDecode now return a typed error naming the filter.
image.rs still sniffs and decodes JPEG on the image path, so that route is
unaffected; what stops is the generic filter claiming a success it did not
achieve. /Crypt stays a pass-through, correctly - decryption already ran.
Not implementing CCITT/JBIG2/JPX is a decision, not an omission: JBIG2's
CVE record is why browsers sandbox it, and JPX via openjpeg would add a C
dependency that breaks the Android cross-compile the team is already
fighting. CCITT is the tractable one and is the recommended next step.
4 corpus fixtures, 14 acceptance tests. Mutation-checked - and one check
initially misled me: removing the reserved-slot seeding did not fail the
tests, because the clear-code branch re-seeds independently and every real
LZW stream opens with a clear code. Removing both fails all three LZW
tests. Recorded in the ADR.
One pre-existing defect deliberately left: the PNG predictors do not
consume the per-row filter-type byte. Fixing it risks every
Flate-with-predictor document in the corpus and is not what this ADR set
out to do, so it is documented rather than quietly half-fixed.
TEST_TARGET=pdf 637 -> 651, TEST_TARGET=pdf-ui 682 -> 696.
rustfmt and clippy -D warnings clean.
|
|||
| 6a18886185 |
feat(pdf): Type 3 fonts and streaming interpretation — Phase 2 complete
The last two items of NIGIG_PDF_FEATURE_PARITY_PLAN.md Phase 2. Design and
merge criteria in REVIEWS/adr/0014-pdf-type3-fonts-and-streaming.md.
TYPE 3 FONTS DREW NOTHING
A Type 3 font's glyphs are not outlines - they are content streams, listed
in /CharProcs and mapped to text space by /FontMatrix. Probing a document
with one:
fonts on page: ["T3"]
T3: subtype=Type3 base=Unknown
-> are the glyph procedures reachable? no CharProcs field exists
-> is /FontMatrix exposed? no field exists
The font was detected and then nothing could be done with it. /CharProcs and
/FontMatrix appeared nowhere in the crate, so the procedures were unreachable
and the text was silently invisible - a page that renders, reports no error,
and is missing content.
New pdf-document/src/type3.rs parses /FontMatrix, /CharProcs, /Differences,
/Widths, /FontBBox and the font's own /Resources, and resolves a character
code to its glyph procedure's decoded bytes. /FontMatrix is applied as
written rather than assumed to be the common 0.001 scale - Type 3 fonts
routinely use other matrices, which is the point of the entry. A missing
/CharProcs entry is a typed error naming the glyph, not a blank.
A THIRD BUG, FOUND WHILE WIRING d0/d1
The interpreter parsed both operators and discarded them:
PdfOp::Type3Width(_wx, _wy) => {}
PdfOp::Type3BBox(_x1, _y1, _x2, _y2) => {}
They are how a Type 3 glyph declares its advance, so even a renderer that
could draw the glyphs would stack them all at one point. Wiring them to the
device exposed that `d1` takes SIX operands - wx wy llx lly urx ury - and the
parser read four, so the "bounding box" was really the advance and the
advance was lost entirely. Now `Type3BBox { wx, wy, bbox }`, reading all six.
STREAMING INTERPRETATION
parse_content_stream materialised every operator into a Vec before
interpreting any of them: peak memory proportional to the whole content
stream, on a stream walked once and discarded. Adds ContentStreamIter and
interpret_streaming, with parse_content_stream reimplemented on top of the
iterator so there is ONE tokeniser rather than two that can drift.
Equivalence is proven, not asserted: a test compares both paths across every
corpus fixture, and a streaming_interpreter fuzz target compares them over
arbitrary bytes, which is where a divergence would actually hide.
4 corpus fixtures, 13 acceptance tests, 9 unit tests. Mutation-checked:
reverting d0 to a no-op fails glyph_advances_reach_the_device.
Phase 2 is now complete; the plan is updated with an item-by-item audit.
Several entries were already done (inline images, Do, text state, shading);
the plan's "biggest gap" was xref streams, closed in ADR 0013.
TEST_TARGET=pdf 615 -> 637, TEST_TARGET=pdf-ui 660 -> 682.
rustfmt and clippy -D warnings clean.
|
|||
| 2faadb777f |
feat(pdf): read xref streams and object streams (PDF 1.5+)
Phase 2 of NIGIG_PDF_FEATURE_PARITY_PLAN.md, the item it calls "the biggest parse-side gap". Design and merge criteria in REVIEWS/adr/0013-pdf-xref-streams.md. The parser could not open a PDF 1.5 file. Not render it wrong - not open it: PARSE FAILED: PDF error at byte 382: expected xref keyword XRefTable::parse_section required the literal bytes `xref` at the startxref offset. A PDF 1.5+ file has an indirect object there instead - the xref stream - so the parse aborted and the entire document was unreadable. Every feature built on top of the parser (encryption, signatures, forms, structure tree, transparency) was unreachable on any file produced in the last twenty years. ObjStm, XRefStm and /Type /XRef appeared nowhere in the crate. Implemented on the read side: - Xref streams: the packed binary table, /W field widths, /Index sparse subsections, and types 0/1/2. A zero-width /W column means "use the default" (type 1) - missing that rule yields a table of all-free entries and an apparently empty document rather than an error. - Object streams: type-2 entries resolve through /ObjStm, reading the header pairs and /First. The xref's index is used but verified against the object number it claims to be, because a wrong-but-in-range index would silently return a different object. - Hybrid files: a traditional table plus /XRefStm. Both are read, with the traditional table winning on conflict, which is the point of the layout. Bounds and refusals rather than silent degradation: /W widths are clamped and every field read is checked against the decoded buffer; a truncated table is flagged, not padded with free entries; an object claiming to live inside itself is refused; a /Type that is not /XRef is named in the error. Scope note: the writer is untouched. ADR 0003 keeps appending a traditional xref section, which remains correct - the appended trailer carries /Prev to the stream, so the chain stays readable by us and by conforming readers. Also verified against the rest of Phase 2: inline images, XObject Do, shading, and the full text state (Tc/Tw/TL/Tz/Ts/Td/TD/Tm/Tf) are already implemented and tested. Type 3 fonts and the streaming interpreter remain genuine gaps, but each degrades one feature rather than the whole file. 6 corpus fixtures, 9 acceptance tests asserting real page content rather than a successful parse, and a parse_xref_stream fuzz target because the table is attacker-controlled binary. Mutation-checked: restoring the old error fails 5 of the 9. TEST_TARGET=pdf 606 -> 615, TEST_TARGET=pdf-ui 651 -> 660. rustfmt and clippy -D warnings clean. |
|||
| f8446fe041 |
feat: nigig-build cost estimator, pay security prefs, location/sync pipeline, pdf parity docs
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
nigig-map / test (push) Has been cancelled
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
- nigig-build: cost_estimator tests + makepad-test dev-dep for UI parity suites - nigig-pay-ui: file-backed SecurePreferenceStore (security_prefs) for biometric opt-in, wired into shared pay sheet + payments frame - nigig-core: rewrite location.rs subscriber model (drop robius_location Manager sendable wrapper), real Nominatim parser, expanded syncing pipeline - nigig-uikit: camera widget layout rework for permission flow - map/rider: drop makepad 'maps' feature (fork map module doesn't compile at pinned rev); i_tree 0.19.0 pin - pdf-cos: remove debug-only xref round-trip test - docs: NIGIG_PDF_FEATURE_PARITY_PLAN.md (10 phases, dart-pdf test inventory, scale table), workflow.md makepad fork-sync + pdf context sections, THIRD_PARTY_NOTICES.md for dart-pdf attribution - pageflipnav: NDK toolchain env notes for android builds |
|||
| 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. |
|||
| 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. |
|||
| 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.
|
|||
| 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. |
|||
| d3ccc2e00f |
test(pdf): close the three gaps carried from Phases 4 to 7
Three items were carried forward as known gaps rather than quietly dropped. This addresses all three; two are closed outright and one is bounded by an environment limit that is now documented rather than implied. 1. Fuzzing had never actually run (Phase 6 step 6.3). The five cargo-fuzz targets were only compile-checked, so "zero panics on arbitrary input" was an aspiration. They have now been run under nightly libFuzzer: parse_object 1,970,750 runs parse_xref 2,471,345 runs decode_stream 1,120,019 runs parse_content_stream 2,655,663 runs parse_document 2,381,367 runs About 10.6 million executions in total, no crashes and no new findings. That is a real result rather than a green checkmark: the three crashes the corpus found in Phase 6 were the ones worth finding, and the fuzzer confirms the fixes hold under adversarial input. 2. Combo dropdown overlay (Phase 4 step 4.3). A combo box that cannot be opened is a text field with extra steps, so the open list is real state, not a rendering detail. Clicking a combo box opens its options; the dropdown takes a click before any field underneath it, matching the draw order; choosing a row sets the value through DocumentFormEditor; clicking elsewhere dismisses it without changing the value. render_open_combo() returns placement data so the drawing code stays trivial and the geometry is testable without a renderer. 3. Makepad event delivery. Upstream added a makepad_test framework, so this is now testable in principle. Adds a test host binary and six UI tests that drive the widget through the Studio protocol: a real click on the fixture link must surface OpenUri on the host, typing must reach the field, and a click on empty space must emit nothing so the positive assertions are not vacuous. They are #[ignore] by default because the Studio hub cannot start an app in this sandbox: the harness launches with --stdin-loop, which Makepad refuses without a Studio websocket, and the build exits 101 before startup. Upstream own spreadsheet-ui and map UI suites fail identically here with the same error, so this is the environment rather than this code. The tests are checked in and compiled by cargo test so they cannot rot, CI runs them where a hub exists, and the module documents how to run them by hand. Getting there also fixed a real defect in the test host: it copied ui.main_view.render() from the spreadsheet app startup hook, but a plain View has no render method, so the app errored at startup. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (270 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (321 tests, 6 ignored) cargo +nightly fuzz run <target> -- -max_total_time=60 (5 targets) 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. |
|||
| 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 |