feat(dng)!: type the C2PA manifest store and report both exclusion ranges - #508
Open
justin13888 wants to merge 9 commits into
Open
justin13888 wants to merge 9 commits into
justin13888 wants to merge 9 commits into
Conversation
C2PA 2.4 §A.3.6 embeds the manifest store in a TIFF-based file as tag 52545 (0xCD41), type UNDEFINED, with an unusual placement rule: one store per asset, its entry in the last IFD of the main chain, and its bytes at the end of the file so a resize moves no other offset. §18.5.5 then asks a signer to exclude two disjoint ranges from its hard binding — the store and the entry's count field. Both gamut-dng (#442) and gamut-tiff (#446) need exactly this, so the new `c2pa` module states it once: - `C2PA_MANIFEST_STORE` and `C2paExclusions { store, count_field }`, two ranges in the crate's own `Range`, never one. - `reserve_entry` puts a one-byte inline placeholder in the directory a codec writes as its last main IFD, so the layout is final while the value pool stays untouched; `append_store` then lands the store at the word-aligned end of the finished file and patches only the entry's count and offset words. A post-write relocation rather than a writer mode, because a codec's pixel data is appended after the writer's stream and "end of the stream" is not "end of the file". - `locate` walks any `ReadAt` source to the chain's last directory and reports both ranges for an out-of-line or inline store; a mistyped or misplaced entry is absence, a store past the end of the file is the same `InvalidInput` `read` gives it. The store is opaque bytes copied verbatim — §A.3.6 says the header's ByteOrder does not govern it — pinned on big-endian fixtures with an asymmetric store. A store shorter than a JUMBF box header (8 bytes) is refused. An audited read of the result is fully classified: the store is the entry's value span and the alignment filler is padding, never a trailer. Refs #442
…nges
The C2PA manifest store (C2PA 2.4 §A.3.6, tag 52545, type UNDEFINED) was
already visible as an untyped `RawTag`; it now has a name, a placement rule
and the exclusion ranges an external signer needs.
- `DngMetadata::c2pa: Option<Vec<u8>>` is the fifth carrier, verbatim bytes
on the same terms as XMP/IPTC/ICC, handed over by `blocks()` as
`MetadataBlock::C2pa`. Its entry goes in IFD 0 — the last and only IFD of
the main chain — and its value is appended after the image data, last in
the file, through `gamut_ifd::c2pa`'s reserve-then-append placement.
- `DngEncoder::with_c2pa_reserved(len)` writes a zero-filled reservation;
`encode_with_report` returns `DngEncodeReport { len, c2pa }` with the two
disjoint ranges §18.5.5 asks for — the store and the entry's count field
(4 bytes classic, 8 BigTIFF). A reservation and a same-sized store are
byte-identical outside the store, and a store of a different size changes
the count field and nothing else. `encode` delegates and is unchanged.
- `DecodedDng::c2pa_exclusions` carries the located ranges beside the bytes
in `metadata.c2pa`, read from the last main-chain IFD; a mistyped entry, or
one in IFD 0 when the chain continues, stays an `ifd0_extra`.
- The bytes cross verbatim in either byte order; the Adobe DNG SDK accepts
the result in little-endian, big-endian and BigTIFF; `deconstruct` claims
the store as IFD 0's value span with the file fully classified.
Carrying the tag raises neither DNGVersion nor DNGBackwardVersion: like XMP
and ICC it is metadata a reader may ignore.
BREAKING CHANGE: `DngMetadata` gains the `c2pa` field. The struct is
deliberately exhaustive, so every struct literal must add `c2pa: None`.
Closes #442
Three defects in the manifest-store module, all found reviewing the placement against the container's own inline rule. `MIN_STORE_LEN` (8) is a JUMBF box header, and the module claimed that bound also kept an appended store out of line "in both variants". It does not: BigTIFF's inline threshold is 8 too, and `value_offset` compares `<=`, so a `count: 8` UNDEFINED value is inline by the container's own rule. `append_store` appended the bytes anyway and wrote an offset into the value word, so the entry read back as the offset (`Undefined([72,0,0,...])`), the appended run was referenced by nothing, and `locate` reported the value word while the encoder reported the appended range -- exclusion ranges over bytes no reader returns. It now gates on the variant's own `inline_threshold()`, so the shortest writable BigTIFF store is nine bytes and the classic-TIFF case is unchanged. A directory carrying two tag-52545 entries named no single store, yet `store_entry` took the first while the eager `Ifd` keeps the last, so bytes and ranges could describe different runs under one name. §A.3.6 admits one store per asset, so more than one entry is now reported as absence. `locate` reported a value too short to hold a JUMBF box header as a store. `references/c2pa/README.md` already prescribes the split this needs: a reader treats such a value as not a manifest store, while a writer refuses it -- an encoder handed a store it cannot write must say so rather than drop it silently. Absence on the read side is also what makes decode -> encode of a foreign file carrying a stub value work at all. `C2paExclusions` becomes `#[non_exhaustive]`, matching `DngEncodeReport`: §18.5.5 names two ranges today and a third must be additive. Refs #442
`is_fully_accounted()` was true for every file this encoder writes until a manifest store was embedded, because 52545 was missing from `KNOWN_TAGS`: `deconstruct` then reported the file's own store as a private tag. The tag joins the list (aliasing `gamut_ifd::c2pa::C2PA_MANIFEST_STORE`, where the clause is stated), and the accounting test now uses the file's `assert_clean` helper like its neighbours instead of asserting a weaker subset -- which is what had hidden the gap. Two smaller corrections to the store's edges: - A BigTIFF store of exactly 8 bytes packs inline, so it cannot be the run at the end of the file §A.3.6 wants. It was reachable through the documented `with_big_tiff(true).with_c2pa_reserved(8)` and produced a file whose reported ranges covered bytes no reader reads back. The encoder now refuses it before any pixel work; nine bytes are the smallest BigTIFF store. - A foreign file whose tag-52545 value is shorter than a JUMBF box header decoded to `Some(short)`, which the encoder then refused -- decode -> encode of a real file was not round-trippable. Such a value is not a manifest store (`references/c2pa/README.md`), so it decodes as absent while the encoder keeps its hard error for a store a caller supplies. `C2PA_MANIFEST_STORE` and `MIN_STORE_LEN` join `C2paExclusions` on the crate root, completing the re-export closure the freeze decisions state: this crate's own docs name them, so a signer should not need a direct `gamut-ifd` dependency to use them. Refs #442
This was referenced Sep 9, 2026
…ion set `append_store` told a caller whose last IFD carries two tag-52545 entries that it "carries no reserved C2PA manifest store entry", because `store_entry` reports absence for a duplicate exactly as it does for a missing entry. The two cases are now distinguished, so the message says what is actually wrong. `C2paExclusions` gains a public `new`. `#[non_exhaustive]` alone left downstream code no way to build one at all, so a host placing a store by its own route -- its own writer, a format this crate does not serialise -- could never name the ranges 18.5.5 asks it to exclude. Keeping the attribute and adding the constructor gives the type both extensibility and constructibility. The read/write asymmetry around an inline BigTIFF store is now stated at both `locate` and `append_store`: `locate` reads that lawful shape, `append_store` refuses to write it, because an inline value is not the run at the end of the file the placement rule is built on and admitting it would give a store two placements to reason about. Liberal in, conservative out -- recorded so it reads as a decision rather than an oversight. Refs #442
…a page
Two surfaces described the same file differently. `metadata.c2pa` read the
store's bytes through the eager `Ifd`, where the LAST duplicate wins, while
`c2pa_exclusions` came from `c2pa::locate`, which reports absence when a
directory carries more than one tag-52545 entry. A file with two 40-byte
entries therefore returned bytes with no ranges -- contradicting
`c2pa_exclusions`' own documentation ("`Some` exactly when the store is") --
and re-encoding that metadata silently produced a one-entry file carrying
only the last duplicate's bytes.
The ranges are now located first and the bytes are taken only if that
succeeded, so a single rule decides both surfaces and they cannot drift
apart again.
Applying that rule exposed a second defect. A declined tag-52545 field is put
back for the extras, but extras were collected only from IFD 0 and the raw
IFD, and C2PA 2.4 A.3.6's other lawful placement -- the store as "the only
entity within a new IFD following the existing one" -- makes a directory with
no image, which is neither of those and becomes no `SubImage` either. Such a
field reached no surface at all, against this decoder's standing promise that
nothing in the file is silently dropped. `DecodedDng::trailing_extra` now
carries the last main-chain directory's fields when nothing else does; it is
empty for every file this crate writes. The remaining case -- an interior
page with no image data -- predates this work and is filed as #525, with the
promise on `ifd0_extra` reworded to say exactly what the four verbatim
channels reach.
`is_known_tag`'s meaning is documented where it could be misread: a tag this
crate recognises, answered from the tag number alone, not a tag some decode
path happened to consume.
Refs #442
The guard deciding whether the last main-chain directory needs its own verbatim channel was three `||`ed comparisons, and the mutation survey found two survivors in it: no test could tell `||` from `&&` there. The disjuncts never disagree on any file the suite builds -- a single-main-IFD DNG makes the first true, a trailing store directory makes all three false -- so the operators between them decided nothing. Stated instead as one membership test over the directories already surfaced, which removes the operators rather than adding a test to compensate for them, and pinned along the axis that was missing: a single-main-IFD file whose IFD 0 carries a field the decoder declined, where `ifd0_extra` must hold it and `trailing_extra` must stay empty. With the condition forced to `false` that test fails; with it forced to `true` the two trailing-directory tests fail. Both directions now die. Refs #442
`C2paExclusions` documents "they never overlap", but `new` was infallible and explicitly declined to check, so a caller could build a set of two empty or overlapping ranges and the type's stated invariant would simply be false. What that set feeds is a signer's hard binding, where a nonsensical exclusion must not pass silently. `new` now returns a `Result`, rejecting an empty range on either side and overlapping ranges, and the type doc states the invariant as an invariant. Abutting ranges stay legal -- touching is not overlapping -- which is the boundary the new comparisons turn on, so the test drives both orders of abutment as well as one-byte overlap each way and full containment. `locate` and `append_store` keep constructing the struct directly: they derive both ranges from a directory they just walked, so there is nothing for a validator to tell them. Refs #442
… made The claim that a declined tag-52545 field reaches the caller listed "duplicated" among the cases, but only the LAST of several entries does: the typed channels are built on the eager `Ifd`, which is last-wins, as the module those channels come from says itself. The claim is narrowed rather than the channel widened -- carrying both would mean changing a model every consumer of the IFD core shares, and a file with two stores is malformed under A.3.6 anyway. The test that pinned it built its two duplicates to be distinguishable and then asserted only that *a* tag-52545 field was present, so it passed on the first entry, the second, or both -- vacuous on exactly the ambiguity it existed to pin. It now asserts how many fields arrive and which bytes they carry. The crate front page and the `RawTag` doc still stated the absolute promise that the field docs had already qualified, and those two are where a reader meets it first. Both now carry the same qualification, naming the last-wins residue and the interior-page one (#525), and `trailing_extra` documents both in one place so the cross-references resolve. Also drops `append_raw_trailing_ifd`, which was character-identical to the existing `append_trailing_ifd` but for one assertion, and justified on a distinction that does not exist -- the existing helper is equally byte-level and its assertion holds on both new fixtures. Refs #442
This was referenced Sep 9, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Types the C2PA manifest store (C2PA 2.4 §A.3.6: tag 52545 /
0xCD41, typeUNDEFINED) on both sides ofgamut-dng, applies the specification's placement rule, and reports the two disjoint exclusion ranges an external signer needs (§18.5.5). The placement and exclusion rules live in a new sharedgamut_ifd::c2pamodule so #446 (plain TIFF) reuses them rather than re-deriving §A.3.6.gamut-ifd (
feat(ifd), minor):c2pa::C2PA_MANIFEST_STORE,C2paExclusions { store, count_field }— two ranges in the crate's ownRange, never one.reserve_entry/append_store: the entry is reserved inline in the last main IFD while the tree is laid out, and the store is appended at the word-aligned end of the finished file with only the entry's count/offset words patched. A post-write relocation rather than a writer mode, because a codec's pixel data is appended after the writer's stream and "end of stream" is not "end of file".locate: walks anyReadAtsource to the chain's last directory and reports both ranges (out-of-line or inline store). A mistyped or misplaced entry is absence; a store past EOF is the sameInvalidInputreadgives it.MMfixtures with an asymmetric store. Stores shorter than a JUMBF box header (8 bytes) are refused.gamut-dng (
feat(dng)!, major —DngMetadatais deliberately exhaustive):DngMetadata::c2pa: Option<Vec<u8>>, the fifth carrier;blocks()yieldsMetadataBlock::C2pa.DngEncoder::with_c2pa_reserved(len)writes a zero-filled reservation;encode_with_reportreturnsDngEncodeReport { len, c2pa: Option<C2paExclusions> }.encodedelegates and is unchanged;EncodeImageuntouched.DecodedDng::c2pa_exclusionscarries the located ranges besidemetadata.c2pa; a mistyped tag, or one in IFD 0 when the chain continues, stays inifd0_extra.deconstructclaims the store as IFD 0'sValue { tag: 52545 }span with the file fully classified — never aTrailer.Not done: parsing the store; touching
gamut-tiff; changing the facade'sC2paPolicy; adding a dependency.Review repairs (commits 3–4)
A read-only review of the first two commits found two Medium defects and three Lows, all fixed here:
value_offsetcompares<=, sowith_big_tiff(true).with_c2pa_reserved(8)— valid per the docs — appended the bytes at EOF while writing an offset into the value word: the entry read back as the offset, the appended run was referenced by nothing, and the reported ranges covered bytes no reader returns. Both layers now gate on the variant's owninline_threshold(); nine bytes is the smallest BigTIFF store, classic TIFF is unchanged. The two docs asserting the false claim are corrected.is_fully_accounted()flipped false the moment a store was embedded, because 52545 was missing fromgamut-dng'sKNOWN_TAGS—deconstructreported the file's own manifest store as a private tag. Fixed with the tag-table entry (aliasinggamut_ifd::c2pa::C2PA_MANIFEST_STORE), and the accounting test now uses the file'sassert_cleanhelper like its neighbours rather than the weaker subset that had hidden the gap.Ifdkeeps the last duplicate,store_entrytook the first). §A.3.6 admits one store per asset, so more than one entry is now absence.Some(short)which the encoder then refused. Applying the rulereferences/c2pa/README.mdalready states, a value below the JUMBF header is not a store: absent on read, still a hard error on write.C2PA_MANIFEST_STOREandMIN_STORE_LENjoinC2paExclusionson thegamut-dngroot;C2paExclusionsis now#[non_exhaustive], matchingDngEncodeReport.Second review round (commits 5–7)
gamut-ifdbut not at the DNG surface.metadata.c2paread bytes through the eagerIfd(last duplicate wins) whilec2pa_exclusionscame fromlocate(absence on duplicates), so a file with two tag-52545 entries returned bytes with no ranges — contradictingc2pa_exclusions' own doc — and re-encoding silently emitted a one-entry file carrying the last duplicate. The ranges are now located first and the bytes taken only if that succeeded, so one rule decides both surfaces and they cannot drift apart again.append_store's message for a duplicated entry no longer claims the entry is missing.SubImageeither. Gap closed, not just documented:DecodedDng::trailing_extracarries that directory's fields (empty for every file this crate writes). The narrower residue that predates this work — an interior main-chain page with no image data — is filed as gamut-dng: an interior main-chain page with no image data reaches no decode surface #525, and the promise onifd0_extranow says exactly what the four verbatim channels reach.||ed comparisons that no test could tell from&&: the disjuncts never disagree on any file the suite builds. Restated as one membership test over the directories already surfaced — removing the operators rather than adding a test to compensate — and pinned along the missing axis (a single-main-IFD file whose IFD 0 carries a declined field:ifd0_extraholds it,trailing_extrastays empty). Verified by hand in both directions before re-running the gate.Third review round (commits 8–9)
Four Lows, all closed.
C2paExclusions::newnow validates — it returned an unchecked set while the type doc promised "they never overlap", and that set feeds a signer's hard binding; empty and overlapping ranges are refused, abutting ones accepted. The preservation claim is narrowed to what is true: a duplicated tag keeps only the last entry (the eagerIfdis last-wins), soSTATUS.mdsays that instead of listing "duplicated" among fields that survive verbatim — and the test that pinned it, which asserted only that some tag-52545 field existed and so passed on either duplicate, now asserts the count and the exact bytes. The absolute promise is qualified where a reader meets it first — the crate front page and theRawTagdoc, not only the field docs — naming both residues (last-wins duplicates, and the interior-page gap #525). A duplicated test helper is deleted in favour of the identical pre-existing one.No human approved this plan. This is an unattended run; the decision record below is what a human reads afterwards.
Validation
Run in the lane worktree (a nested worktree, hence the
__CARGO_TEST_ROOTprefix on the fmt tasks — an environment artefact, not a manifest change).CARGO_BUILD_JOBS=2on crate-scoped runs; workspace gates inside asystemd-run --scope -p MemoryMax=16Gwithulimit -v 12000000.All rows below are the run at head
677d38eunless noted.cargo test -p gamut-ifd -p gamut-dng --all-featuresgamut-ifdlib 161 (18c2pa) plusfidelity4 /hardening_audit16 /robustness8 /streaming5;gamut-dnglib 188,tests/c2pa.rs15,deconstruct21,roundtrip27,oracle_adobe18,oracle_libtiff1,adobe_samples6,corpus2,color_profile6,real_world_shapes10,rewrite4,subimages2, doctests__CARGO_TEST_ROOT=<worktree> mise run fmtthen… mise run fmt-checkmise run check-testsmise run check-commitscargo clippy -p gamut-ifd -p gamut-dng --all-targets --all-features -- -D warningsdoc_lazy_continuationerrors in a new doc comment; reworded, then clean)mise run lint(workspace, capped scope)a645321; later commits are covered by the crate-scoped clippy above and by CI's green Clippy job at6ea9565)mise run test(workspace, capped scope)6ea9565; not re-run for round 3:C2paExclusionsis named nowhere outsidegamut-ifd/gamut-dngand its only::newcall site is one gamut-ifd test, so the signature change cannot reach another crate)mise run mutants-diff(the selection CI blocks on, capped scope)C2paExclusions::new. (Earlier rounds: 66/57 ata645321, 75/66 at0b5b735, 78/69 at6ea9565, all 0 survivors; the round-2 run at6e9fb63reported 2 survivors in a new guard, fixed structurally in6ea9565.)RUSTDOCFLAGS=-D warnings cargo doc -p gamut-ifd -p gamut-dng --no-deps --all-featuresgamut-ifdaudit.rs/segment.rs/stream.rs,gamut-dngdecoder.rs:152,lib.rs:56) — pre-existing; the repo'slinttask is clippy-only and does not gate rustdoc. Every link added here resolves.CI on
677d38e: observed to terminal after the final push; all seven required checks green at the preceding head6ea9565as well.No
Cargo.tomlchanged, socheck-release-deps/check-ffi-featureswere not required; no C-surface type changed.Risks and rollout
gamut-dng: everyDngMetadata { … }struct literal must addc2pa: None. The one in-tree literal (tests/roundtrip.rs) is updated; release-plz cuts the major from the!commit.DngDecoder::decodenow also runsgamut_ifd::c2pa::locate(a directory-body walk, no value fetch); it cannot newly fail on a filereadaccepts.DngRewriteof a file carrying a store relocates it into the value pool like any other value; a rewrite invalidates the binding regardless. Unchanged behaviour, now documented.DNGVersion/DNGBackwardVersion(metadata a reader may ignore, like XMP/ICC; the SDK validates the file).Issue
Closes #442
Closes #513
#513 was filed by this lane when
tags.rssat outside its manifest; the manifest was then extended on review and the one-line tag-table entry is included here, so the issue closes with this PR rather than outliving it.Decisions taken
Appended by the lane (forks the record did not cover):
Decided by the orchestrator on review of a645321, and applied in commits 3–4:
Decided by the orchestrator on re-review of 0b5b735, and applied in commits 5–7:
Decided by the orchestrator on the third review of 6ea9565, and applied in commits 8–9:
Unresolved review notes
Accepted residual — the 8-byte JUMBF bound is stated twice.
gamut_heic::c2pa::JUMBF_HEADER_LENandgamut_ifd::c2pa::MIN_STORE_LENboth encode the same 8-byte JUMBF box-header bound from the same clause (C2PA 2.4 §8.4.2.3, recorded inreferences/c2pa/README.md). The dependency graph gives them no shared home —gamut-ifdsits belowgamut-heicand neither may depend on the other — and factoring it out would mean a new crate for one integer. Accepted on review, recorded ingamut-ifd/STATUS.md; nothing filed.Filed, not fixed here. #525 — an interior main-chain page carrying no image data reaches no typed decode surface. It predates this PR (the last page is what §A.3.6's placement creates, and
trailing_extracovers that);deconstructstill accounts for its bytes. The issue records why the honest fix is per-page and therefore more than a one-liner.Not re-reviewed. Commits 5–7 (the second round of repairs) have not themselves been through a review pass.