andodeki
ad3fe19b90
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
Ten PDF feature commits landed without an ADR, covering three whole phases.
Every feature gets one; these are the four that were owed. Written against
the code as it stands and re-verified by running it, not transcribed from
the commit messages.
0020 CCITT, JBIG2 and JPEG 2000 — the codecs ADR 0015 refused by name
0021 Phase 4 completion — stamping, reconciliation, CFF, cmap, and the
audit that corrected a false "complete" in ADR 0019
0022 Editing — content_edit, page_ops, flatten, catalog_edit
0023 Redaction and compaction, and the three reader defects they found
Verified rather than assumed, on the tree at f75c1cc:
ccitt 30, jbig2 29, jpx 44 unit tests; jpx_roundtrip's 7 compare against
OpenJPEG-generated pixels, exactly, because a JPEG 2000 bug produces a
plausible image rather than an error.
Redaction re-checked with a test written from outside the module. The
middle line is the one worth keeping:
after redact, secret present = true <- earlier revision
after compact, secret present = false
public text retained = true
Redaction alone leaves the secret in the bytes. That is why the two
shipped together, and why the report carries
earlier_revisions_retain_content.
Determinism re-checked by generating the same document from four separate
processes: byte-identical. The HashMap key-order defect really is fixed.
This ADR contains a correction to its own first draft. 0022 initially
listed text-run rewriting and annotation sub-types as deferred, on the
strength of a grep for "rewrite" finding nothing. Both are implemented —
the rewriter is ContentEditor::set_text, which preserves the operator kind
so a ' keeps its line advance and a TJ keeps its kerning; and "callout" and
"comment" are dart-pdf's names for a FreeText with a /CL and a Text
annotation, not distinct /Subtype values. Checking the plan's claim against
the code was right; concluding from a failed grep was not.
One criterion across the four is left unticked, and it is real: the ui.rs
interaction tests, blocked on the Makepad headless backend since Phase 1.
0021 also records a process note. ADR 0019's merge criteria were derived
from what had been built, so all of them ticked while three unbuilt items
stayed invisible. Criteria should come from the plan text first, then be
marked done or explicitly deferred — a deferral stated is fine, a deferral
unstated is a false claim.
pdf: 1187 passed. No code changed.
144 lines
6.5 KiB
Markdown
144 lines
6.5 KiB
Markdown
# ADR 0020: CCITT, JBIG2 and JPEG 2000 — the three codecs Phase 3 refused
|
|
|
|
- **Status:** Accepted
|
|
- **Date:** 2026-08-17
|
|
- **Review item:** `NIGIG_PDF_FEATURE_PARITY_PLAN.md` §1 Phase 3, "Lossy/
|
|
bitonal: CCITT G4 (and G3), JBIG2 (arith coding, huffman, refinement),
|
|
JPX/JPEG2000"
|
|
- **Supersedes:** the `REFUSED_CODECS` decision in ADR 0015, in part
|
|
- **Related:** ADR 0015 (filters — refused these three by name), ADR 0016
|
|
(the JPEG decoder that was a stub)
|
|
- **Written retrospectively.** The work landed in `b26e6a1`, `81e846a` and
|
|
`674b2be` without an ADR; this documents it against the code as it
|
|
stands, with every claim re-verified by running it.
|
|
|
|
## Context
|
|
|
|
ADR 0015 refused CCITT, JBIG2 and JPX **by name**, with typed errors, and
|
|
said so loudly rather than returning compressed bytes as though they were
|
|
decoded. That was the right call at the time — ADR 0016 had just found a
|
|
JPEG "decoder" that returned black rectangles at full alpha — but it left
|
|
three codecs unimplemented, and between them they cover most scanned
|
|
documents in existence. CCITT G4 is what a fax machine and almost every
|
|
document scanner emits; JBIG2 is what a modern scanner emits instead; JPX
|
|
turns up in archival and medical PDFs.
|
|
|
|
A viewer that refuses all three renders a blank page for a large fraction
|
|
of real-world scanned files. The refusal was honest, but honest and useless.
|
|
|
|
## Decision
|
|
|
|
Implement all three, as pure Rust in `pdf-cos`, with no C dependency. The
|
|
engine crates compile no C today and that property is worth keeping: it is
|
|
what makes the WASM and mobile targets viable and keeps the untrusted-input
|
|
surface inside the memory-safety guarantees of the language.
|
|
|
|
| Codec | File | LOC | Tests |
|
|
|---|---|---|---|
|
|
| CCITT G3/G4 | `ccitt.rs` | 1,320 | 30 |
|
|
| JBIG2 generic region | `jbig2.rs` | 1,282 | 29 |
|
|
| JPEG 2000 | `jpx.rs` | 2,110 | 44 |
|
|
|
|
### Where each one is decoded, and why they differ
|
|
|
|
`CCITTFaxDecode` joins `SUPPORTED_FILTERS` and decodes through the generic
|
|
filter path. `JBIG2Decode` and `JPXDecode` stay in `REFUSED_CODECS` — but
|
|
the reason changed, and the reason is the whole point:
|
|
|
|
```rust
|
|
("JBIG2Decode",
|
|
"JBIG2 is decoded on the image path, where /Width and /Height are known"),
|
|
("JPXDecode",
|
|
"JPEG 2000 is decoded on the image path, where the codestream \
|
|
carries its own geometry"),
|
|
```
|
|
|
|
This is not a leftover. CCITT's parameters arrive in `/DecodeParms`, so the
|
|
filter layer has everything it needs. JBIG2 needs the image dictionary's
|
|
`/Width` and `/Height`, which the filter layer cannot see. JPX carries its
|
|
own geometry *inside the codestream*, which may disagree with the image
|
|
dictionary, and resolving that conflict is an image-layer decision.
|
|
|
|
Returning bytes from the generic path for either would repeat exactly the
|
|
`DCTDecode` defect ADR 0015 fixed: a caller receives a `Vec<u8>`, believes
|
|
it is decoded, and gets garbage. Refusing by name with an accurate reason is
|
|
correct here, and the reason string is what tells a maintainer it is
|
|
deliberate.
|
|
|
|
### Fixtures generated by a reference implementation
|
|
|
|
The JPX corpus is generated by **OpenJPEG**, through Pillow, into
|
|
`tests/corpus/jpx/` with expected pixels in `expected.json`. The generator's
|
|
own doc comment states the reasoning better than a summary would:
|
|
|
|
> a JPEG 2000 bug does not raise an error, it produces a slightly soft or
|
|
> banded image that looks completely plausible, and only a reference
|
|
> encoder's pixels can tell you it happened.
|
|
|
|
Every fixture is **lossless** — reversible 5/3 wavelet, no quantisation — so
|
|
the comparison is exact. An irreversible fixture would need a tolerance, and
|
|
a tolerance is where a subtly wrong decoder hides. That is the same argument
|
|
ADR 0016 reached the hard way when a uniform 64-level error in the IDCT took
|
|
three attempts to find.
|
|
|
|
## Verification
|
|
|
|
Re-run against the tree at `f75c1cc`, not taken from the commit messages:
|
|
|
|
```
|
|
cargo test -p nigig-pdf-cos --lib ccitt:: 30 passed
|
|
cargo test -p nigig-pdf-cos --lib jbig2:: 29 passed
|
|
cargo test -p nigig-pdf-cos --lib jpx:: 44 passed
|
|
cargo test -p nigig-pdf-cos --test jpx_roundtrip
|
|
a_lossless_grayscale_codestream_decodes_exactly ... ok
|
|
a_lossless_rgb_image_decodes_exactly ... ok
|
|
an_rgb_image_using_the_colour_transform_decodes_exactly ... ok
|
|
the_jp2_container_decodes_to_the_same_pixels_as_the_raw_codestream ... ok
|
|
a_larger_lossless_image_decodes_exactly ... ok
|
|
a_truncated_codestream_does_not_return_the_whole_image ... ok
|
|
a_corrupt_codestream_never_panics ... ok
|
|
```
|
|
|
|
"Decodes exactly" is against OpenJPEG's pixels, not ours.
|
|
|
|
Two of those seven are the ones that matter for untrusted input: a truncated
|
|
codestream must **not** return a whole image (returning a plausible partial
|
|
image is the silent-empty failure), and a corrupt one must not panic.
|
|
|
|
## Merge criteria
|
|
|
|
- [x] CCITT G3 and G4 decode; registered in `SUPPORTED_FILTERS`
|
|
- [x] JBIG2 generic region decodes on the image path
|
|
- [x] JPEG 2000 decodes, both raw codestream and JP2 container
|
|
- [x] Reversible 5/3 wavelet exact against OpenJPEG
|
|
- [x] Multi-component transform (MCT) exact against OpenJPEG
|
|
- [x] Fixtures generated by a reference encoder, not by us
|
|
- [x] Truncated input does not yield a whole image
|
|
- [x] Corrupt input never panics
|
|
- [x] JBIG2 and JPX remain refused at the *generic filter* boundary, with
|
|
a reason naming where they are decoded instead
|
|
- [x] No C dependency added
|
|
- [x] `TEST_TARGET=pdf` green
|
|
|
|
## Consequences
|
|
|
|
**Positive.** Scanned documents render. Between CCITT and JBIG2 that is most
|
|
scanner output of the last thirty years.
|
|
|
|
**Negative.** 4,712 lines of dense codec arithmetic to maintain, none of it
|
|
exercised by the ordinary text-and-vector path. The JPX wavelet in
|
|
particular is the kind of code where a sign error produces a plausible
|
|
image, which is why the OpenJPEG fixtures are load-bearing rather than
|
|
decorative: without them this code is untestable in practice.
|
|
|
|
**Risk.** Each decoder is a parser of hostile input in a security-sensitive
|
|
position. They are pure Rust, so a bug is a wrong image or a panic rather
|
|
than memory corruption, and the corpus includes truncated and corrupt cases
|
|
for each. The fuzz targets should grow to cover all three; today the
|
|
manifest declares 14 targets and the codec surface is newer than that list.
|
|
|
|
**Not done, deliberately:** JBIG2 refinement regions, symbol dictionaries
|
|
and Huffman-coded segments (only the arithmetic-coded generic region is
|
|
implemented); JPEG 2000 irreversible 9/7 wavelet at full precision, ROI
|
|
coding, and progression orders beyond those the fixtures exercise. Each
|
|
would need its own reference fixtures before it could be claimed.
|