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).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
