Skip to content

gamut-xmp: XmpSidecar::read silently truncates a catenated multi-packet sidecar to its first packet #562

Description

@justin13888

The behaviour

XmpSidecar::read reads the first <?xpacket?> packet of a .xmp file and silently discards
everything after it. There is no error, no flag, and no way for a caller to tell a one-packet file
from a truncated read.

The mechanism is XmpPacket::scansplit_packet (crates/gamut-xmp/src/packet.rs): the body is
the bytes between the first <?xpacket header and the next <?xpacket instruction, whichever
that is. In a catenated file the "next" instruction is the first packet's own trailer, so the body
ends there and the remaining packets are never looked at.

Reproduced with two of gamut's own XmpSidecar::write outputs concatenated (xmp:Rating = 5
then xmp:CreatorTool = gamut):

XmpSidecar::read(&file)  ->  Ok(meta) with 1 property
  get_text(xmp, "Rating")       == Some("5")
  get_text(xmp, "CreatorTool")  == None      // the whole second packet is gone

The reference implementation rejects the same bytes.

Why it is worth a decision

Adobe XMP Part 3, "External storage of metadata", asks that applications

Write external metadata as though it were embedded and then had the XMP packets extracted and
catenated by a postprocessor
.

(vendored references/xmp/xmp-part3.pdf, "External storage of metadata" bullets.)

That phrase describes precisely a file holding several packets end to end. Part 3 does not say what
a reader should do with one, and does not require a writer to emit more than one — XmpSidecar::write
emits exactly one, which is conformant. But it is the sentence that makes a multi-packet .xmp a
shape a conforming producer may hand us, so "first packet wins, rest dropped" is a choice, not an
absence of one.

Silent data loss is the part that deserves the decision: a caller merging a sidecar into an image
today can lose properties without any signal.

The options

  1. Reject a file carrying more than one packet, with a typed XmpError. Matches the reference
    implementation's posture, cannot lose data, and is a breaking change only for input gamut
    currently mis-reads.
  2. Read every packet and merge the graphs, which is the reading of Part 3's "catenated"
    sentence that loses nothing. Needs a stated conflict rule for a property present in two packets
    (last wins? first wins? error?), and that rule is itself unspecified by Part 3.
  3. Document the truncation and keep it — the status quo, now stated in XmpSidecar::read's
    documentation and in the module docs, so at least it is not a surprise.

(3) is what ships today; it is the honest floor, not an argument against (1) or (2). Note the
options are not symmetric: (1) and (2) both need a decision about XmpMeta::from_packet too, which
has the same single-packet behaviour for embedded payloads, where a catenation is not something a
container hands over.

Acceptance, if taken

  • The decision recorded in crates/gamut-xmp/STATUS.md, naming which of the three it is and why.
  • A test over a genuinely catenated file — two XmpSidecar::write outputs concatenated is the
    cheapest fixture — asserting the chosen behaviour.
  • A differential check against the reference engine for the rejection case, since the engine
    already rejects these bytes.
  • XmpSidecar::read, the sidecar module docs and README.md updated together; they are where a
    caller meets this.

Raised while reviewing #507 (issue #421). The behaviour predates that pull request; #507 documents
it and does not change it, because choosing between the three options has real consequences either
way and is not that pull request's scope.

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