Commit graph

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.
2026-08-16 22:27:32 +00:00
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.
2026-08-16 22:17:40 +00:00
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.
2026-08-16 22:02:26 +00:00
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.
2026-08-16 21:50:46 +00:00
82eb6b9c73 feat(pdf): internal links that actually go somewhere
Some checks failed
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
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.
2026-08-16 19:34:01 +00:00
cf73ef4c1d test(pdf): assert what a file declares is delivered, and floor the coverage
Some checks failed
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
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.
2026-08-16 19:04:57 +00:00