Skip to content

gamut-ifd: Ifd::set_sub_ifd documents an invariant nothing enforces, and a violation is undetectable downstream #601

Description

@justin13888

The invariant

Ifd::set_sub_ifd (crates/gamut-ifd/src/entry.rs) says:

The pointer field (a LONG/LONG8 array of the children's offsets) is synthesised by the
writer, so tag must not also be set as a regular field.

Nothing checks it. set and set_sub_ifd write to two independent stores, so

ifd.set_sub_ifd(tags::EXIF_IFD, vec![child]);
ifd.set(tags::EXIF_IFD, Value::Long(vec![8]));   // accepted

leaves a directory that violates the documented contract, and write then resolves the conflict
silently in whichever direction its emit order happens to pick.

Why it matters beyond the setter

The half a consumer can see is worse than the half it cannot. A reader decides "this tag is a
pointer" from a directory's fields (ifd.get(tag) with a LONG/IFD/LONG8/IFD8 value),
because that is all a file carries; a caller building a directory in memory expresses the same
thing as a group. A crate that validates only the groups it was handed is therefore blind to
exactly the shape a reader misreads — which is the defect PR #520 repaired in gamut-tiff
(TiffMetadata::check inspected sub_ifds() while resolve_pointers inspects get(tag), so a
standard pointer tag carried as a plain LONG encoded cleanly and then failed the crate's own
reader with read out of bounds or sub-IFD pointer loop).

gamut-tiff now guards its own subtree, and gamut-exif uses the group setter correctly, so no
in-workspace path is currently broken. But the guard is per-consumer, and the invariant belongs to
the type that documents it: every present and future consumer of gamut-ifd has to rediscover it,
and a hand-built or third-party-parsed directory reaches all of them.

Options

  1. Make it unrepresentable. set refuses (or debug_asserts against) a tag that already has a
    group, and set_sub_ifd removes any field under that tag — the cheapest fix, and it makes the
    doc sentence true by construction. set is currently infallible, so refusing means either a
    fallible variant or silent precedence; removing on set_sub_ifd and documenting the precedence
    is the smaller change.
  2. Expose the check. A pub fn — or an entry in the existing invariants module — that
    answers "does this directory carry a pointer tag as both a field and a group", so every consumer
    validates identically rather than each writing its own.
  3. Validate in write. Turn the conflict into a typed error at the one place the ambiguity
    actually has to be resolved. Catches every consumer, but only at the last moment and only for
    directories that get written.

(2) composes best with how gamut-tiff already validates its Exif subtree; (1) is the one that
makes the class impossible.

Where

crates/gamut-ifd/src/entry.rsIfd::set, Ifd::set_sub_ifd; crates/gamut-ifd/src/writer.rs.

Found while reviewing #446 / PR #520; gamut-ifd is frozen base territory for that lane, so the
repair was made in the consumer and the question is raised here.

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