Skip to content

gamut-dng: an interior main-chain page with no image data reaches no decode surface #525

Description

@justin13888

Found while closing a review finding on #442 (PR #508). That PR fixed the last main-chain
directory; this issue is the residue, which predates it.

The gap

DngDecoder::decode surfaces every unmodelled IFD field through four verbatim channels:

  • DecodedDng::ifd0_extra — IFD 0,
  • DecodedDng::raw_extra — the raw sub-IFD,
  • SubImage::extra_tags — each directory that decodes as an image,
  • DecodedDng::trailing_extra — the last main-chain directory when it is none of those
    (added in feat(dng)!: type the C2PA manifest store and report both exclusion ranges #508, because C2PA 2.4 §A.3.6 lets a manifest store be "the only entity within a new
    IFD following the existing one", which is a directory with no image).

A main-chain page that is neither the first nor the last and carries no image data falls
through all four: decode_sub_image returns None for it, so no SubImage is created and no
extra_tags are attached, and it is not IFD 0, the raw IFD, or the last page. Its fields reach
no surface.

That contradicts the decoder's stated promise, on ifd0_extra: "Nothing those four reach is
silently dropped" — the qualifier is honest, but the underlying intent (issue #109's "all
metadata explicitly represented") is not met for this shape.

Byte accounting is unaffected: deconstruct walks the whole chain and still classifies those
bytes, so the file is fully accounted; it is the typed decode surface that drops them.

Reproducing

Encode any DNG, then append two trailing main-chain IFDs carrying only non-image tags (the
helper append_raw_trailing_ifd in crates/gamut-dng/tests/c2pa.rs builds one at the byte
level). The last page's fields arrive in trailing_extra; the middle page's arrive nowhere.

Options

  1. Generalise trailing_extra to every uncovered main-chain page. That means keeping the set
    of main-chain indices from walk_ifds (it currently returns only the last one) and reporting
    per-page rather than as one flat list, since a caller cannot otherwise tell which page a tag
    came from. Renaming the field would be semver-major for gamut-dng.
  2. Add a separate per-page channel and leave trailing_extra as the §A.3.6 case it was
    introduced for.

Option 1 is the honest shape; the naming is what makes it more than a one-liner. Either way the
promise on ifd0_extra and the note in gamut-dng/STATUS.md should be updated together with the
fix.

Related

#442, PR #508, epic #109's explicitness clause.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions