Skip to content

gamut-tiff: an ExifIFD pointer array naming two directories silently loses the second #599

Description

@justin13888

What happens

TiffDecoder::metadata reduces the ExifIFD group to its first child and drops the rest:

let exif = ifd0.sub_ifds().iter()
    .find(|group| group.tag == tags::EXIF_IFD)
    .and_then(|group| group.ifds.first())   // <- the rest of the array is discarded
    .cloned();

and TiffMetadata::apply writes exactly one back (ifd0.set_sub_ifd(tags::EXIF_IFD, vec![exif.clone()])).
So a file whose ExifIFD field is a LONG array of two or more offsets round-trips into a file
with one, and nothing — not an error, not a deconstruct anomaly, not a doc sentence — says a
directory was dropped.

Reproduction

A one-page file built with ifd0.set_sub_ifd(tags::EXIF_IFD, vec![a, b]), where a and b carry
different ExposureTime values, then read and re-encoded through the public seam:

written ExifIFD field    = Some(Long([26, 44]))
read back exif           = Some(Some(Rational([(1, 250)])))   // only `a`
re-encoded ExifIFD field = Some(Long([170]))                  // one offset, `b` gone

The same shape applies to the other three pointer tags inside the Exif subtree
(SubIFDs, GPSInfo, InteroperabilityIFD): resolve_pointers does keep every child of those
arrays as a group, so those survive — it is only the ExifIFD group at IFD 0, the one the seam
narrows to a single Ifd field on TiffMetadata, that loses them.

Why it is not obviously wrong

TiffMetadata::exif is a single Option<Ifd> because EXIF 2.3 defines exactly one Exif private
directory per image, so a conformant file never has more than one offset here. But the reader
cannot tell a caller's hand-built directory or a third-party file from a camera's, and the crate's
stated contract is that what a file holds is what the caller gets, or a typed error saying it is
not — silent narrowing is neither.

Options

  1. Refuse an ExifIFD array of length > 1 as Error::InvalidInput, matching how the crate
    already refuses the other shapes it cannot hand back unchanged (TiffMetadata::check).
  2. Widen TiffMetadata::exif to carry every directory of the array. A public API change on a
    1.x crate, for input no conformant file produces.
  3. Document the narrowing on TiffMetadata::exif and TiffDecoder::metadata and leave the
    behaviour. Cheapest, and consistent with "the reader reports what it can use", but it keeps a
    silent loss.

(1) or (3) is the real choice; the decision belongs with whoever owns the seam's contract, not with
the pull request that found it.

Where

crates/gamut-tiff/src/metadata.rsread_metadata and TiffMetadata::apply.

Found while reviewing #446 / PR #520.

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