Skip to content

gamut-exif: to_bytes re-emits a thumbnail pointer whose bytes the report says were dropped #548

Description

@justin13888

What happens

ExifReader::read_thumbnail removes the JPEGInterchangeFormat (0x0201) tag from the stored 1st
IFD only when bytes were actually read:

let mut ifd = ifd;
if jpeg.is_some() {
    ifd.remove(ptr);
}

When the range was out of bounds — the case leniency exists for — jpeg is None, so the pointer
survives into the model and Exif::to_bytes writes it back out verbatim. The emitted blob then
claims a thumbnail at an offset that addresses nothing, while the ReadReport for the same parse
says the bytes were dropped.

Reproduction (executed)

A 1st IFD with JPEGInterchangeFormat = 0xFFFF and JPEGInterchangeFormatLength = 16, parsed
leniently and re-serialised:

report:            [Dropped { region: ThumbnailJpeg, tag: Some(513), offset: 65535, reason: OutOfBounds }]
pointer in model:  Some(65535)
re-parsed from to_bytes(): pointer still Some(65535), report still names the same drop

So the round trip preserves a dangling pointer rather than either dropping it or repairing it, and
a consumer of the emitted blob has no report to tell it the pointer is a lie.

Why it is filed rather than fixed

The behaviour is pre-existing: the same conditional removal is on master, byte for byte, and
predates the read report. Repairing it changes what the writer emits for a malformed input,
which is well outside the manifest of #419 (whose deliverable is the report). PR #522 documents the
interaction instead: the report claims completeness over the thumbnail bytes, and does name this
drop — it is the emitted blob, not the report, that is silent.

Options a fix should weigh

  1. Remove the pointer unconditionally, so the model never carries an address it could not follow.
    Consistent with how the Exif/GPS/Interop pointer tags are already stripped, and with
    Thumbnail::from_jpeg, which deliberately does not store the offset because the writer
    synthesises it. Changes to_bytes output for a malformed input.
  2. Keep the pointer but have the writer refuse to emit an offset it cannot satisfy.
  3. Leave it and document it as a preservation guarantee — hard to defend, since the preserved value
    is knowably wrong.

Option 1 looks right, but it is a writer behaviour change and wants its own equivalence sweep.

Refs #419, #522.

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