[ 
https://issues.apache.org/jira/browse/TIKA-4775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095539#comment-18095539
 ] 

ASF GitHub Bot commented on TIKA-4775:
--------------------------------------

dschmidt commented on PR #2946:
URL: https://github.com/apache/tika/pull/2946#issuecomment-4944389762

   This is what my agent says:
   
   ```
   Overview: the direction is right and the architecture is sound — one hardened
   SAX flattener (XmpSaxFlattener -> (uri, path, value, lang) leaves) feeding a
   single (uri, localName) property table in XmpExtractor, replacing the three
   DOM-based paths. XXE is handled correctly (XMLReaderUtils.parseSAX +
   OfflineContentHandler, verified), the five PDF custom schemas are migrated
   1:1 (verified against the table), docinfo-vs-XMP precedence is preserved,
   the URI-keyed Google Camera mapping is prefix-proof without xmpcore's global
   schema registry, and HEIF/WebP XMP are genuinely new capabilities. But there
   are six correctness findings, two of which are crash regressions.
   
   BUGS
   1. Bare rdf:li outside Bag/Seq/Alt -> NPE (XmpSaxFlattener:121, 
liCount.peek()
      on an empty deque). PDMetadataExtractor catches only IO/SAX/TikaException,
      IDMLParser now propagates where the deleted code swallowed Throwable, so
      malformed XMP kills the whole PDF/IDML parse. The old paths recorded and
      continued.
   2. rdf:Alt handling: for single-valued properties the LAST alternative wins
      (emit() does set per li), where the old path effectively kept x-default;
      for multi-valued ones (dc:title is a text bag) every translation
      accumulates as a separate value. testLanguageVariants pointedly never
      asserts TITLE after a multi-lang alt.
   3. The property table lookup is nesting-blind: xmpMM:InstanceID inside
      xmpMM:Pantry/Ingredients (standard InDesign/Illustrator output) overwrites
      the document-level xmpMM:InstanceID.
   4. The rdf:value + qualifier pattern drops the value: the qualifier bumps
      childCount, so the parent is no longer treated as a leaf.
   5. xmpMM:History parallel arrays lose alignment: absent action/when/
      softwareAgent used to be padded with "", now they are skipped, so
      HISTORY_ACTION[i] no longer matches HISTORY_WHEN[i].
   6. WebP: a foreign RIFF chunk > 64MB before the XMP chunk aborts the whole
      scan (ImageXmp.readRiffChunk returns null) instead of skipping it; the cap
      should only gate the target chunk's allocation.
   
   RISKS
   - The MAX_EVENT_HISTORY_IN_XMPMM=1024 cap did not survive the rewrite and the
     flattener's leaf list is unbounded — a hostile packet inflates metadata
     without limit.
   - xmp-raw:* keys are built from the document's *prefix*, not the namespace
     URI: the same property yields different keys across files and two
     namespaces bound to one prefix collide.
   - Extended XMP: no MD5 verification of the reassembled packet against the
     GUID (XMP Part 3 defines the GUID as that digest); missing chunks leave
     zero-filled gaps; the HasExtendedXMP regex misses multi-space/newline
     attribute forms; a second standard packet keeps an extended buffer paired
     to the previous GUID.
   - HEIF: scanAndExtract takes the first <?xpacket anywhere in the stream, so
     an embedded resource's XMP (e.g. a motion-photo video inside mdat) can be
     attributed to the image.
   - Is dropping xmp:About intentional? rdf:about is skipped with the other RDF
     attributes and nothing replaces the old mapping.
   
   NITS
   - xmp-commons now pulls in full pdfbox (plus fontbox transitively) solely for
     DateConverter, which lands on standalone image/IDML consumers.
   - jempbox.version in tika-parent and the jempbox|/xmpbox| Embed-Dependency
     patterns in tika-bundle-standard are dead after this PR.
   
   Nice touches worth keeping: date normalization is TZ-deterministic (verified
   empirically under two machine TZs), the JPEG double-parse is cleanly killed
   via JPEG_READERS_NO_XMP, and the iloc/box walks in HeifXmp are properly
   bounds-checked with sane caps.
   
   Test gaps mirroring the bugs: no coverage for malformed bare rdf:li, the
   nested-struct collision, rdf:value qualifiers, history alignment, caps,
   Alt-winner on the canonical key, multiple rdf:RDF blocks, or equality
   assertions for partial dates (YYYY / YYYY-MM currently only assertNotNull —
   they inflate to fabricated full timestamps).
   ```
   




> Experiment unifying xmp parsing in 4.x
> --------------------------------------
>
>                 Key: TIKA-4775
>                 URL: https://issues.apache.org/jira/browse/TIKA-4775
>             Project: Tika
>          Issue Type: Task
>            Reporter: Tim Allison
>            Priority: Minor
>
> We parse xmp at least 3 different ways currently, and we're basing what keys 
> we're looking for based on the container file mime type. We should normalize 
> xmp parsing.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to