Copilot commented on code in PR #3115: URL: https://github.com/apache/tika/pull/3115#discussion_r3915894889
########## tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/MotionPhoto.java: ########## @@ -0,0 +1,299 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tika.parser.image; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.xml.sax.SAXException; + +import org.apache.tika.extractor.EmbeddedDocumentExtractor; +import org.apache.tika.extractor.EmbeddedDocumentUtil; +import org.apache.tika.io.TikaInputStream; +import org.apache.tika.metadata.Google; +import org.apache.tika.metadata.HttpHeaders; +import org.apache.tika.metadata.Metadata; +import org.apache.tika.metadata.TikaCoreProperties; +import org.apache.tika.mime.MediaType; +import org.apache.tika.parser.ParseContext; +import org.apache.tika.sax.EmbeddedContentHandler; +import org.apache.tika.sax.XHTMLContentHandler; + +/** + * The video of a motion photo, appended after the image and described by the + * XMP the image parsers already extract (TIKA-4869): + * <ul> + * <li>a Motion Photo lists its parts in {@code Container:Directory}: the + * primary image first, the video last with nothing after it, each with + * an {@code Item:Length};</li> + * <li>the older MicroVideo gives {@code Camera:MicroVideoOffset}, the + * number of bytes from the end of the file to the start of the video.</li> + * </ul> + * Either way the video ends at the end of the file, so its start follows from + * its length. What is found there is typed by content: the declared + * {@code Item:Mime} is not used as a detection hint, because a hint would + * make a wrong length pass as a video, and nothing is emitted when detection + * recognizes nothing. + * <p> + * The same holds for HEIC motion photos, whose video sits in a trailing + * {@code mpvd} box; its 8 byte header is the primary item's padding, so the + * video still ends at the end of the file. + */ +final class MotionPhoto { + + /** + * The name the video is emitted under, with the extension of whatever it + * turns out to be. + */ + private static final String NAME = "motion-photo"; + + private static final String ITEM = "]/Container:Item/"; + private static final String DIRECTORY = "xmp-raw:Container:Directory["; + + /** + * The {@code Item:Semantic} of the video. + */ + private static final String MOTION_PHOTO = "MotionPhoto"; + + /** + * A directory holds a handful of items; this only bounds the walk. + */ + private static final int MAX_ITEMS = 64; + + /** + * Enough of the video for the detectors to recognize it. + */ + private static final int DETECTION_PREFIX = 8 * 1024; + + /** + * The branch below an emitted trailer, which is not searched for a trailer + * of its own: what is appended to an image may be an image again, and a + * crafted file can nest that as deep as it likes. + */ + private static final class Nested { + } + + private static final Nested NESTED = new Nested(); + + private MotionPhoto() { + } + + /** + * Emits the trailer as an embedded document, or nothing when the image + * declares none, when the declared length does not fit the file, or when + * the bytes there are not recognized. The last two are what sharing a + * motion photo out of a gallery leaves behind, a common enough thing that + * it is not worth an exception on a file that is otherwise fine; the XMP + * that promised the video is in the metadata for a client to see. + */ + static void extract(TikaInputStream tis, Metadata metadata, XHTMLContentHandler xhtml, + ParseContext context) throws IOException, SAXException { + if (context.get(Nested.class) != null) { + return; + } + Declaration declared = declaration(metadata); + if (declared == null) { + return; + } + //a length the file cannot hold is settled from what the stream already + //knows, before an image gets spilled to disk on the strength of it + if (tis.hasLength() && declared.length >= tis.getLength()) { + return; + } + Trailer trailer = locate(tis, declared, context); + if (trailer == null) { + return; + } + Metadata trailerMetadata = Metadata.newInstance(context); + //the name has to be set before the parse, and the declaration names the + //format the file was written with, which detection cannot always tell + //apart: an MP4 with the isom brand types as quicktime (TIKA-3646), and + //the MicroVideo format declares nothing at all. Where the two disagree + //about the kind of file it is, the bytes win. + boolean fromDetection = declared.mime != null + && !declared.mime.startsWith(trailer.type.getType() + "/"); + String extension = declared.mime == null ? "" : EmbeddedDocumentUtil + .getExtensionForMediaType( + fromDetection ? trailer.type.toString() : declared.mime); + trailerMetadata.set(TikaCoreProperties.RESOURCE_NAME_KEY, NAME + extension); + if (fromDetection && !extension.isEmpty()) { + trailerMetadata.set(TikaCoreProperties.RESOURCE_NAME_EXTENSION_INFERRED, true); + } + trailerMetadata.set(HttpHeaders.CONTENT_TYPE, trailer.type.toString()); + trailerMetadata.set(TikaCoreProperties.EMBEDDED_RESOURCE_TYPE, + TikaCoreProperties.EmbeddedResourceType.ATTACHMENT.name()); + EmbeddedDocumentExtractor extractor = + EmbeddedDocumentUtil.getEmbeddedDocumentExtractor(context); + if (!extractor.shouldParseEmbedded(trailerMetadata, context)) { + return; + } + InputStream bytes = open(trailer); + if (bytes == null) { + return; + } + context.set(Nested.class, NESTED); + try (TikaInputStream embedded = TikaInputStream.get(bytes)) { + extractor.parseEmbedded(embedded, new EmbeddedContentHandler(xhtml), trailerMetadata, + context, true); + } finally { + context.set(Nested.class, null); + } + } + + /** + * What the declaration points at: where the bytes start and what they turn + * out to be, or null when they are not there, are not recognized, or + * cannot be read. An image that parsed is not failed over a trailer that + * is out of reach. + */ + private static Trailer locate(TikaInputStream tis, Declaration declared, + ParseContext context) { + try { + Path file = tis.getPath(); + long start = Files.size(file) - declared.length; + if (start <= 0) { + return null; + } + MediaType type = detect(file, start, context); + if (type == null || MediaType.OCTET_STREAM.equals(type)) { + return null; + } Review Comment: `tis.getPath()` can be `null` for non-file-backed streams (or depending on how the `TikaInputStream` was constructed). If it’s null here, `Files.size(file)` will throw a `NullPointerException` and fail the whole image parse. Handle this explicitly (e.g., return `null` when `file == null`, or materialize/spool to a temp file before calling `Files.size(...)`). ########## tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-xmp-commons/src/main/java/org/apache/tika/parser/xmp/XmpSaxFlattener.java: ########## @@ -77,6 +79,26 @@ static boolean isContainer(String u, String l) { return RDF.equals(u) && (l.equals("Bag") || l.equals("Seq") || l.equals("Alt")); } + /** + * The prefix a namespace is normally written with, for the few whose + * raw keys are read elsewhere in Tika. An XMP prefix is the writer's + * own choice (ISO 16684-1) and rewriting a packet is enough to change + * it, so a key built from the document's prefix moves with it: a + * motion photo that has been through exiftool lists its parts under + * GContainer:Directory rather than Container:Directory. + */ + static final Map<String, String> CANONICAL_PREFIX = + Map.of(Google.CONTAINER_NS, "Container", Google.ITEM_NS, "Item"); + + /** + * The name a path segment is keyed under: the document's own qName, + * except for the namespaces above. + */ + static String canonical(String uri, String qName, String localName) { + String prefix = CANONICAL_PREFIX.get(uri); + return prefix == null ? qName : prefix + ":" + localName; Review Comment: If `localName` is empty (which can happen depending on SAX parser configuration/behavior), this will produce malformed path segments like `Container:` / `Item:` and break lookups that rely on these keys. Safer behavior is to fall back to `qName` when `localName` is null/empty, even when the namespace URI is one of the canonicalized ones. -- 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]
