Copilot commented on code in PR #3090:
URL: https://github.com/apache/tika/pull/3090#discussion_r3900930813
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/test/java/org/apache/tika/parser/mp4/MP4ParserTest.java:
##########
@@ -179,16 +181,54 @@ public void testMP4MultipleCovers() throws Exception {
assertEquals(3, metadataList.size());
//a png data atom (well-known type 14) followed by a jpeg one (13)
+ //covr carries no picture type, so the first image is the thumbnail
Metadata front = metadataList.get(1);
assertEquals("image/png", front.get(HttpHeaders.CONTENT_TYPE));
- assertEquals(TikaCoreProperties.EmbeddedResourceType.INLINE.toString(),
+
assertEquals(TikaCoreProperties.EmbeddedResourceType.THUMBNAIL.toString(),
front.get(TikaCoreProperties.EMBEDDED_RESOURCE_TYPE));
Metadata back = metadataList.get(2);
assertEquals("image/jpeg", back.get(HttpHeaders.CONTENT_TYPE));
assertEquals(TikaCoreProperties.EmbeddedResourceType.INLINE.toString(),
back.get(TikaCoreProperties.EMBEDDED_RESOURCE_TYPE));
}
+ /**
+ * A cover in a second udta box is still an inline picture: the first
+ * cover of the file is its thumbnail, not the first of every box
+ */
+ @Test
+ public void testCoversAcrossUserDataBoxes() throws Exception {
+ byte[] file;
+ try (InputStream is =
getResourceAsStream("/test-documents/testMP4_coverArt.m4a")) {
+ file = is.readAllBytes();
+ }
+ //append a copy of the file's udta box (with its covr) at the top level
+ int udta = indexOf(file, "udta".getBytes(StandardCharsets.ISO_8859_1))
- 4;
+ int size = ByteBuffer.wrap(file, udta, 4).getInt();
+ ByteArrayOutputStream bos = new ByteArrayOutputStream();
Review Comment:
`indexOf(...)` can return -1 (or match an incidental "udta" byte sequence),
which makes `udta = ... - 4` negative/out of bounds and yields a
hard-to-diagnose exception before the assertions. Add guards that the udta
marker was found at a plausible position and that the declared box size fits
within the file before slicing/copying.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-audiovideo-module/src/main/java/org/apache/tika/parser/ogg/OggAudioParser.java:
##########
@@ -117,8 +114,14 @@ protected static void extractChannelInfo(Metadata
metadata, int channelCount) {
}
}
- protected static void extractComments(Metadata metadata,
XHTMLContentHandler xhtml,
- VorbisStyleComments comments, ParseContext context)
+ /**
+ * @return the pictures carried in the comments; the caller emits them
+ * through {@link #extractPictures(List, XHTMLContentHandler,
ParseContext)},
+ * {@link FlacParser} first merges them with the native PICTURE blocks so
+ * one file yields one thumbnail
+ */
+ protected static List<PictureBlock> extractComments(Metadata metadata,
+ XHTMLContentHandler xhtml, VorbisStyleComments comments,
ParseContext context)
throws IOException, TikaException, SAXException {
Review Comment:
`extractComments(...)` no longer uses the `ParseContext context` parameter
after the refactor (pictures are returned and emitted by the caller). Keeping
an unused parameter is misleading and makes future refactors harder; consider
removing it and updating the small set of call sites accordingly.
--
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]
