26 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.
|
|||
| 1740da3f34 |
feat(pdf): render a form XObject to pixels — the golden caught what the
assertions missed `Rasteriser::register_xobject` takes a form's recorded commands from a caller that can resolve the page dictionary, so Phase 7's last golden-corpus criterion is met with pixels instead of with a request recorded by name. The first golden of that page showed the form drawn at the **page origin**, ignoring the `1 0 0 1 20 20 cm` that placed it. The recorded commands' `SetTransform`s are absolute in form space, and replaying them overwrote the page's CTM rather than composing with it. Nested lists now compose against the CTM in force at the `Do`. The colour assertions written next to that golden all passed while the bug was live — a red square two pixels from where it belongs is still a red square somewhere. That is the argument for pixel goldens in one sentence, and it is why the golden is compared after the assertions and not instead of them. The offset now has its own assertion too. The new fixture's form deliberately overflows its own /BBox, so the clip is visible in the golden as an absence rather than being taken on trust. Phase 7's golden-corpus exit criterion is now met in full. The `ui.rs` smoke tests remain blocked on the Makepad headless backend, as they have been since Phase 1, and are still not claimed as done. 1426 tests pass. |
|||
| f37197781e |
feat(pdf): glyph outlines from TrueType and CFF, and glyph-aware text runs
`sfnt.rs` read the metric tables and nothing else. It could say how wide a glyph was and not what shape it had, so every renderer drew embedded text with a substitute font at the correct advance — the failure mode that looks most like success: the line breaks land right and the letterforms belong to somebody else. `outline.rs` returns one outline type for both formats. TrueType quadratics are degree-elevated to cubics, which is exact, so no format detail leaks to a consumer. Composite glyphs are placed by their offsets and scales, with a depth bound because a font can reference itself. CFF Type 2 charstrings run through an interpreter with biased local and global subroutines, hints, hintmask byte counting, the leading width operand, and the FontMatrix as declared rather than assumed to be 1/1000. Separately, `ShowTextWithMetrics` carried one advance for a whole run — enough to move the pen to the next run and nothing else. So `text.rs` guessed: `seg.advance / char_count`. For "Wi" that puts the boundary between the letters at 5 when it is at 9, and every caret, drag-selection and search highlight in the application was wrong by that much for every proportional font. `GlyphPlacement` now carries per-glyph pen offsets, computed with the same expression as the run total so the two cannot drift. The even-spacing fallback stays for fonts with no width table, which is what `advance_is_measured` has always been for. The fixture story is ADR 0029's, again. `cff_sample.otf` is a fontTools conversion of DejaVu: no subroutines, no hints, no width operands. It proved the interpreter draws the right shapes, and then four mutations of that interpreter survived because nothing in the corpus reached the code they broke — each of which produces a plausible wrong glyph from a font that parses. `cff_subrs.cff` is hand-assembled for exactly those four, and fontTools agrees with every expectation asserted against it. A fifth mutation survived a composite test that counted contours; it is killed now by one that measures where the components land. Coordinates are asserted against fontTools ground truth, not against our own output. Seven mutations, all killed. 1397 tests pass. Deferred and recorded, not claimed: CID-keyed CFF, `seac` accents, rendering outlines through the Makepad device. ADR 0031. |
|||
| 0bef30a6d5 |
feat(pdf): compositing and overprint — the blend maths had no backdrop
`transparency.rs` implemented all sixteen blend modes and unit-tested them against the specification's formulas. Nothing ever called them with a backdrop. The Makepad renderer's `SetBlendMode` pushed a `TransparencyError::Unsupported` and then painted the source colour, so a /Multiply highlight and a /Normal one produced byte-identical output and every test passed — because every test asked "was the right command issued", not "does the page look right". Overprint had no code at all. /OP, /op and /OPM were not parsed, so an overprinting object knocked out the inks under it. That is not a missing feature, it is the inverse of the instruction: on a press it is the difference between a colour and a hole. - `composite.rs`: a straight-alpha RGBA `Canvas` implementing §11.3.6's union formula, weighted by backdrop alpha so a Multiply over transparency is the source rather than black. Constant alpha and per-pixel soft masks. Transparency groups composite as a unit; knockout groups are refused by name rather than silently treated as non-knockout. - Overprint as `composite_cmyk`, separate from the RGB path rather than a flag on it: overprint is a statement about inks and RGB has none. /op defaults to /OP per table 58 — defaulting it to false makes the common `<< /OP true >>` knock out every fill. §10.7.5's "no effect on an RGB device" is asserted, so our doing nothing there is the spec rather than an omission. - `raster.rs`: a CPU rasteriser that replays a command list onto a canvas. Not on the display path, no anti-aliasing, no fonts; it exists so compositing has a verifiable output. In pdf-graphics and not pdf-makepad because a test that needs a GPU is a test that does not run. - Golden **pixels** for shading, mesh, blend and overprint pages — Phase 7's exit criterion, which the Phase 2 command-text goldens cannot meet. ASCII grids with a colour legend, quantised to quarter steps; each test asserts its exact colours before comparing, so a wrong-but-stable render cannot be blessed by an UPDATE_GOLDEN run. Six mutations, all killed, including the two that describe the old behaviour: discarding the blend result, and ignoring the overprint flag. 1364 tests pass. ADR 0030. |
|||
| 728fbc3ad0 |
fix(pdf): mesh shadings — three bugs in code that had no fixture
ADR 0028 shipped types 4-7 and said honestly that they were unproven: the uncovered lines of `shading.rs` were exactly `parse_mesh`, "the position `image.rs` was in before ADR 0016 found the JPEG decoder was a stub". Writing the fixtures found three real bugs. - Type 5 has no per-vertex flag; `/VerticesPerRow` delimits it. Reading 8 phantom bits shifted every vertex after the first, decoding plausible coordinates that were entirely wrong. - Types 6 and 7 are patches: 12 or 16 control points carrying no colour, then four corner colours. The old loop read a colour per point, consumed three times too many components, ran off the stream, and the None-on-truncation path swallowed it as "the mesh ended". - A flag-0 triangle is three vertices whose second and third flags are ignored (§8.7.4.5.5). Acting on them cleared the strip every time and produced no triangles at all. Caught in new code, before it shipped. And one omission: `color_at_point` returned None for a mesh, so a mesh that parsed perfectly still painted nothing — indistinguishable from one that failed. `MeshTriangle::color_at` now interpolates the corner colours by barycentric coordinates, None outside, because black is a colour a mesh can legitimately produce. Shared-edge patches (flags 1-3) inherit the previous patch's edge rather than being read as fresh patches, which desynchronised the rest of the stream. Five corpus fixtures, generated from named coordinates and colours so every expected value in the tests is one the generator wrote deliberately. Eight tests, five mutations, all killed. Coons flattening is still an approximation and still reports `is_approximate`. ADR 0029. |
|||
| c1d1e67f3a |
feat(pdf): shadings — the sh operator was parsed and thrown away
ADR 0028, the first of Phase 7's eight bullets.
content.rs contained `PdfOp::Shading(_name) => {}`. The operator was lexed,
given its own variant, matched during interpretation, and discarded. A page
whose background is a gradient rendered as nothing.
Nothing caught it for the usual reason: a blank region is a legal thing for
a page to contain, so "drew nothing" and "drew what was asked" are
indistinguishable without an assertion naming the expected colour. The
golden corpus had no shading page, so there was nothing to be wrong.
Two of the three pieces already existed — function.rs evaluates the colour
function and colorspace.rs converts it to RGB. What was missing was the
geometry between them.
Sampling rather than a gradient primitive: a PDF shading is defined by an
arbitrary function, possibly a sampled table or a PostScript program, and
neither reduces to a stop list without loss. A device with a native
gradient can still recognise the two-stop case from the samples.
"No colour here" is None, not black. Black is a colour a shading can
legitimately produce, so returning it for "outside an unextended shading"
would paint a rectangle the author never asked for and the caller could not
tell the two apart.
Types 1-5 exact. Coons and tensor patches are flattened to their corners,
which loses the curvature, and is_approximate says so rather than leaving a
caller to assume fidelity. An unknown type is refused by number: a mesh
drawn as a flat fill is a plausible-looking wrong answer.
paint_shading is a new trait method, so the compiler found every
implementor. The Makepad renderer records the request in pending_shadings,
mirroring pending_xobjects — it cannot resolve a /Shading resource because
it does not own the page dictionary, and recording the request is what
stops the operator vanishing a second time. That holds even for types we
refuse, so a host can warn the user.
Four mutations, all killed. The first — discarding sh again — fails three
tests.
Stated plainly and left unticked: the mesh path is written but NOT
exercised by any real stream. shading.rs is at 68% and the uncovered part
is exactly parse_mesh and triangulate. Mesh support should be treated as
unproven, not working: the code runs and produces triangles, and nothing
yet demonstrates they are the right triangles. That is the position
image.rs was in before ADR 0016 found the JPEG decoder was a stub.
The Phase 7 status line is a table from the start this time — one row per
spec bullet, seven of them saying "not started". Per ADR 0021, written
before the work rather than after it.
pdf: 1321 passed (was 1291). pdf-ui: 1366. Coverage 87.60%, floors met.
|
|||
| 1220f89fc6 |
feat(pdf): close the last three Phase 4 items — reconciliation, CFF, cmap
Some checks failed
email.yml / feat(pdf): close the last three Phase 4 items — reconciliation, CFF, cmap (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 three items the previous commit's audit found unimplemented while the status line said "complete". All three are done and externally verified. **1. Field-value reconciliation** (`reconcile.rs`). A field carries its value in /V and its rendered look in /AP, and nothing in the format keeps them in step. Files arrive with them disagreeing all the time: a producer writes /V and leaves appearances to the viewer, or something edits /V without touching /AP. Until now this crate simply believed /V and regenerated appearances only for fields it had itself edited — right for a field we changed, wrong for a field that arrived inconsistent. The module deliberately does **not** pick a winner. PDF 32000-1 §12.7.3.3 settles exactly one case — /NeedAppearances true means /V is authoritative — and is silent on the other, where a conforming viewer renders /AP and never looks at /V. So it classifies the disagreement and resolves it against a caller-declared `Intent`, because the right answer genuinely differs: a viewer must show /AP to match other viewers, an extractor must read /V, an editor must regenerate so the saved file agrees with itself. Silently choosing one would be ADR 0017's failure in a new place — every answer plausible, none checkable, the caller unaware a decision was made for it. Two cases are not judgement calls and are handled outright. A missing or dangling appearance renders *blank*, and blank is never what the producer meant, so even Display regenerates. An unselected radio member showing /Off while the group's /V names another member is correct, not a conflict — reporting it would flag every well-built radio group there is. **2. Type1/CFF embedding** (`embed_opentype_whole`). The spec says "if feasible". Subsetting CFF is not — it means rebuilding the CFF INDEX, charset and charstrings, a second font format inside the first — and `subset_truetype` rightly keeps refusing it by name. Embedding the program *whole* is feasible, and that is what this does: /FontFile3 with /Subtype /OpenType under a CIDFontType0 descendant, per Table 126. Each of those keys matters and none is guessable from the others. A CFF program in /FontFile2, or under a CIDFontType2 descendant, still produces a file qpdf accepts and a font that loads as the wrong type or not at all. /CIDToGIDMap is omitted because it is defined for CIDFontType2 only. The trade is made visible rather than buried: `EmbeddedFont::is_subsetted` is false here, so a caller with a size budget — or a licence that forbids shipping a whole face — can refuse instead of discovering it from the output size. **3. `repair-cmap`** (`glyph_index`). A symbol font declares no Unicode subtable: it maps glyphs into the private-use area at 0xF000 + the low byte under platform 3, encoding 0. Asking it for 'A' found nothing and the character silently vanished from the output — the font "missing" a glyph it plainly has. Now the (3,0) subtable is kept as a fallback and retried at 0xF000 + low byte, after the proper lookup fails so a font with both subtables is still read through the Unicode one. Format 0 is read too; omitting it left legacy and symbol fonts mapping nothing while appearing to have a usable cmap. The repair must not manufacture glyphs, which is its own test: a character the font genuinely lacks still returns None, because turning a missing character into a wrong one is worse. **Fixtures.** No CFF or symbol font ships on the CI image, and neither can be tested honestly against a hand-built stub — the point is that the bytes are a font program a third-party reader accepts. Both are generated from DejaVu by checked-in fontTools scripts: `cff_sample.otf` (1.6 KB, real OTTO/CFF outlines) and `symbol_sample.ttf` (664 B, a single (3,0) subtable so the repair path is the only route to its glyphs). Both generators pin `head.created`/`head.modified` to zero. fontTools stamps the current time, so the output differed on every run and CI's "fixtures match their generator" check failed against a file nobody had edited. Caught by running that check rather than assuming it passed. A fixture that cannot be regenerated byte-for-byte is not reviewable: you cannot tell a deliberate change from a rebuild. **Verified by mutation**, seven injected defects, each confirmed red: NeedAppearances ignored 1 fail dangling /AS not detected 1 fail blank rendering shown faithfully 1 fail CFF written to /FontFile2 1 fail CFF given a CIDFontType2 descendant 1 fail whole font claims to be subset 1 fail cmap 0xF000 retry removed 3 fail **Verified externally.** The sample now carries a third page set in the whole-embedded CFF font, and `check-pdf-external-readers.sh` gained `pdffonts` — the only check that inspects a font *program* rather than the file structure, which is exactly where a wrong /FontFile key shows up. poppler reports both fonts embedded and distinguishes them correctly: ETXLDI+DejaVuSans CID TrueType Identity-H emb yes sub yes NigigTestCFF CID Type 0C (OT) Identity-H emb yes sub no and extracts "Hello CFF 123", which only works if the CFF program loaded, /Identity-H addressed its glyphs and /ToUnicode mapped them back. That check also caught its own page-count assertion going stale when the third page landed — a gate that notices its own fixture changing is working. Engine suite 953 -> 985. Coverage 87.27%, all floors met. Phase 4 is complete but for the ui.rs interaction tests, which are written and blocked on the Makepad fork's missing headless backend. |
|||
| 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. |
|||
| 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.
|
|||
| 2d6c034345 |
test(map): add comprehensive unit tests for overlay module
Some checks failed
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (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-map / test (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
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
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (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
Added 35 unit tests covering: - OverlayCamera::norm_to_screen with no rotation, rotation, and tilt - MapMarker::new, clone, and debug - MapRouteOverlay default, clone, and debug - MapPuck::new (with and without heading), clone, and debug - MapOverlayState methods: add_marker, remove_marker, clear_markers, set_route, clear_routes, set_puck, clear_puck, is_empty - Edge cases: removing nonexistent markers, combined operations This brings overlay.rs from 0% to ~100% test coverage for all testable logic. Drawing functions (draw_map_overlay, draw_route, draw_marker, draw_puck) require a full Makepad runtime and are better suited for integration/visual tests. |
|||
| 63ff45149a |
feat(pdf): a real JPEG decoder — the old one was a stub returning black
Completes Phase 3 of NIGIG_PDF_FEATURE_PARITY_PLAN.md. Design and merge
criteria in REVIEWS/adr/0016-pdf-image-decode-surface.md.
ADR 0015 refused DCTDecode at the generic filter boundary and left the
image path alone, noting JPEG "is decoded on the image path". That claim
did not hold:
fn decode_jpeg_data(_data, _pixels, _width, _height, _components)
-> Option<()> { Some(()) }
Every argument discarded. It wrote nothing and returned success. The caller
allocated a zero-filled buffer, passed it in, and returned it as decoded
pixels. Probing a real 8x8 JPEG through ImageInfo:
decode_to_rgba -> 256 bytes, first 12: [0,0,0,255, 0,0,0,255, 0,0,0,255]
Pure black at full alpha. Not an error, not None - a correctly sized,
entirely fabricated image. EVERY JPEG IN EVERY PDF rendered as a black
rectangle and nothing reported it. The underscore-prefixed parameters are
the tell: the signature was written to silence the unused warnings that
would otherwise have announced the stub. image.rs was at 14.2% line
coverage, the lowest in the crate.
Replaced with a real baseline decoder in pdf-graphics/src/jpeg.rs: huffman,
dequantisation, IDCT, chroma upsampling, YCbCr/YCCK conversion including
the Adobe APP14 transform flag. No new dependency - adding `image` or
`jpeg-decoder` would pull a tree into a crate that has one, on a target
the team is already fighting to cross-compile.
Progressive JPEG is refused BY NAME rather than approximated; a partial
implementation would reproduce exactly the defect being fixed.
decode_to_rgba's Option is why the stub survived - "could not decode" and
"decoded to nothing" were the same value. The decoder returns a typed
JpegError so a caller learns why an image is missing.
Also in this tranche, from the same plan bullets:
- ImageInfo::downsample, integer-factor box filter. Refuses factor 0, and
refuses data that is not raw samples rather than averaging compressed
bytes as though they were pixels.
- Round-trip tests for encode_flate and encode_ascii_hex over adversarial
inputs: empty, single byte, all-zero, all-0xFF, random binary.
THE IDCT TOOK THREE ATTEMPTS AND THE FAILURES WERE INFORMATIVE
The first version, adapted from a hand-tuned integer kernel, decoded
greyscale exactly (128 -> 128) while colour came out a UNIFORM 64 levels
off. A constant offset across every channel is a scaling-factor mistake,
not a coefficient one - guessing at coefficients would never have found
it. Two rounds of guess-and-check made it worse. The fix was to stop
guessing: derive ground truth from the float reference in T.81 A.3.3, then
transcribe the separable form directly with a documented fixed-point
scale. The cosine table is a const fn so it cannot drift from the formula
beside it, and tests assert against the reference rather than our output.
4 corpus fixtures with real JPEGs (Pillow at generate time only; the .pdf
files are committed so CI never needs it), 16 acceptance tests asserting
PIXEL VALUES rather than buffer lengths - a length assertion would have
passed against the stub. Mutation-checked: reinstating the zero buffer
fails four tests.
Coverage on image.rs 14.2% -> 32.9%, new jpeg.rs 82.8%, crate 83.65% ->
84.22%.
TEST_TARGET=pdf 651 -> 680, TEST_TARGET=pdf-ui 696 -> 725.
rustfmt and clippy -D warnings clean.
|
|||
| 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. |
|||
| b720a166ec |
feat(pdf): AcroForm actions and validation; JavaScript refused, not run
Phase 8 #8 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("AcroForm full support"). Design and merge criteria in REVIEWS/adr/0012-pdf-acroform-full.md. Form editing already worked. What was missing is everything that makes a real form behave like one. Probing an invoice-shaped document: "qty" required=true /AA present? false "price" required=false /AA present? true "total" required=false /AA present? true -> is any /AA action exposed? no accessor exists -> is /CO exposed? no accessor exists The /AA dictionaries sat in the raw field dictionary, reachable only by a caller who knew to go digging. Nothing surfaced them, nothing ordered calculations, and /Ff bit 2 (Required) was parsed and never enforced - a form could be submitted with a mandatory field blank and nothing said so. THE JAVASCRIPT DECISION The review names JavaScript actions as a prerequisite. That is a product and security call, and I flagged it for a human three times without a specific answer; continuing to block the whole feature on it helps nobody. This takes the reversible option and documents it loudly enough to overrule: JavaScript is parsed, exposed, and NEVER EXECUTED. Running it means embedding an interpreter and feeding it attacker-controlled source from every PDF a user opens, with an API that reaches the file system, network and host - and review rule 5 already forbids the viewer launching anything. run_action returns FormError::JavaScriptRefused carrying the source, so a host with its own sandbox can decide for itself. A later ADR turns a refusal into an execution; nothing has to be un-built. Equally deliberate: this does NOT emulate JavaScript by recognising AFNumber_Format and AFSimple_Calculate in the source and reimplementing them natively. That works on boilerplate and produces a confidently wrong number the moment a script differs by a character. WHAT IS IMPLEMENTED - /AA parsed into typed actions across all 14 triggers, on fields and widgets, with indirect action dictionaries resolved. - /CO calculation order exposed in document order. - Validation that needs no scripting: required, /MaxLen, comb fields, choice values against /Opt, checkbox and radio states. - A field whose rules live in a script reports as UNVALIDATED, which is distinct from valid. Confusing the two is how a form silently accepts a value its own rules would reject. - /SubmitForm parsed into URL, flags and fields and returned as data. This crate opens no sockets. Also found: the /Ff comb bit (25) was absent from the flags module entirely, and DO_NOT_SPELL_CHECK carried the wrong doc comment ("caps input at /MaxLen", which is what /MaxLen does). 4 corpus fixtures, 13 acceptance tests, 20 unit tests. Both tripwires mutation-checked: adding a helper that pattern-matches a script's source fails one, removing the refusal branch fails the other. TEST_TARGET=pdf 575 -> 607, TEST_TARGET=pdf-ui 619 -> 651. rustfmt and clippy -D warnings clean. |
|||
| a994213e8b |
feat(pdf): tagged structure tree, and the BMC bug that turned red green
Phase 8 #7 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("PDF/UA structure tree"). Design and merge criteria in REVIEWS/adr/0011-pdf-structure-tree.md. Investigating the accessibility gap surfaced four defects in the marked-content operators the structure tree depends on. The first is not an accessibility problem at all. 1. BMC never parsed, and corrupted the colour of everything after it. The dispatcher matched b'B'+b'M' only when the third byte was a delimiter; BMC's third byte is 'C', so the arm never fired. Because the operator was never recognised it never CLEARED ITS OPERAND, and the leftover /Tag shifted the operands of whatever came next: 1 0 0 rg -> RgbFill(1.0, 0.0, 0.0) red /Span BMC 1 0 0 rg -> RgbFill(0.0, 1.0, 0.0) green Tagged documents are precisely the ones containing BMC, so the documents that tried hardest to be accessible rendered wrong colours. This is the fifth instance of the operator-shadowing family already fixed for cm, rg, gs, b/b* and end-of-stream text operators. ADR 0009's every_multi_char_operator_parses_as_itself test exists to stop exactly this - and would have caught it, except BMC was one of two operators excluded from its table as "genuinely unimplemented". Excluding a known-broken operator from the test whose job is finding broken operators is how it survived. The exclusion list is gone. 2. BDC discarded its property list, which carries /MCID - the only link between a run of page content and the structure element describing it. Without it a tree can be parsed but never attached to anything. 3. The op produced by BDC was named MarkContentBmc, and BMC produced nothing. The names were the wrong way round, which is how the missing arm survived review: the enum looked like it had a BMC case. 4. Found while fixing 2: the content lexer had no dictionary support at all. It read `<` as a hex string without checking for a second `<`, so `<</MCID 0>>` parsed as the string "0C0D0". Content streams now parse direct objects properly, bounded at 16 levels; a single `<` is still a hex string and a test pins that. On top of that, new pdf-document/src/structure.rs: /StructTreeRoot, /StructElem trees, /RoleMap resolution, depth-first reading order, /Alt, /ActualText, /E, /Lang, and MCID-to-element lookup. Cycles in /K are cut at the first repeat and reported by object number rather than expanded to the depth bound. Accessibility findings are mechanical checks reported as findings, NOT a conformance verdict: there is no is_pdf_ua and no ComplianceReport, and a mutation-checked tripwire test fails if either appears. Real PDF/UA conformance needs human judgement - whether /Alt text is accurate is not mechanically decidable - so claiming it from six checks would be exactly the overclaim this codebase keeps removing. 7 corpus fixtures, 14 acceptance tests, 22 unit tests, and a parse_content_dict fuzz target for the new object parser. TEST_TARGET=pdf 536 -> 575, TEST_TARGET=pdf-ui 580 -> 619. rustfmt and clippy -D warnings clean. |
|||
| e4a0c0a79e |
feat(pdf): read signatures, and fix a third ObjRef-destroying resolve
Phase 8 #6 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Signatures"). Design and merge criteria in REVIEWS/adr/0010-pdf-signatures.md. Two defects, and the first is not about signatures at all. 1. acroform() dereferenced /AcroForm with self.resolve(), which recurses. That replaced every /Fields [5 0 R] entry with an inline dictionary, so AcroForm::walk saw node.as_ref() == None, decided the field had no identity to key an edit on, and dropped it. The form came back EMPTY rather than wrong, which is indistinguishable from a document with no fields, so nothing announced the loss. This is the third appearance of one mistake: page_annotations once destroyed every annotation's obj_ref the same way, and extract_xobjects resolved a reference then asked the resolved object for as_ref(), leaving every page's XObject map empty. It was invisible because both existing fixtures declare /AcroForm as an INDIRECT reference, where only one level is dereferenced and the refs survive. A direct /AcroForm dictionary - equally legal - hits it. The new fixture uses one deliberately; reverting the one-line fix fails 9 of the 12 new acceptance tests. 2. Nothing read signatures. FieldType::Signature was classified and then ignored; /ByteRange, /Contents, /SubFilter and /DocMDP appear nowhere in the codebase. A signed contract was presented exactly like an unsigned one. New pdf-document/src/signature.rs reads the signature dictionary and checks BYTE-RANGE INTEGRITY, which needs no cryptography and catches the common real-world tampering: whether the signed range reaches the end of the file. A signature that stops short leaves appended bytes uncovered, which is exactly how an incremental-update attack hides content behind a signature that still verifies. Cryptographic verification is NOT implemented and cannot be faked: VerificationStatus has no Valid variant. That is enforced by the type, not by convention, because the failure mode for a signature feature is not "it doesn't work" - it is a green tick beside a document nobody checked. A test asserts the capability's absence so adding Valid without the cryptography breaks the build rather than shipping a false tick. Signing is out of scope entirely: no private keys in this crate. Also verified ADR 0003's claim that an append-only save preserves a signed byte range, byte for byte, rather than leaving it asserted. Implementation bug worth recording: /Contents was initially hex-decoded, but the COS lexer already decodes <...>. Running it twice on the common 128-zero-byte placeholder - which contains no hex digits - produced an EMPTY vector, silently discarding the signature blob while every other field looked right. Caught by asserting the blob is non-empty rather than asserting the parse returned Ok. 6 corpus fixtures, 12 acceptance tests, 28 unit tests, and a parse_signature fuzz target because /ByteRange is four attacker-controlled integers used to index the file. TEST_TARGET=pdf 497 -> 536 passing. rustfmt and clippy -D warnings clean. |
|||
| d8d29c226d |
feat(pdf): transparency, and four operator-parsing bugs it exposed
Phase 8 #5 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Advanced transparency"). Design and merge criteria in REVIEWS/adr/0009-pdf-transparency.md. The headline defect was not that transparency was missing. `gs` was MIS-PARSED as CloseStroke: /GS0 gs 1 0 0 rg 100 100 200 200 re f -> [CloseStroke, RgbFill, Rectangle, FillWinding] so every page setting a graphics state gained a stroked path the document never asked for, and lost its alpha, blend mode and soft mask silently. Downstream everything was dead: StrokeExtGState/FillExtGState were never constructed, set_fill_opacity was never called and emitted no command when it was, and PdfPage::ext_gstate was read by no code at all. New pdf-graphics/src/transparency.rs: full /ExtGState (ca, CA, BM, SMask, LW, LC, LJ, ML, D, AIS, TK), all sixteen blend modes including the four non-separable ones, soft masks with /S, /G, /BC and a /TR evaluated through ADR 0006's PdfFunction, and /Group parsing. Wired end to end: parser -> PdfOp::SetExtGState -> device -> RenderCommand -> Makepad renderer. Compositing is NOT claimed. Backdrop blending needs render-to-texture, which an engine-neutral crate has no framebuffer for. Constant alpha is applied because it needs no backdrop; blend modes and soft masks are reported through TransparencyError::Unsupported rather than dropped, because a silently ignored /Multiply looks exactly like a correct /Normal. The ADR required a test enumerating every multi-character operator, on the grounds that fixing the third instance of a shadowing bug (after cm and rg) without preventing the fourth is not a fix. It immediately found three more live bugs, none of them transparency-related: - `b` and `b*` dropped their close-path, mapping to FillStroke* instead of CloseFillStroke*, so every closed-and-stroked path drew with a gap. - Text operators within three bytes of the end of a content stream were mis-parsed: the b'T' arm guarded `*i + 3 < len` while reading only two bytes, so a stream ending in `/F1 12 Tf` parsed as FillWinding. And the transparency-group fixture found a fourth: - PdfPage::xobjects was empty for every page of every document. extract_xobjects called doc.resolve(), which follows the reference, then asked the resolved object for as_ref() - always None - and only accepted a bare dict when every XObject is a stream. No `Do` operator could be resolved through the page model. Same defect as the one that once destroyed annotation object references; it now has its own regression test. T* and BMC are genuinely unimplemented and are deliberately excluded from the guard's table rather than papered over. 7 corpus fixtures, 12 acceptance tests, 32 unit tests asserting the §11.3.5 formulas (not our own output), and a parse_ext_gstate fuzz target. TEST_TARGET=pdf 451 -> 497, TEST_TARGET=pdf-ui 495 -> 541. rustfmt and clippy -D warnings clean. |
|||
| 00f1dfbc12 |
fix(pdf): fuzz every target, and close two ADR 0004 gaps
Three defects found by auditing the ADR merge criteria against the code rather than against memory. 1. The scheduled fuzz job ran five hardcoded targets. Four had been added since and were never fuzzed: parse_revision_chain, decrypt, eval_function and parse_colorspace. eval_function is the sharpest of those - it executes PostScript taken verbatim from an untrusted file. The list now comes from `cargo fuzz list` and the job fails rather than passing vacuously if it comes back empty. `cargo fuzz list` reads the manifest, so a target file added without its [[bin]] entry would still be skipped silently. The engine job, which runs on every push, now checks the two agree. Both directions of that guard were exercised before committing. 2. ADR 0004 rule 3 promises an annotation whose appearance cannot be generated "keeps its original /AP and is reported as skipped". The keeping worked - to_dict clones the source dictionary - but nothing reported it: SaveReport only tracked skipped appearances for form fields. A caller who moved a stamp was never told its artwork still showed the old position. Adds SaveReport::annotation_appearances_skipped and appearance_is_generated, which enumerates the out-of-scope types explicitly so a new AnnotationType fails to compile until classified. 3. SetContents and SetFlags had no round-trip test. Both were implemented and unit-tested against the in-memory model, but neither was ever reparsed from written bytes - the assertion ADR 0004 calls central. New fixture annotations/stamp.pdf carries real /AP artwork for a Stamp (undrawable: must be preserved and reported) beside a Square (drawable: must not be reported), so the reporting cannot pass by reporting everything. The stamp test was mutation-checked: it fails when the reporting line is removed. ADR 0003 and 0004 merge criteria are now ticked. 0003's were genuine paperwork - every box traced to an existing named test. 0004's were not, and its ADR now records what was missing rather than implying it always worked. TEST_TARGET=pdf 447 -> 451 passing. rustfmt and clippy -D warnings clean. |
|||
| 7d21532ebf |
feat(pdf): real colour spaces, ICC profiles and PDF functions
Phase 8 #4 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Advanced colour"). Design and merge criteria in REVIEWS/adr/0006-pdf-advanced-color.md. The interpreter tracked only the *name* of the active colour space and then passed sc/scn operands to the device as if they were already RGBA. Every non-device space therefore rendered a confident wrong colour with no error: /Spot cs 1.0 scn full tint of a spot ink -> pure red /Idx cs 3 scn palette entry 3 -> near-black /Lab cs 50 0 0 scn mid gray -> white (clamped) /DevN cs (5 inks) five colorants -> inks 5+ discarded /ICCBased profile-defined colour -> profile discarded DeviceCMYK also used the additive 1-c-k conversion, which crushes any colour printed over black. Three new modules in pdf-graphics: - function.rs PDF functions, all four types. Type 4 runs on a bounded interpreter: depth 32, 32768 tokens, stack 100, 100000 steps, and an unknown operator is an error rather than a no-op that would leave a plausible wrong colour. - icc.rs ICC matrix/TRC and gray kTRC profiles, applied exactly. LUT-class profiles are reported as such and the caller falls back to /Alternate; they are never pretended to be matrix profiles. - colorspace.rs All eleven families, converting through XYZ with Bradford adaptation and a real sRGB transfer function. Wiring: - PdfDevice gains set_stroke_components/set_fill_components, so SC/SCN reach the device as components of the active space instead of being read positionally as RGBA. - cs/CS now resets to the space's initial colour (table 74), which is why golden/colors.txt gains a line. - PdfPage::color_spaces carries /Resources /ColorSpace fully dereferenced with streams decoded; a half-resolved space would make every ICC profile, palette and type 0/4 transform silently fall back. - A space that cannot be resolved keeps the previous colour and records a typed ColorError. No colour is invented, and no error is swallowed. Tests: 12 corpus fixtures under tests/corpus/color/, 14 acceptance tests in pdf-document/tests/color.rs asserting numeric RGB (the broken code produced a colour for every one of these; only the value was wrong), plus unit tests per function type and per curve type. Two fuzz targets added: eval_function and parse_colorspace. TEST_TARGET=pdf 387 -> 447 passing, TEST_TARGET=pdf-ui 431 -> 491. rustfmt and clippy -D warnings clean. |
|||
| 147ca7de23 |
test(pdf): prove the AES-256 path and harden malformed encryption
Closes the gaps in ADR 0005's own merge criteria. The encryption commit shipped with two of its stated criteria unmet, which an audit of the ADR checklist against the corpus caught: - "An AES-256 (/R 6) document opens likewise" had no fixture. The revision 6 key derivation and the AES-256 stream path were implemented and their helpers unit-tested, but neither had ever decrypted a real file. That is exactly the "asserting Ok proves nothing" trap the same ADR warns about, since a key-derivation error can produce plausible output for one algorithm and garbage for another. - "Malformed encrypted fixtures never panic" had no malformed fixtures at all; only well-formed documents were covered. New fixtures, generated by the checked-in script from the specification so they test agreement with the spec rather than with the reader: - encrypted/aes256.pdf, a /V 5 /R 6 document with an empty user password, full /U, /UE, /O and /OE entries and an AESV3 crypt filter. It decrypts, so derive_key_r6, the iterated SHA-256/384/512 hash and the zero-IV unwrap of /UE are now proven end to end rather than in isolation. - encrypted/truncated_u.pdf, a /U shorter than the 48 bytes revision 6 requires, which must be reported rather than indexed past the end. - encrypted/missing_o.pdf, an /Encrypt dictionary with no /O. - encrypted/absurd_length.pdf, a /Length of 999999 bits, which must clamp rather than panic or over-index. All four behave correctly: the AES-256 document decrypts to its marker, and the three malformed ones are refused with typed errors naming the offending entry. Encryption tests go from 13 to 17. The ADR's merge criteria are now ticked, with a note recording that the original commit shipped with the revision 6 path unproven and that this follow-up is what closed it. Not done here: the decrypt fuzz target could not be re-run. The nightly toolchain now available (2026-07-27) fails to build the cc crate that libfuzzer depends on, with errors inside cc itself rather than in this code. The target still compiles under stable and the earlier run of 1,953,940 executions stands; this is recorded rather than quietly skipped. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (387 tests) Both rustfmt and clippy -D warnings clean. |
|||
| 01e16b6383 |
feat(pdf): implement encryption (Phase 8)
Phase 8 feature 3 of 10, designed in REVIEWS/adr/0005-pdf-encryption.md.
An encrypted PDF did something worse than fail: it succeeded. Probing a
structurally valid RC4-encrypted file through the parser gave
parsed OK: pages=1
page 0 content bytes=44
content parsed into 0 ops
No error and no warning. The document reported a page, the page reported
content, and the content interpreted to nothing because it was ciphertext.
The user saw a blank page and was told the file was fine. That is the defect
class Phase 0 existed to remove, and it was the worst one left in the PDF
stack because it was silent.
New pdf-cos/src/encrypt.rs implements the standard security handler for
reading:
- V1/R2 RC4 40-bit, V2/R3 RC4 40 to 128-bit, V4/R4 crypt filters selecting
RC4 or AES-128, and V5/R6 AES-256 with the SHA-256 based revision 6 hash.
- The empty user password, which is the common case for a document
encrypted only to set permissions, and explicit user or owner passwords.
The owner path recovers the user password from /O and re-derives.
- Per-object keys, as the spec requires. Reusing one keystream across
objects would be a real cryptographic break, so the object and generation
numbers are mixed in by construction and a test asserts the keys differ.
Every primitive comes from audited RustCrypto crates: aes, cbc, rc4, md-5
and sha2, all MIT OR Apache-2.0, which deny.toml already permits. Phase 0
deleted a hand-rolled MD5/SHA/AES/RC4 implementation from this codebase and
called it a CVE factory; ADR 0005 keeps that rule.
Refusals rather than half-open documents: a public-key or otherwise
unsupported handler is refused and named, an unsupported V/R combination is
refused, and a wrong password returns a distinct error so a caller can
prompt again rather than reporting a damaged file.
Permissions are parsed and exposed but deliberately not enforced, and the
code says why: once content is decrypted a caller can read it regardless, so
enforcing here would imply a guarantee that does not exist.
Saving an encrypted document stays refused, as ADR 0003 established.
Decrypting and then writing plaintext would silently strip the protection
the author applied, which is not a decision a library should make.
Fixtures: tests/corpus/encrypted/ gains RC4 40-bit, RC4 128-bit, AES-128 and
an unsupported-handler document. The generator implements the handler's
algorithms independently from the specification, so a fixture that decrypts
shows the reader agrees with the spec rather than merely with itself. Each
plaintext contains a marker the tests assert on, and one test additionally
asserts the decrypted content interprets to real render commands, because
asserting Ok from the parser is exactly what the old broken behaviour did.
Fuzzing: adds a decrypt target covering key derivation, which consumes
attacker-controlled /O, /U, /P, /Length, filter names and file id. Run for
real rather than compile-checked: 1,953,940 executions, no crashes.
Validation:
TEST_TARGET=pdf ./tools/test-rust-clean.sh (383 tests)
TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (427 tests, 6 ignored)
Both rustfmt and clippy -D warnings clean.
|
|||
| 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. |
|||
| 1c7dc9e9c0 |
feat(pdf): complete Phase 7 performance work
Phase 7 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md, taken only now because the review is explicit that it comes after correctness is proven. The caching and threading live in pdf-graphics rather than the Makepad crate because none of it needs a GPU. That is what lets the staleness, eviction and cancellation rules be tested without a window; the widget keeps only the parts that genuinely need a Cx. Step 7.1, off-thread parsing (new pdf-graphics/src/worker.rs): - RenderWorker interprets content streams on a background thread and returns results tagged with the Generation they were requested for. - PendingPages tracks in-flight pages so the widget can draw a placeholder and never queues the same page twice. - Drop joins the thread rather than detaching it: a detached thread writing into a dropped channel is the kind of shutdown race that surfaces as a flaky test months later. - A content stream that fails to parse yields an empty page, so one broken page cannot take down the document. Step 7.2, texture and memory management (new pdf-graphics/src/cache.rs): - PageCache is an LRU keyed by page index with a byte budget, not an entry count: one image-heavy page can outweigh fifty text pages, so counting entries would evict the wrong things. - retain_around() releases pages that scrolled out of view, keeping a margin so a small scroll does not immediately re-render. - Decoding produces DecodedImage bytes off-thread; GPU upload stays on the UI thread. - A page larger than the whole budget is still stored, since refusing it would mean re-rendering it every frame. Step 7.3, render command cache: - CachedPage holds the interpreted Vec<RenderCommand>, so replaying a page skips re-parsing its content stream. - invalidate_appearance() marks a form edit dirty without discarding the commands, because a form edit changes what is drawn over the page, not the page content stream. Widget wiring: PdfPageWidget carries a generation, refuses PageContent from a superseded document, and draws a placeholder while a page is still rendering. Exit criterion (new pdf-document/tests/phase7_exit_criterion.rs, 9 tests) against a new 60-page corpus fixture. Measured here: first page 3ms against the 200ms budget, and a warm cache read 389x faster than re-parsing (1us vs 389us). The budget is deliberately loose because a shared CI runner is unpredictable and a flaky performance test gets muted, and a muted test is worse than none; it still catches the order-of-magnitude regression the review is guarding against. Memory is asserted bounded across 30 document switches, and every page of an abandoned document is refused. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (270 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (307 tests) Both rustfmt and clippy -D warnings clean. |
|||
| b23df6a5cb |
feat(pdf): complete Phase 6 testing infrastructure
Phase 6 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md: "Real PDFs. Real regressions. No test theater." Step 6.1, corpus. 27 fixtures across basic, fonts, forms, annotations, images, edge and malformed, in the layout the review specifies. They are produced by tests/corpus/generate.py rather than committed as opaque blobs, because a corpus you cannot read is a corpus you cannot trust; CI regenerates them and fails if they differ. Hand-rolled rather than library-produced, since fixtures for a parser must contain constructs a library refuses to emit. Step 6.2, corpus tests (pdf-document/tests/corpus.rs, 27 tests). Text, vectors, Flate, multipage, CID fonts, every form field type, inherited field keys, link actions, hidden annotations, XObjects, inline images with embedded EI bytes, rotation, crop boxes, nested CTMs and content arrays. Step 6.3, robustness (pdf-document/tests/robustness.rs, 3 tests) plus five cargo-fuzz targets. cargo-fuzz needs nightly and libFuzzer so it cannot gate a stable CI run; the harness covers the same ground deterministically by mutating the real corpus with a fixed-seed PRNG, so a failure is reproducible from the seed rather than only from a saved artefact. The fuzz targets remain the deeper coverage-guided search and run on a schedule. Three crashes on untrusted input, all found by this work and all previously reachable from a malformed file: - collect_pages_ref recursed forever on a /Kids cycle. Stack overflow aborts the process; it cannot be caught. Now tracks visited nodes and bounds depth. - PdfDocument::resolve and the COS lexer recursed once per nesting level, so a file of 5000 open brackets overflowed the stack. Both are now bounded. - decode_85_group multiplied an accumulator that a malformed group can overflow, and subtracted below zero on a digit outside the valid range. Both panic in a debug build. Now saturating. Step 6.4, CI (.forgejo/workflows/pdf.yml). An engine job that runs the corpus and robustness suites under rustfmt and clippy -D warnings; a separate makepad-integration job so a missing system library is not reported as a PDF regression; and a scheduled fuzz job. The engine job also enforces the two architectural rules mechanically rather than in prose: no Makepad dependency or import in the engine crates, and no process or URL launching anywhere in them. Also fixes .gitignore: the blanket *.pdf rule silently excluded all 27 fixtures, which would have left CI unable to run them on a fresh clone. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (233 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (268 tests) Both rustfmt and clippy -D warnings clean; all five fuzz targets compile. |