This is an automated email from the ASF dual-hosted git repository. tballison pushed a commit to branch TIKA-4835-spill-sites in repository https://gitbox.apache.org/repos/asf/tika.git
commit 55156aa176b139ad69b95d7bfb23d1bcc2ed2955 Merge: 58e33ebfa3 379c88a92e Author: tallison <[email protected]> AuthorDate: Wed Aug 26 11:41:41 2026 -0400 TIKA-4835 -- merge primitives review fixes; POIFS budget accounting; ImageIO CHANGES.txt | 35 +++-- docs/modules/ROOT/pages/pipes/configuration.adoc | 27 ++-- .../apache/tika/digest/BufferingDigestSink.java | 52 ++++--- .../org/apache/tika/digest/CompositeDigester.java | 55 +++---- .../java/org/apache/tika/digest/DigestHelper.java | 20 ++- .../java/org/apache/tika/digest/DigestSink.java | 67 +++++++++ .../main/java/org/apache/tika/digest/Digester.java | 6 +- .../apache/tika/digest/InputStreamDigester.java | 22 ++- .../org/apache/tika/io/CachingInputStream.java | 5 +- .../java/org/apache/tika/io/CachingSource.java | 4 +- .../apache/tika/io/MemorySeekableByteChannel.java | 3 +- .../org/apache/tika/io/TemporaryResources.java | 9 +- .../java/org/apache/tika/io/TikaInputStream.java | 16 +- .../org/apache/tika/digest/DigestHelperTest.java | 102 +++++++++++++ .../org/apache/tika/digest/DigestSinkTest.java | 166 +++++++++++++++++++-- .../apache/tika/digest/FailingTestTranslator.java | 54 +++++++ .../apache/tika/io/InMemoryContentViewTest.java | 40 +++-- .../org/apache/tika/io/StreamCacheBudgetTest.java | 48 ++++-- .../org/apache/tika/io/TemporaryResourcesTest.java | 22 --- ....apache.tika.extractor.EmbeddedStreamTranslator | 1 + .../org/apache/tika/parser/image/ImageParser.java | 8 +- .../parser/image/ImageParsersNoTempFileTest.java | 37 ++++- .../detect/microsoft/POIFSContainerDetector.java | 74 +++++---- .../POIFSContainerDetectorNoTempFileTest.java | 45 ++++++ .../detect/microsoft/POIFSDeclaredSizeTest.java | 50 +++++-- .../apache/tika/pipes/core/server/PipesServer.java | 39 +++-- .../core/server/CacheMemoryBudgetSeedingTest.java | 18 +++ 27 files changed, 796 insertions(+), 229 deletions(-) diff --cc CHANGES.txt index 2c0ebe5208,06ce3f95d7..4f886b4ba2 --- a/CHANGES.txt +++ b/CHANGES.txt @@@ -1,39 -1,23 +1,50 @@@ Release 4.1.0 - unreleased + * Parsers and detectors no longer spool in-memory input to a temp file just + to read it back with random access: the JPEG, TIFF and WebP parsers, the + OpenDocument parser's inline pictures, the OLE2 container detector's + entry-name read, and PDFParser (the incremental-update scan and the main + load). Together these were the bulk of the temp bytes behind 4.0.0's + batch slowdown on spinning disks (see docs/.../pipes/performance.adoc). + One rule everywhere: content goes through the stream cache (bounded by + the pipes cache memory budget, 1MB per object without one); what stays + in memory is read in place through a zero-copy view, what spills is + read from the file. PDFBox and metadata-extractor no longer receive a + second heap copy of the document. The PDF renderer now re-opens the + document from byte 0 for every render, which fixes per-page rendering + (RENDER_PAGES_AT_PAGE_END) silently producing only the first page -- - for file-backed input too. PDFParser.getPDDocumentFromStream is ++ for file-backed input too. The ImageIO-based ImageParser (PNG, GIF, BMP, ...) ++ reads image headers from memory instead of through ImageIO's default ++ file cache, which created a temp file per image and wrote every byte it ++ read to it (600k writes on a 20k-file sample; the page cache absorbed most ++ of them before they reached disk, so this is syscall and file-churn ++ savings more than disk bytes). PDFParser.getPDDocumentFromStream is + deprecated and no longer called; override the new + getPDDocument(RandomAccessRead, ...) instead. ImageMetadataExtractor + gains TikaInputStream and InputStream overloads. One visible metadata + change: JPEG/TIFF/WebP images read from memory no longer carry + metadata-extractor's file-system tags (img:File Name, img:File Size, + img:File Modified Date), which described whatever file was read rather + than the image; input backed by a file still carries them (TIKA-4835). + * tika-pipes: the cache memory budget (how much rewindable content a forked - worker keeps in memory before spilling to disk) now defaults to a quarter - of the fork's heap, so raising -Xmx raises it; it was a fixed 256MB. It is - one pool per forked JVM shared by all of its threads. - -Dtika.pipes.cacheMemoryBudgetBytes in forkedJvmArgs still overrides it - (below the quarter-heap ceiling; <=0 disables). TikaInputStream.hasFile() - now also reports content the stream cache spilled on its own, not only - content a getPath() call put on disk. Digester gains digestSink(), an - OutputStream that digests as it is written; DigestHelper uses it for - translated embedded streams, which no longer touch a temp file. - TemporaryResources and CachingSource now close every remaining resource - when one close() throws an unchecked exception (TIKA-4835). + worker keeps in memory before spilling to disk; new since 4.0.0, which had + no budget at all) defaults to a quarter of the fork's heap, so raising + -Xmx raises it. It is one pool per forked JVM shared by all of its threads. + -Dtika.pipes.cacheMemoryBudgetBytes in forkedJvmArgs overrides it (below + the quarter-heap ceiling; <=0 disables); the fork logs the value and its + source at startup. TikaInputStream.hasFile() now also reports content the + stream cache spilled on its own, not only content a getPath() call put on + disk. TikaInputStream.inMemoryContent(channel) gives a zero-copy read-only + view of cached content for consumers that need random access. Digester + gains digestSink(), a DigestSink that digests as it is written and can be + aborted; DigestHelper uses it for translated embedded streams, which no + longer touch a temp file when the digester implements digestSink (all of + Tika's do; one that only implements digest() still buffers), and a failed + translation now publishes no digest instead of a digest of the fragment. + TemporaryResources.closeAll(Closeable...) closes every argument even when + one throws unchecked; TemporaryResources, CachingSource, CachingInputStream + and CompositeDigester use it (TIKA-4835). * Pipes plugins no longer bundle their own Jackson: jackson-core, -databind and -annotations are provided by the host (tika-serialization) and the diff --cc tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ImageParser.java index c764d9e403,c764d9e403..8f33335111 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ImageParser.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ImageParser.java @@@ -28,6 -28,6 +28,7 @@@ import javax.imageio.ImageIO import javax.imageio.ImageReader; import javax.imageio.metadata.IIOMetadata; import javax.imageio.stream.ImageInputStream; ++import javax.imageio.stream.MemoryCacheImageInputStream; import org.apache.commons.io.input.CloseShieldInputStream; import org.slf4j.Logger; @@@ -169,8 -169,8 +170,11 @@@ public class ImageParser extends Abstra if (iterator.hasNext()) { ImageReader reader = iterator.next(); try { -- try (ImageInputStream imageStream = ImageIO -- .createImageInputStream(CloseShieldInputStream.wrap(stream))) { ++ // Not ImageIO.createImageInputStream(InputStream): with the JDK default ++ // useCache=true that is a FileCacheImageInputStream, which writes every byte ++ // it reads to a temp file. Header reads are small; keep them in memory. ++ try (ImageInputStream imageStream = ++ new MemoryCacheImageInputStream(CloseShieldInputStream.wrap(stream))) { reader.setInput(imageStream); try { int numImages = reader.getNumImages(true); diff --cc tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/test/java/org/apache/tika/parser/image/ImageParsersNoTempFileTest.java index 7a72e536a6,0000000000..095d885b84 mode 100644,000000..100644 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/test/java/org/apache/tika/parser/image/ImageParsersNoTempFileTest.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/test/java/org/apache/tika/parser/image/ImageParsersNoTempFileTest.java @@@ -1,138 -1,0 +1,173 @@@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; ++import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.io.ByteArrayInputStream; ++import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; ++import java.nio.file.attribute.PosixFilePermissions; +import java.util.stream.Stream; ++import javax.imageio.ImageIO; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.xml.sax.helpers.DefaultHandler; + +import org.apache.tika.TikaTest; +import org.apache.tika.io.CacheMemoryBudget; +import org.apache.tika.io.TemporaryResources; +import org.apache.tika.io.TikaInputStream; ++import org.apache.tika.metadata.HttpHeaders; +import org.apache.tika.metadata.Metadata; +import org.apache.tika.metadata.TIFF; +import org.apache.tika.parser.ParseContext; +import org.apache.tika.parser.Parser; + +/** + * Embedded images usually arrive already in memory; the image parsers must not spool them to + * disk just to read metadata. The file-system tags are the visible consequence: they describe + * whatever file was read, so in-memory input must not carry them and real files must. + */ +public class ImageParsersNoTempFileTest extends TikaTest { + + private static final String FILE_NAME_KEY = ImageMetadataExtractor.UNKNOWN_IMG_NS + "File Name"; + private static final String FILE_SIZE_KEY = ImageMetadataExtractor.UNKNOWN_IMG_NS + "File Size"; + private static final String FILE_MODIFIED_KEY = + ImageMetadataExtractor.UNKNOWN_IMG_NS + "File Modified Date"; + + @TempDir + Path tempDir; + + private static final String JPEG = "/test-documents/testJPEG_EXIF.jpg"; + private static final String TIFF_RES = "/test-documents/testTIFF.tif"; + private static final String WEBP = "/test-documents/testWebp_Alpha_Lossless.webp"; + private static final String WEBP_WIDTH = ImageMetadataExtractor.UNKNOWN_IMG_NS + "Image Width"; ++ private static final String PNG = "/test-documents/testPNG.png"; + + private static ParseContext context() { + ParseContext context = new ParseContext(); + context.set(CacheMemoryBudget.class, new CacheMemoryBudget(64L * 1024 * 1024)); + return context; + } + + private byte[] bytes(String resource) throws Exception { + try (InputStream is = getResourceAsStream(resource)) { + return is.readAllBytes(); + } + } + + @Test + public void testJpegInMemory() throws Exception { + assertNotSpooled(new JpegParser(), JPEG, TIFF.IMAGE_WIDTH.getName()); + } + + @Test + public void testTiffInMemory() throws Exception { + assertNotSpooled(new TiffParser(), TIFF_RES, TIFF.IMAGE_WIDTH.getName()); + } + + @Test + public void testWebPInMemory() throws Exception { + assertNotSpooled(new WebPParser(), WEBP, WEBP_WIDTH); + } + ++ /** ++ * ImageIO's own file cache is outside TemporaryResources entirely, and its cache file is ++ * gone by the time a parse returns -- so the watched-directory check cannot see it. Make ++ * the cache directory unwritable instead: a parser that asks ImageIO for a file-backed ++ * stream then fails with "Can't create cache file"; one that reads from memory never ++ * notices. ++ */ ++ @Test ++ public void testImageIoParserUsesNoFileCache() throws Exception { ++ assumeTrue(Files.getFileStore(tempDir).supportsFileAttributeView("posix"), "needs POSIX permissions"); ++ Path cache = Files.createDirectory(tempDir.resolve("imageio-cache")); ++ File before = ImageIO.getCacheDirectory(); ++ boolean useCache = ImageIO.getUseCache(); ++ ImageIO.setUseCache(true); ++ ImageIO.setCacheDirectory(cache.toFile()); ++ Files.setPosixFilePermissions(cache, PosixFilePermissions.fromString("r-xr-xr-x")); ++ try { ++ assertNotSpooled(new ImageParser(), PNG, TIFF.IMAGE_WIDTH.getName()); ++ } finally { ++ Files.setPosixFilePermissions(cache, PosixFilePermissions.fromString("rwxr-xr-x")); ++ ImageIO.setCacheDirectory(before); ++ ImageIO.setUseCache(useCache); ++ } ++ } ++ + @Test + public void testJpegFromFile() throws Exception { + assertKeepsFileTags(new JpegParser(), JPEG, TIFF.IMAGE_WIDTH.getName(), "jpg"); + } + + @Test + public void testTiffFromFile() throws Exception { + assertKeepsFileTags(new TiffParser(), TIFF_RES, TIFF.IMAGE_WIDTH.getName(), "tif"); + } + + @Test + public void testWebPFromFile() throws Exception { + assertKeepsFileTags(new WebPParser(), WEBP, WEBP_WIDTH, "webp"); + } + + private void assertNotSpooled(Parser parser, String resource, String widthKey) + throws Exception { + byte[] bytes = bytes(resource); + Metadata metadata = new Metadata(); ++ if (resource.endsWith(".png")) { ++ metadata.set(HttpHeaders.CONTENT_TYPE, "image/png"); ++ } + try (TemporaryResources tmp = new TemporaryResources()) { + tmp.setTemporaryFileDirectory(tempDir); + // a stream-backed, non-file TikaInputStream whose only spill target is tempDir + TikaInputStream tis = TikaInputStream.get(new ByteArrayInputStream(bytes), tmp, metadata); + parser.parse(tis, new DefaultHandler(), metadata, context()); + // temp files live until tmp closes, so any spool would be visible right here + try (Stream<Path> files = Files.list(tempDir)) { - assertEquals(0, files.count(), "parser spooled an in-memory image to disk"); ++ assertEquals(0, files.filter(Files::isRegularFile).count(), ++ "parser spooled an in-memory image to disk"); + } + } + assertNotNull(metadata.get(widthKey), "metadata was extracted"); + assertNull(metadata.get(FILE_NAME_KEY), "in-memory input must not carry file tags"); + assertNull(metadata.get(FILE_SIZE_KEY), "in-memory input must not carry file tags"); + assertNull(metadata.get(FILE_MODIFIED_KEY), "in-memory input must not carry file tags"); + } + + private void assertKeepsFileTags(Parser parser, String resource, String widthKey, String ext) + throws Exception { + byte[] bytes = bytes(resource); + Path image = tempDir.resolve("image." + ext); + Files.write(image, bytes); + Metadata metadata = new Metadata(); + try (TikaInputStream tis = TikaInputStream.get(image, metadata)) { + parser.parse(tis, new DefaultHandler(), metadata, context()); + } + assertNotNull(metadata.get(widthKey), "metadata was extracted"); + assertEquals(image.getFileName().toString(), metadata.get(FILE_NAME_KEY), + "a real file keeps metadata-extractor's file-system tags"); + assertNotNull(metadata.get(FILE_SIZE_KEY), "a real file keeps its size tag"); + } +} diff --cc tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/detect/microsoft/POIFSContainerDetector.java index 4b5f470815,356dcba2f1..a0500ae985 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/detect/microsoft/POIFSContainerDetector.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/detect/microsoft/POIFSContainerDetector.java @@@ -562,22 -554,9 +562,15 @@@ public class POIFSContainerDetector imp } - /** - * Largest object opened from memory. POI's stream loader copies the whole object into a - * heap array, so this bounds that copy; the cache underneath is already budget-bounded. - * Embedded OLE2 objects are typically a few hundred KB. - */ - private static final long MAX_IN_MEMORY_POIFS = 8L * 1024 * 1024; - - private Set<String> getTopLevelNames(TikaInputStream stream) throws IOException { - // Force the document stream to a (possibly temporary) file - // so we don't modify the current position of the stream. + private Set<String> getTopLevelNames(TikaInputStream stream, ParseContext context) + throws IOException { + if (!stream.hasFile()) { + Set<String> names = getTopLevelNamesInMemory(stream, context); + if (names != null) { + return names; + } + } + // random access over content that is not in memory: use the file Path file = stream.getPath(); if (file == null) { @@@ -604,78 -583,6 +597,107 @@@ } } + /** - * Opens the container from memory. Returns null -- and the caller uses the file -- when - * the object is too large, its header would make POI allocate too much, or POI's stream - * loader rejects it (it is stricter than the file loader on truncated objects). ++ * Opens the container from memory. POI's stream loader copies the object into its own ++ * heap array, sized from the header rather than the content, and keeps it for the ++ * container's lifetime -- so that allocation is reserved from the CacheMemoryBudget ++ * before it is made. Returns null, and the caller uses the file, when the content is not ++ * in memory, the budget has no room for the copy, or POI's stream loader rejects the ++ * object (it is stricter than the file loader on truncated objects). Without a budget ++ * only objects under the cache's threshold have an in-memory view, which bounds the copy. + */ + private Set<String> getTopLevelNamesInMemory(TikaInputStream stream, ParseContext context) + throws IOException { - if (stream.hasLength() && stream.getLength() > MAX_IN_MEMORY_POIFS) { - return null; - } ++ CacheMemoryBudget budget = context == null ? null : context.get(CacheMemoryBudget.class); + // the budget decides how much the drain below keeps in memory - stream.enableRewind(context == null ? null : context.get(CacheMemoryBudget.class)); ++ stream.enableRewind(budget); + POIFSFileSystem fs = null; ++ long reserved = 0; + try (SeekableByteChannel channel = stream.getSeekableByteChannel()) { - if (TikaInputStream.inMemoryContent(channel) == null || - channel.size() > MAX_IN_MEMORY_POIFS || - declaredInMemorySize(channel) > MAX_IN_MEMORY_POIFS) { ++ if (TikaInputStream.inMemoryContent(channel) == null) { + return null; + } ++ // POI allocates what the header declares. The bytes in hand are the truth: a ++ // header that declares more than they can account for is lying, and the file ++ // loader (block by block, no such allocation) is the only safe way to read it. ++ long copy = honestDeclaredSize(channel); ++ if (copy < 0) { ++ return null; ++ } ++ if (budget != null) { ++ if (budget.tryReserve(copy) == 0) { ++ return null; ++ } ++ reserved = copy; ++ } + fs = new POIFSFileSystem(Channels.newInputStream(channel)); + Set<String> names = getTopLevelNames(fs.getRoot()); + stream.setOpenContainer(fs); ++ if (reserved > 0) { ++ long charged = reserved; ++ stream.addCloseableResource(() -> budget.release(charged)); ++ reserved = 0; ++ } + fs = null; + return names; + } catch (SecurityException e) { + throw e; + } catch (IOException | RuntimeException e) { + return null; + } finally { ++ if (reserved > 0) { ++ budget.release(reserved); ++ } + if (fs != null) { + closeQuietly(fs); + } + } + } + + /** - * The heap POI would allocate for this object if opened from a stream. It sizes that buffer - * from the header's declared BAT count rather than the actual length, so a 512-byte object - * can demand hundreds of MB; the file loader reads block by block and never allocates it. - * Returns 0 when the header cannot be read, leaving the rejection to POI. ++ * The heap POI's stream loader would allocate for this object -- sized from the header's ++ * declared BAT count, not the content -- or -1 when the header cannot be read or declares ++ * more than the content can account for. A valid header covers at most one BAT block of ++ * unused entries beyond the actual size; anything past that is a malformed or hostile ++ * header (a 512-byte object can declare hundreds of MB) and must not be opened from a ++ * stream at all. + */ - static long declaredInMemorySize(SeekableByteChannel channel) throws IOException { ++ static long honestDeclaredSize(SeekableByteChannel channel) throws IOException { + byte[] header = new byte[POIFSConstants.SMALLER_BIG_BLOCK_SIZE]; + long start = channel.position(); + try { + channel.position(0); + ByteBuffer buffer = ByteBuffer.wrap(header); + while (buffer.hasRemaining() && channel.read(buffer) > 0) { + // fill the header block + } + if (buffer.hasRemaining()) { - return 0; ++ return -1; + } + } finally { + channel.position(start); + } + try { - return BATBlock.calculateMaximumSize( - new HeaderBlock(UnsynchronizedByteArrayInputStream.builder().setByteArray(header).get())); ++ HeaderBlock hb = new HeaderBlock( ++ UnsynchronizedByteArrayInputStream.builder().setByteArray(header).get()); ++ long declared = BATBlock.calculateMaximumSize(hb); ++ long oneBatSpan = (long) hb.getBigBlockSize().getBigBlockSize() * ++ hb.getBigBlockSize().getBATEntriesPerBlock(); ++ long actual = channel.size(); ++ return declared > actual + oneBatSpan ? -1 : declared; + } catch (IOException | RuntimeException e) { - return 0; ++ return -1; + } + } + + private static void closeQuietly(POIFSFileSystem fs) { + try { + fs.close(); + } catch (IOException e) { + LOG.debug("failed to close abandoned POIFSFileSystem", e); + } + } + public MediaType detect(TikaInputStream tis, Metadata metadata, ParseContext parseContext) throws IOException { // Check if we have access to the document if (tis == null) { diff --cc tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/detect/microsoft/POIFSContainerDetectorNoTempFileTest.java index b00f2e57e7,0000000000..6a06ded324 mode 100644,000000..100644 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/detect/microsoft/POIFSContainerDetectorNoTempFileTest.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/detect/microsoft/POIFSContainerDetectorNoTempFileTest.java @@@ -1,71 -1,0 +1,116 @@@ +/* + * 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.detect.microsoft; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.stream.Stream; + +import org.apache.poi.poifs.filesystem.POIFSFileSystem; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import org.apache.tika.TikaTest; +import org.apache.tika.io.CacheMemoryBudget; +import org.apache.tika.io.TemporaryResources; +import org.apache.tika.io.TikaInputStream; +import org.apache.tika.metadata.Metadata; +import org.apache.tika.mime.MediaType; +import org.apache.tika.parser.ParseContext; + +/** + * Detection used to spool every in-memory OLE2 object to a temp file to read its + * top-level entry names; it must now open the container from memory. + */ +public class POIFSContainerDetectorNoTempFileTest extends TikaTest { + + @TempDir + Path tempDir; + + @Test + public void testNoTempFileForInMemoryInput() throws Exception { + byte[] bytes; + try (InputStream is = getResourceAsStream("/test-documents/testWORD.doc")) { + bytes = is.readAllBytes(); + } + ParseContext context = new ParseContext(); + context.set(CacheMemoryBudget.class, new CacheMemoryBudget(64L * 1024 * 1024)); + Metadata metadata = new Metadata(); + try (TemporaryResources tmp = new TemporaryResources()) { + tmp.setTemporaryFileDirectory(tempDir); + TikaInputStream tis = TikaInputStream.get(new ByteArrayInputStream(bytes), tmp, metadata); + MediaType type = new POIFSContainerDetector().detect(tis, metadata, context); + assertEquals(MediaType.application("msword"), type); + try (Stream<Path> files = Files.list(tempDir)) { + assertEquals(0, files.count(), "detector spooled an in-memory OLE2 object to disk"); + } + assertTrue(tis.getOpenContainer() instanceof POIFSFileSystem, "open container kept for the parser"); + assertEquals(0, tis.getPosition(), "detection must not move the stream"); + assertEquals(0xd0, tis.read(), "stream still readable from the start"); + } + } ++ ++ /** POI's copy is charged to the budget for the container's lifetime, then released. */ ++ @Test ++ public void testInMemoryCopyIsChargedAndReleased() throws Exception { ++ byte[] bytes; ++ try (InputStream is = getResourceAsStream("/test-documents/testWORD.doc")) { ++ bytes = is.readAllBytes(); ++ } ++ CacheMemoryBudget budget = new CacheMemoryBudget(64L * 1024 * 1024); ++ ParseContext context = new ParseContext(); ++ context.set(CacheMemoryBudget.class, budget); ++ Metadata metadata = new Metadata(); ++ TikaInputStream tis; ++ try (TemporaryResources tmp = new TemporaryResources()) { ++ tis = TikaInputStream.get(new ByteArrayInputStream(bytes), tmp, metadata); ++ assertEquals(MediaType.application("msword"), ++ new POIFSContainerDetector().detect(tis, metadata, context)); ++ assertTrue(tis.getOpenContainer() instanceof POIFSFileSystem); ++ assertTrue(budget.getReservedBytes() >= bytes.length, ++ "POI's header-sized copy is charged while the container is open"); ++ } ++ assertEquals(0, budget.getReservedBytes(), "released with the stream"); ++ } ++ ++ /** No room in the budget for POI's copy: detection still succeeds, from the file. */ ++ @Test ++ public void testFallsBackToFileWhenBudgetRefusesTheCopy() throws Exception { ++ byte[] bytes; ++ try (InputStream is = getResourceAsStream("/test-documents/testWORD.doc")) { ++ bytes = is.readAllBytes(); ++ } ++ CacheMemoryBudget budget = new CacheMemoryBudget(4096); ++ ParseContext context = new ParseContext(); ++ context.set(CacheMemoryBudget.class, budget); ++ Metadata metadata = new Metadata(); ++ try (TemporaryResources tmp = new TemporaryResources()) { ++ tmp.setTemporaryFileDirectory(tempDir); ++ TikaInputStream tis = TikaInputStream.get(new ByteArrayInputStream(bytes), tmp, metadata); ++ assertEquals(MediaType.application("msword"), ++ new POIFSContainerDetector().detect(tis, metadata, context)); ++ assertTrue(tis.getOpenContainer() instanceof POIFSFileSystem); ++ assertTrue(tis.hasFile(), "opened from the file instead"); ++ } ++ assertEquals(0, budget.getReservedBytes(), "nothing left charged"); ++ } +} diff --cc tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/detect/microsoft/POIFSDeclaredSizeTest.java index 348655d6e5,0000000000..6baedcb2f7 mode 100644,000000..100644 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/detect/microsoft/POIFSDeclaredSizeTest.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/detect/microsoft/POIFSDeclaredSizeTest.java @@@ -1,115 -1,0 +1,137 @@@ +/* + * 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.detect.microsoft; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayInputStream; ++import java.io.InputStream; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.nio.channels.SeekableByteChannel; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import org.apache.tika.TikaTest; +import org.apache.tika.io.CacheMemoryBudget; +import org.apache.tika.io.TemporaryResources; +import org.apache.tika.io.TikaInputStream; +import org.apache.tika.metadata.Metadata; +import org.apache.tika.parser.ParseContext; + +/** + * POI sizes its in-memory OLE2 buffer from the header's declared BAT count rather than the + * actual length, so a 512-byte object can demand hundreds of MB. The in-memory detection path - * must reject those on the declared size, not on the real one. ++ * only believes a header the bytes in hand can account for, and reserves that from the ++ * budget before POI allocates it. + */ +public class POIFSDeclaredSizeTest extends TikaTest { + + private static final int BAT_COUNT_OFFSET = 0x2C; + private static final int SECTOR_SHIFT_OFFSET = 0x1E; + + @TempDir + Path tempDir; + + /** A bare 512-byte OLE2 header declaring {@code batCount} BAT blocks and nothing else. */ + private static byte[] header(int batCount) { + byte[] data = new byte[512]; + byte[] magic = {(byte) 0xd0, (byte) 0xcf, 0x11, (byte) 0xe0, + (byte) 0xa1, (byte) 0xb1, 0x1a, (byte) 0xe1}; + System.arraycopy(magic, 0, data, 0, magic.length); + data[SECTOR_SHIFT_OFFSET] = 9; // 2^9 = 512-byte blocks + ByteBuffer.wrap(data).order(ByteOrder.LITTLE_ENDIAN).putInt(BAT_COUNT_OFFSET, batCount); + return data; + } + + private SeekableByteChannel channelFor(byte[] bytes, String name) throws Exception { + Path p = tempDir.resolve(name); + Files.write(p, bytes); + return Files.newByteChannel(p, StandardOpenOption.READ); + } + + @Test - public void testDeclaredSizeFollowsHeaderNotLength() throws Exception { - // (1 + 3813 * 128) * 512 == 249_889_280, just under POI's 250MB allocation ceiling ++ public void testLyingHeaderIsRejected() throws Exception { ++ // (1 + 3813 * 128) * 512 == 249_889_280, just under POI's 250MB allocation ceiling, ++ // declared by 512 bytes of content + try (SeekableByteChannel channel = channelFor(header(3813), "hostile.ole")) { - long declared = POIFSContainerDetector.declaredInMemorySize(channel); - assertEquals(249_889_280L, declared, - "a 512-byte object declares a ~238MB in-memory buffer"); ++ assertEquals(-1, POIFSContainerDetector.honestDeclaredSize(channel), ++ "the content cannot account for what the header declares"); + assertEquals(0, channel.position(), "the size probe must not move the channel"); + } + } + + @Test - public void testModestHeaderIsNotRejected() throws Exception { ++ public void testHonestHeaderIsBelievedWithinOneBatBlock() throws Exception { ++ // header only, declaring one BAT block: 129 sectors, 512 bytes present -- within slack + try (SeekableByteChannel channel = channelFor(header(1), "modest.ole")) { - assertEquals((1 + 128) * 512L, POIFSContainerDetector.declaredInMemorySize(channel)); ++ assertEquals((1 + 128) * 512L, POIFSContainerDetector.honestDeclaredSize(channel)); ++ } ++ // two BAT blocks declared by 512 bytes: one block past what the content covers ++ try (SeekableByteChannel channel = channelFor(header(2), "twoblocks.ole")) { ++ assertEquals(-1, POIFSContainerDetector.honestDeclaredSize(channel)); ++ } ++ } ++ ++ @Test ++ public void testRealDocumentHeaderIsHonest() throws Exception { ++ byte[] bytes; ++ try (InputStream is = getResourceAsStream("/test-documents/testWORD.doc")) { ++ bytes = is.readAllBytes(); ++ } ++ try (SeekableByteChannel channel = channelFor(bytes, "real.doc")) { ++ long declared = POIFSContainerDetector.honestDeclaredSize(channel); ++ assertTrue(declared >= bytes.length && declared <= bytes.length + 128 * 512L, ++ "a real header declares about its own size: " + declared + " vs " + bytes.length); + } + } + + @Test - public void testTooShortForAHeaderReportsZero() throws Exception { ++ public void testTooShortForAHeaderIsRejected() throws Exception { + try (SeekableByteChannel channel = channelFor(new byte[16], "short.bin")) { - assertEquals(0, POIFSContainerDetector.declaredInMemorySize(channel), - "no header to read; leave the rejection to POI"); ++ assertEquals(-1, POIFSContainerDetector.honestDeclaredSize(channel)); + } + } + + /** - * End to end: the crafted object must not be opened in memory, so no POIFSFileSystem is - * retained. Before the declared-size check this allocated ~238MB first. ++ * End to end: the crafted object is refused before any reservation, so POI never ++ * allocates its declared size and no POIFSFileSystem is retained -- even with a budget ++ * large enough to have said yes. + */ + @Test + public void testHostileHeaderDoesNotBecomeAnOpenContainer() throws Exception { + ParseContext context = new ParseContext(); - context.set(CacheMemoryBudget.class, new CacheMemoryBudget(64L * 1024 * 1024)); ++ CacheMemoryBudget budget = new CacheMemoryBudget(1024L * 1024 * 1024); ++ context.set(CacheMemoryBudget.class, budget); + Metadata metadata = new Metadata(); + try (TemporaryResources tmp = new TemporaryResources()) { + TikaInputStream tis = TikaInputStream.get( + new ByteArrayInputStream(header(3813)), tmp, metadata); + POIFSContainerDetector detector = new POIFSContainerDetector(); + assertTrue(detector.detect(tis, metadata, context) != null); + assertNull(tis.getOpenContainer(), + "a header-only object must not be opened from memory"); + } ++ assertEquals(0, budget.getReservedBytes(), "nothing was ever reserved for it"); + } +}
