6 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| 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. |
|||
| 82eb6b9c73 |
feat(pdf): internal links that actually go somewhere
ADR 0017 left destinations.rs at 0% coverage as an open item. The obvious
reading is "an untested module". The real one is worse: nothing called it.
It was pub use'd from lib.rs and referenced from nowhere else in the
workspace. 0% was not a gap in the tests, it was the symptom of dead code,
and nothing else was doing the job.
Meanwhile PdfAnnotation read a link's target as
dict.get_name("Dest") - a *name* /Dest and nothing else. Not
/Dest [4 0 R /Fit], and not /A << /S /GoTo /D ... >>, which is how internal
links are written in practically every real document.
The corpus has had one since Phase 6, in annotations/links.pdf, and no test
asserted where it went:
Link { uri: None, dest: None } -> action=None
Clicking it did nothing. No error, no warning - the viewer got no action and
correctly performed none. A link to nowhere and a link the reader cannot
parse look identical from outside. The viewer was already wired for this:
PdfAction::GoToPage exists, is matched in test_host.rs, and was never
constructed by anything. A complete delivery path with nothing at the source.
Now: all three legal spellings parse, named destinations resolve through the
/Names /Dests tree *and* the pre-1.2 /Root /Dests dictionary, and resolution
happens in page_annotations where the catalogue is in reach.
XYZ keeps Option per component because null is meaningful there and only
there - it means "leave unchanged". Reading it as 0.0 scrolls to the origin
at 0% magnification. Zoom 0 means the same as null and is normalised.
Lookup uses a deliberate shallow resolve. Deep-resolving a destination array
replaces [4 0 R /Fit] with the page dictionary and destroys the only thing
identifying the target - the defect that once emptied every AcroForm
(ADR 0006) and every annotation reference (ADR 0004).
GoToAction now requires /S to be GoTo. The old code ignored /S and took /D
from whatever it was handed, so a /GoToR (another file), /Launch (a program)
or /JavaScript carrying a /D was reported as a local page jump. Refuse by
verb, same policy as ADR 0012. An unresolvable destination is left
unresolved, never defaulted to page 0: silently landing on page one is the
worst outcome because it looks like the link worked.
Seven mutations, all killed. M1 - removing the /S check - reported as
surviving on the first attempt. It had not survived: the patch string
omitted an interleaved comment so the mutation never applied and I measured
the unmutated build. A harness that does not verify its own mutation says
"weak test" when the truth is "never ran", and the conclusion would have
been to delete a real security check. Every mutation now asserts it applied.
destinations.rs 0% -> 98.65%; total 83.42% -> 83.86%. Floors added for
destinations.rs and annotations.rs, verified to fail when breached.
AnnotationType::Link changes shape (dest: Option<String> ->
destination: Option<Destination>) and AnnotationAction gains
GoToDestination; the old field could not express an explicit destination, so
keeping it meant keeping the bug. AnnotationAction loses Eq because a
destination carries f64 coordinates.
pdf: 724 passed (was 695). pdf-ui: 769 passed (was 725). ADR 0018.
|
|||
| cf73ef4c1d |
test(pdf): assert what a file declares is delivered, and floor the coverage
Every serious bug in this stack has had one shape: a valid, well-typed,
empty-or-default value where the file plainly declared content. xobjects
empty for every document; acroform() dropping every field behind an
indirect reference; DCTDecode returning its own compressed bytes; a JPEG
decoder that was a stub returning black. None errored, none panicked, and
the tests asserted Ok, which they got.
Coverage would not have caught any of them. Measured when each shipped:
page.rs 92.4%, form.rs 93.6%, content.rs 89.2%, xref.rs 95.2%. The buggy
lines ran; nobody checked what they produced.
So: a property test that walks the raw object graph of every corpus
fixture, counts what the file declares, and requires the API to deliver
it - fonts, xobjects, graphics states, colour spaces, form fields,
filters, MediaBox. It reimplements the resolution rule independently of
page.rs on purpose; a test that asks the code under test what to expect
agrees with the bug.
It failed the day it was written, on a shape the corpus had never
contained. Every fixture wrote /Resources inline, and all six extractors
read it with dict.get_dict("Resources") - which returns None for an
indirect reference and never consulted /Parent. A page with
"/Resources 5 0 R", the commonest shape in real PDFs, reported no fonts,
no xobjects, no graphics states and no colour spaces. Same for a page
inheriting resources from its /Pages node. Empty, not wrong, so nothing
failed.
Fixed by resolving /Resources once in PdfPage::from_obj through a helper
implementing the full inheritance rule (32000-1 Table 30), and passing
the resolved dictionary down. Indirect /MediaBox entries resolve too.
Six resources/ fixtures cover the shapes that were missing.
Mutation-checked: reverting inheritance kills 5 tests, the sub-dict
reference 3, indirect MediaBox 2, and removing the depth bound hangs.
One mutation survived - a visited-set guarding a /Parent cycle, which
the depth bound already handles - so it was deleted rather than left as
untested defence with a reassuring comment.
tools/test-pdf-coverage.sh enforces a floor instead of printing a number,
with per-file floors as well as a total: image.rs could fall from 33% to
5% and move the total by under a point. All three failure modes verified
to fail. It caught a bug in itself first - its ignore regex matched its
own work directory and reported a confident TOTAL 0.00%.
.gitattributes marks *.pdf binary. An xref entry must be exactly 20 bytes
(7.5.4), so with a one-digit generation field it ends in a space, and
git diff --check was reporting unfixable "trailing whitespace" on every
fixture in the corpus.
TEST_TARGET=pdf: 695 passed, 0 failed (was 680). Coverage 83.42%.
ADR 0017 records the four mutations so they can be repeated by hand.
|