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 4a687bbe54bc9c55d7078e4e5486034b5dc24986 Merge: 55156aa176 3235a919f4 Author: tallison <[email protected]> AuthorDate: Wed Aug 26 16:42:42 2026 -0400 TIKA-4835 -- merge primitives; review fixes; docs for the channel/view contract CHANGES.txt | 50 ++++++--- docs/modules/ROOT/pages/advanced/spooling.adoc | 30 +++++- docs/modules/ROOT/pages/configuration/index.adoc | 5 +- .../ROOT/pages/configuration/metadata-filters.adoc | 6 +- .../pages/configuration/parsers/tess4j-parser.adoc | 5 +- .../ROOT/pages/developers/serialization.adoc | 6 +- .../pages/maintainers/release-guides/tika.adoc | 18 ++-- docs/modules/ROOT/pages/migration-to-4x/index.adoc | 4 +- .../pages/migration-to-4x/metadata-changes-4x.adoc | 12 ++- .../pages/migration-to-4x/migrating-to-4x.adoc | 9 +- docs/modules/ROOT/pages/pipes/performance.adoc | 5 +- .../ROOT/pages/pipes/plugins/elasticsearch.adoc | 4 +- docs/modules/ROOT/pages/pipes/plugins/jdbc.adoc | 2 +- docs/modules/ROOT/pages/pipes/plugins/kafka.adoc | 4 +- .../ROOT/pages/pipes/plugins/opensearch.adoc | 4 +- docs/modules/ROOT/pages/pipes/plugins/solr.adoc | 6 +- .../ROOT/pages/pipes/plugins/writing-a-plugin.adoc | 12 ++- docs/modules/ROOT/pages/pipes/unpack-config.adoc | 2 +- docs/modules/ROOT/pages/using-tika/docker.adoc | 4 +- .../src/main/java/org/apache/tika/gui/TikaGUI.java | 4 +- .../apache/tika/digest/BufferingDigestSink.java | 34 ++++-- .../org/apache/tika/digest/CompositeDigester.java | 22 ++-- .../java/org/apache/tika/digest/DigestHelper.java | 36 +++++-- .../java/org/apache/tika/digest/DigestSink.java | 45 +++++--- .../main/java/org/apache/tika/digest/Digester.java | 12 +-- .../java/org/apache/tika/io/ByteArraySource.java | 12 ++- .../org/apache/tika/io/CachingInputStream.java | 9 ++ .../java/org/apache/tika/io/CachingSource.java | 10 ++ .../main/java/org/apache/tika/io/FileSource.java | 5 + .../java/org/apache/tika/io/ReopenableSource.java | 5 + .../main/java/org/apache/tika/io/StreamCache.java | 5 + .../java/org/apache/tika/io/TikaInputSource.java | 9 ++ .../java/org/apache/tika/io/TikaInputStream.java | 23 ++-- .../org/apache/tika/digest/DigestHelperTest.java | 26 +++++ .../org/apache/tika/digest/DigestSinkTest.java | 119 +++++++++++++++++---- .../apache/tika/digest/FailingTestTranslator.java | 13 ++- .../apache/tika/io/InMemoryContentViewTest.java | 74 +++++++++++++ .../org/apache/tika/metadata/TestMetadata.java | 119 +++++++++++++++++++++ ....apache.tika.extractor.EmbeddedStreamTranslator | 14 +++ tika-ml/tika-ml-junkdetect-tools/pom.xml | 5 +- .../ml/junkdetect/JunkFilterEncodingDetector.java | 11 +- .../tika/parser/ocr/tess4j/Tess4JConfig.java | 7 +- .../tika/parser/image/AbstractImageParser.java | 6 +- .../org/apache/tika/parser/image/ImageParser.java | 8 +- .../parser/image/ImageParsersNoTempFileTest.java | 30 ------ .../detect/microsoft/POIFSContainerDetector.java | 25 ++++- .../detect/microsoft/POIFSDeclaredSizeTest.java | 29 +++-- .../apache/tika/parser/ocr/ImagePreprocessor.java | 2 +- .../apache/tika/parser/ocr/TesseractOCRConfig.java | 4 - .../java/org/apache/tika/parser/pdf/PDFParser.java | 6 +- .../apache/tika/parser/pdf/PDFRandomAccess.java | 23 +++- .../tika/renderer/pdf/pdfbox/PDFBoxRenderer.java | 17 ++- .../apache/tika/pipes/core/FontCacheWarmer.java | 55 ++++++++++ .../tika/pipes/core/SharedServerModeTest.java | 2 + .../pipes/core/async/AsyncChaosMonkeyTest.java | 2 + .../org.junit.jupiter.api.extension.Extension | 15 +++ .../resources/configs/tika-config-passback.json | 2 +- .../resources/configs/tika-config-truncate.json | 61 ----------- .../resources/configs/tika-config-uppercasing.json | 2 +- .../configs/tika-config-write-limiter.json | 2 +- .../src/test/resources/junit-platform.properties | 17 +++ .../atlassianjwt/AtlassianJwtFetcherFactory.java | 4 +- .../tika/pipes/emitter/azblob/AZBlobEmitter.java | 4 +- .../pipes/emitter/azblob/AZBlobEmitterFactory.java | 4 +- .../pipes/fetcher/azblob/AZBlobFetcherFactory.java | 4 +- .../tika/pipes/emitter/es/ESEmitterFactory.java | 4 +- .../pipes/emitter/fs/FileSystemEmitterFactory.java | 4 +- .../pipes/fetcher/fs/FileSystemFetcherFactory.java | 4 +- .../apache/tika/pipes/emitter/gcs/GCSEmitter.java | 4 +- .../tika/pipes/emitter/gcs/GCSEmitterFactory.java | 4 +- .../tika/pipes/fetcher/gcs/GCSFetcherFactory.java | 4 +- .../googledrive/GoogleDriveFetcherFactory.java | 4 +- .../pipes/fetcher/http/HttpFetcherFactory.java | 4 +- .../tika/pipes/emitter/jdbc/JDBCEmitter.java | 4 +- .../pipes/emitter/jdbc/JDBCEmitterFactory.java | 4 +- .../tika/pipes/emitter/kafka/KafkaEmitter.java | 4 +- .../pipes/emitter/kafka/KafkaEmitterFactory.java | 4 +- .../MicrosoftGraphFetcherFactory.java | 4 +- .../opensearch/OpenSearchEmitterFactory.java | 4 +- .../apache/tika/pipes/emitter/s3/S3Emitter.java | 4 +- .../tika/pipes/emitter/s3/S3EmitterFactory.java | 4 +- .../tika/pipes/fetcher/s3/S3FetcherFactory.java | 4 +- .../tika/pipes/emitter/solr/SolrEmitter.java | 4 +- .../pipes/emitter/solr/SolrEmitterFactory.java | 4 +- .../server/core/TikaServerIntegrationTest.java | 4 +- 85 files changed, 887 insertions(+), 324 deletions(-) diff --cc CHANGES.txt index 4f886b4ba2,5ae3270114..e22631edfa --- a/CHANGES.txt +++ b/CHANGES.txt @@@ -1,32 -1,5 +1,27 @@@ 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. 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 ++ for file-backed input too. 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; new since 4.0.0, which had no budget at all) defaults to a quarter of the fork's heap, so raising diff --cc docs/modules/ROOT/pages/advanced/spooling.adoc index d3fc2d67ba,d3fc2d67ba..c66a4b191f --- a/docs/modules/ROOT/pages/advanced/spooling.adoc +++ b/docs/modules/ROOT/pages/advanced/spooling.adoc @@@ -49,16 -49,16 +49,36 @@@ Several file formats are most efficient The current architecture follows a simple principle: **each component that needs random access is responsible for obtaining it**. --A detector or parser is handed a `TikaInputStream`; when it needs random access --it simply asks for a file: ++A detector or parser is handed a `TikaInputStream`; when it needs random access it asks for ++a seekable view of the content and lets `TikaInputStream` decide whether that view is backed ++by memory or by a file: [source,java] ---- --Path path = tis.getPath(); --// or --File file = tis.getFile(); ++try (SeekableByteChannel channel = tis.getSeekableByteChannel()) { ++ ByteBuffer view = TikaInputStream.inMemoryContent(channel); ++ if (view != null) { ++ readInPlace(view); // in memory: no copy, no temp file ++ } else { ++ readFromFile(tis.getFile()); // spilled: the file already exists ++ } ++} ---- ++Two rules go with that form: ++ ++* The `ByteBuffer` aliases the cache's own array and is valid *only while that channel is ++ open*. Keep the channel open for as long as the view is in use. A view that outlives its ++ channel still reads correctly but is no longer counted against the memory budget. ++* `inMemoryContent()` returns `null` whenever the content is not in memory -- file-backed ++ input, or a cache that spilled. That is the normal case for large content, not an error. ++ ++`getFile()` remains correct when a consumer genuinely requires a `java.io.File` -- many ++third-party libraries do -- but it always materialises one. Prefer the channel/view form when ++the consumer can accept a buffer or a stream. For a worked example see ++`PDFRandomAccess.open(...)` in `tika-parser-pdf-module`, one factory that makes this choice ++for the PDF parser, its incremental-update scan, and the renderer. ++ `TikaInputStream` handles the spooling transparently based on how it was initialized: * **Initialized with `Path`**: The file is used directly for random access. No spooling needed. diff --cc docs/modules/ROOT/pages/pipes/performance.adoc index f35296d633,f35296d633..8f40cb5168 --- a/docs/modules/ROOT/pages/pipes/performance.adoc +++ b/docs/modules/ROOT/pages/pipes/performance.adoc @@@ -242,8 -242,8 +242,9 @@@ architecture suggested it would be * Several parsers and detectors asked for a `java.io.File` even when the document was already in memory: the JPEG/TIFF/WebP metadata extractors, the OLE2 container detector, the OpenDocument parser's inline pictures, the -- digest of translated embedded streams, and the PDF incremental-update scan -- each wrote the bytes out just to read them back. ++ digest of translated embedded streams, the PDF incremental-update scan, and ++ PDFParser's main document load and renderer each wrote the bytes out just to ++ read them back. On a spinning-disk host where the temp directory, the corpus, and the outputs share spindles, every temp byte is a seek taken away from a corpus read or an diff --cc tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/AbstractImageParser.java index 4ae61217c4,4ae61217c4..954649b2b0 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/AbstractImageParser.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/AbstractImageParser.java @@@ -18,7 -18,7 +18,6 @@@ 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.ContentHandler; @@@ -89,7 -89,7 +88,10 @@@ public abstract class AbstractImagePars XHTMLContentHandler xhtml = new XHTMLContentHandler(handler, metadata, context); xhtml.startDocument(); Path path = tis.getPath(); -- try (InputStream pathStream = Files.newInputStream(path)) { ++ // a TikaInputStream over the path, not a raw stream: the content is already on ++ // disk, so this takes the file path in extractMetadata instead of caching a ++ // second copy in memory whose budget reservation nothing here would release ++ try (TikaInputStream pathStream = TikaInputStream.get(path)) { extractMetadata(pathStream, new EmbeddedContentHandler(xhtml), metadata, context); } catch (SecurityException e) { throw e; 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 095d885b84,0000000000..9f970d28b5 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,173 -1,0 +1,143 @@@ +/* + * 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.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 a0500ae985,356dcba2f1..cb046e3780 --- 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 @@@ -597,107 -583,6 +597,124 @@@ public class POIFSContainerDetector imp } } + /** + * 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. ++ * object (it is stricter than the file loader on truncated objects). ++ * <p> ++ * With no budget in the context there is no accounting at all, and a byte[]-backed ++ * stream has no spill threshold to bound it either, so the copy is capped outright: ++ * above {@link #MAX_UNBUDGETED_COPY} the file path -- which is what 4.0.0 always did -- ++ * is used instead. + */ ++ /** ++ * Ceiling on POI's in-memory copy when no CacheMemoryBudget is present to account for it. ++ * Embedded OLE2 objects are typically a few hundred KB; past this the file loader's lazy ++ * block reads cost less than the heap. ++ */ ++ private static final long MAX_UNBUDGETED_COPY = 16L * 1024 * 1024; ++ + private Set<String> getTopLevelNamesInMemory(TikaInputStream stream, ParseContext context) + throws IOException { + CacheMemoryBudget budget = context == null ? null : context.get(CacheMemoryBudget.class); + // the budget decides how much the drain below keeps in memory + stream.enableRewind(budget); + POIFSFileSystem fs = null; + long reserved = 0; + try (SeekableByteChannel channel = stream.getSeekableByteChannel()) { + 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 == null) { ++ if (copy > MAX_UNBUDGETED_COPY) { ++ return null; ++ } ++ } else { + if (budget.tryReserve(copy) == 0) { + return null; + } + reserved = copy; + } ++ // POI reads from wherever the channel sits; do not rely on it being fresh ++ channel.position(0); + fs = new POIFSFileSystem(Channels.newInputStream(channel)); + Set<String> names = getTopLevelNames(fs.getRoot()); + stream.setOpenContainer(fs); ++ fs = null; // published: the stream owns it now, the finally must not close it + 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'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 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 -1; + } + } finally { + channel.position(start); + } + try { + 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 -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/POIFSDeclaredSizeTest.java index 6baedcb2f7,0000000000..8450153bec 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,137 -1,0 +1,154 @@@ +/* + * 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.assertNotNull; +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 + * 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 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")) { + 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 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.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 testTooShortForAHeaderIsRejected() throws Exception { + try (SeekableByteChannel channel = channelFor(new byte[16], "short.bin")) { + assertEquals(-1, POIFSContainerDetector.honestDeclaredSize(channel)); + } + } + + /** - * 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. ++ * End to end. NOTE: this asserts only that the crafted object does not become an open ++ * container and leaves nothing charged -- both of which also hold if the declared-size ++ * guard is deleted, because POI throws on the truncated read either way. The guard's ++ * real effect is the ~238MB POI would allocate first, and measuring that needs ++ * com.sun.management, which forbidden-apis bans. The guard's arithmetic and its input ++ * channel type are pinned by the unit tests above instead. + */ + @Test + public void testHostileHeaderDoesNotBecomeAnOpenContainer() throws Exception { + ParseContext context = new ParseContext(); + 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); ++ new POIFSContainerDetector().detect(tis, metadata, context); + assertNull(tis.getOpenContainer(), + "a header-only object must not be opened from memory"); + } - assertEquals(0, budget.getReservedBytes(), "nothing was ever reserved for it"); ++ assertEquals(0, budget.getReservedBytes(), "nothing left charged"); ++ } ++ ++ /** The channel type production actually uses is in-memory, not a file. */ ++ @Test ++ public void testLyingHeaderIsRejectedOverAnInMemoryChannel() throws Exception { ++ try (TemporaryResources tmp = new TemporaryResources()) { ++ TikaInputStream tis = TikaInputStream.get(new ByteArrayInputStream(header(3813)), tmp, ++ new Metadata()); ++ tis.enableRewind(null); ++ try (SeekableByteChannel channel = tis.getSeekableByteChannel()) { ++ assertNotNull(TikaInputStream.inMemoryContent(channel), "precondition: in memory"); ++ assertEquals(-1, POIFSContainerDetector.honestDeclaredSize(channel)); ++ } ++ } + } +} diff --cc tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/parser/pdf/PDFParser.java index bfdf538101,c61f33cc45..9267506822 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/parser/pdf/PDFParser.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/parser/pdf/PDFParser.java @@@ -494,18 -495,21 +494,22 @@@ public class PDFParser implements Parse ParseContext context) throws IOException, EncryptedDocumentException { try { - PDDocument pdDocument = null; if (tis.hasFile()) { // File based -- send file directly to PDFBox - pdDocument = - getPDDocument(tis.getPath(), password, streamCacheCreateFunction, metadata, context); - } else { - tis.setCloseShield(); + return getPDDocument(tis.getPath(), password, streamCacheCreateFunction, metadata, context); + } + // PDFBox owns the reader and closes it with the document + RandomAccessRead ra = PDFRandomAccess.open(tis, context); + try { + return getPDDocument(ra, password, streamCacheCreateFunction, metadata, context); + } catch (IOException | RuntimeException e) { - ra.close(); + try { - pdDocument = getPDDocumentFromStream(tis, password, - streamCacheCreateFunction, metadata, context); - } finally { - tis.removeCloseShield(); ++ ra.close(); ++ } catch (IOException closeFailure) { ++ e.addSuppressed(closeFailure); + } + throw e; } - return pdDocument; } catch (IOException e) { if (e.getMessage() != null && e.getMessage().contains("No security handler for filter")) { diff --cc tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/parser/pdf/PDFRandomAccess.java index 923f5043d4,0000000000..4db4a092f2 mode 100644,000000..100644 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/parser/pdf/PDFRandomAccess.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/parser/pdf/PDFRandomAccess.java @@@ -1,66 -1,0 +1,79 @@@ +/* + * 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.pdf; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.SeekableByteChannel; + +import org.apache.pdfbox.io.RandomAccessRead; +import org.apache.pdfbox.io.RandomAccessReadBuffer; +import org.apache.pdfbox.io.RandomAccessReadBufferedFile; + +import org.apache.tika.io.CacheMemoryBudget; +import org.apache.tika.io.TikaInputStream; +import org.apache.tika.parser.ParseContext; + +/** + * One way to hand PDFBox a document: in-memory content is read in place through a zero-copy + * view, anything else through PDFBox's buffered file reader. Neither copies the document, and + * every call starts from byte 0 regardless of the stream's position -- so a renderer can + * re-open the same document per page. Closing the returned reader releases the view. + */ +public final class PDFRandomAccess { + + private PDFRandomAccess() { + } + + public static RandomAccessRead open(TikaInputStream tis, ParseContext context) + throws IOException { + if (tis.hasFile()) { + return new RandomAccessReadBufferedFile(tis.getFile()); + } + tis.enableRewind(context == null ? null : context.get(CacheMemoryBudget.class)); + SeekableByteChannel channel = tis.getSeekableByteChannel(); - ByteBuffer view = TikaInputStream.inMemoryContent(channel); - if (view == null) { - // the drain spilled: the content is on disk now - channel.close(); - return new RandomAccessReadBufferedFile(tis.getFile()); ++ try { ++ ByteBuffer view = TikaInputStream.inMemoryContent(channel); ++ if (view == null) { ++ // the drain spilled: the content is on disk now ++ channel.close(); ++ return new RandomAccessReadBufferedFile(tis.getFile()); ++ } ++ return wrap(channel, view); ++ } catch (IOException | RuntimeException e) { ++ try { ++ channel.close(); ++ } catch (IOException closeFailure) { ++ e.addSuppressed(closeFailure); ++ } ++ throw e; + } ++ } ++ ++ private static RandomAccessRead wrap(SeekableByteChannel channel, ByteBuffer view) { + return new RandomAccessReadBuffer(view) { + @Override + public void close() throws IOException { + try { + super.close(); + } finally { + channel.close(); + } + } + }; + } +} diff --cc tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/renderer/pdf/pdfbox/PDFBoxRenderer.java index 20bc2117ef,8c5c94592a..7a421d47d2 --- a/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/renderer/pdf/pdfbox/PDFBoxRenderer.java +++ b/tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/renderer/pdf/pdfbox/PDFBoxRenderer.java @@@ -25,6 -25,7 +25,7 @@@ import java.util.Collections import java.util.Set; import org.apache.pdfbox.Loader; -import org.apache.pdfbox.io.RandomAccessReadBuffer; ++import org.apache.pdfbox.io.RandomAccessRead; import org.apache.pdfbox.pdmodel.PDDocument; import org.apache.pdfbox.rendering.ImageType; import org.apache.pdfbox.rendering.PDFRenderer; @@@ -100,9 -100,7 +101,21 @@@ public class PDFBoxRenderer implements if (tis.getOpenContainer() != null) { pdDocument = (PDDocument) tis.getOpenContainer(); } else { - pdDocument = Loader.loadPDF(new RandomAccessReadBuffer(tis)); + // a fresh reader from byte 0 each time, so per-page renders do not depend on - // where the stream was left; the document closes it - pdDocument = Loader.loadPDF(PDFRandomAccess.open(tis, parseContext)); ++ // where the stream was left; the document closes it -- but only once it exists, ++ // so a failed load must close the reader itself or its channel pin (and the ++ // budget behind it) leaks for the life of the stream ++ RandomAccessRead ra = PDFRandomAccess.open(tis, parseContext); ++ try { ++ pdDocument = Loader.loadPDF(ra); ++ } catch (IOException | RuntimeException e) { ++ try { ++ ra.close(); ++ } catch (IOException closeFailure) { ++ e.addSuppressed(closeFailure); ++ } ++ throw e; ++ } mustClose = true; } PageBasedRenderResults results = new PageBasedRenderResults(new TemporaryResources());
