You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tika.apache.org by ta...@apache.org on 2026/08/27 10:41:47 UTC
(tika) branch TIKA-4845-poifs-declared-size created (now 0cfd43b5fa)
This is an automated email from the ASF dual-hosted git repository.
tballison pushed a change to branch TIKA-4845-poifs-declared-size
in repository https://gitbox.apache.org/repos/asf/tika.git
at 0cfd43b5fa TIKA-4845 - improve BAT estimation
This branch includes the following new commits:
new 0cfd43b5fa TIKA-4845 - improve BAT estimation
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(tika) 01/01: TIKA-4845 - improve BAT estimation
Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
tballison pushed a commit to branch TIKA-4845-poifs-declared-size
in repository https://gitbox.apache.org/repos/asf/tika.git
commit 0cfd43b5fa88578fae5ff8ee0796f032e28b3deb
Author: tallison <ta...@apache.org>
AuthorDate: Thu Aug 27 06:40:06 2026 -0400
TIKA-4845 - improve BAT estimation
---
CHANGES.txt | 3 +
.../detect/microsoft/POIFSContainerDetector.java | 36 ++++--
.../detect/microsoft/POIFSDeclaredSizeTest.java | 131 +++++++++++++++++++--
3 files changed, 153 insertions(+), 17 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index d517042ccb..ff2906b2b3 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,5 +1,8 @@
Release 4.1.0 - unreleased
+ * Stop spooling OLE2 objects whose header over-reserves BAT capacity
+ (TIKA-4845).
+
* Add Micrometer reporting and opt-in endpoint for tika-server (TIKA-4839).
* Improve spooling/decrease number of spills to disk (TIKA-4835).
diff --git 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
index cb046e3780..7438e94f44 100644
--- 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
@@ -605,6 +605,11 @@ public class POIFSContainerDetector implements Detector {
* 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).
* <p>
+ * None of this would be needed if POI could be handed the bytes it already has. Its API
+ * takes a File or FileChannel (no copy) or an InputStream (copy sized by the header) with
+ * nothing in between, and POIFSFileSystem._data is protected while readCoreContents() is
+ * private, so a subclass cannot supply a ByteArrayBackedDataSource either.
+ * <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 --
@@ -617,6 +622,16 @@ public class POIFSContainerDetector implements Detector {
*/
private static final long MAX_UNBUDGETED_COPY = 16L * 1024 * 1024;
+ /**
+ * BAT blocks a small object may reserve regardless of its length. Every valid header
+ * declares at least one, and writers round up; the unit is the sector size, so this is
+ * 256KB for 512-byte sectors and 16MB for 4K ones.
+ */
+ private static final int MIN_DECLARED_BAT_BLOCKS = 4;
+
+ /** Past this multiple of the bytes in hand the header is not describing this object. */
+ private static final int MAX_DECLARED_AMPLIFICATION = 16;
+
private Set<String> getTopLevelNamesInMemory(TikaInputStream stream, ParseContext context)
throws IOException {
CacheMemoryBudget budget = context == null ? null : context.get(CacheMemoryBudget.class);
@@ -674,10 +689,15 @@ public class POIFSContainerDetector implements Detector {
/**
* 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.
+ * an implausible multiple of the bytes in hand.
+ * <p>
+ * The bound is on amplification, over a floor of a few BAT blocks. Real writers reserve
+ * BAT capacity ahead of use -- SolidWorks declares 26 BAT blocks where 16 would do, and
+ * small objects routinely declare several times their own length -- so "one BAT block of
+ * slack" rejects valid files. The floor is in sectors rather than bytes because a 4K-sector
+ * object cannot declare less than 4MB even when it is nearly empty. What must not get
+ * through is a 512-byte object declaring hundreds of MB, four orders of magnitude past
+ * anything a writer produces.
*/
static long honestDeclaredSize(SeekableByteChannel channel) throws IOException {
byte[] header = new byte[POIFSConstants.SMALLER_BIG_BLOCK_SIZE];
@@ -698,10 +718,12 @@ public class POIFSContainerDetector implements Detector {
HeaderBlock hb = new HeaderBlock(
UnsynchronizedByteArrayInputStream.builder().setByteArray(header).get());
long declared = BATBlock.calculateMaximumSize(hb);
- long oneBatSpan = (long) hb.getBigBlockSize().getBigBlockSize() *
- hb.getBigBlockSize().getBATEntriesPerBlock();
+ long sector = hb.getBigBlockSize().getBigBlockSize();
+ long batSpan = sector * hb.getBigBlockSize().getBATEntriesPerBlock();
long actual = channel.size();
- return declared > actual + oneBatSpan ? -1 : declared;
+ long ceiling = Math.max(sector + MIN_DECLARED_BAT_BLOCKS * batSpan,
+ actual * MAX_DECLARED_AMPLIFICATION);
+ return declared > ceiling ? -1 : declared;
} catch (IOException | RuntimeException e) {
return -1;
}
diff --git 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
index 8450153bec..a002d71a2b 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
@@ -17,6 +17,7 @@
package org.apache.tika.detect.microsoft;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -32,6 +33,8 @@ import java.nio.file.StandardOpenOption;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
import org.apache.tika.TikaTest;
import org.apache.tika.io.CacheMemoryBudget;
@@ -43,8 +46,8 @@ 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.
+ * refuses a header that declares an implausible multiple of the bytes in hand, and reserves
+ * what it does believe from the budget before POI allocates it.
*/
public class POIFSDeclaredSizeTest extends TikaTest {
@@ -56,15 +59,27 @@ public class POIFSDeclaredSizeTest extends TikaTest {
/** A bare 512-byte OLE2 header declaring {@code batCount} BAT blocks and nothing else. */
private static byte[] header(int batCount) {
+ return header(batCount, 9);
+ }
+
+ /** As above, with an explicit sector shift: 9 for 512-byte sectors, 12 for 4K. */
+ private static byte[] header(int batCount, int sectorShift) {
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
+ data[SECTOR_SHIFT_OFFSET] = (byte) sectorShift;
ByteBuffer.wrap(data).order(ByteOrder.LITTLE_ENDIAN).putInt(BAT_COUNT_OFFSET, batCount);
return data;
}
+ /** The same header followed by zeros out to {@code totalLen}. */
+ private static byte[] headerPaddedTo(int batCount, int totalLen) {
+ byte[] data = new byte[totalLen];
+ System.arraycopy(header(batCount), 0, data, 0, 512);
+ return data;
+ }
+
private SeekableByteChannel channelFor(byte[] bytes, String name) throws Exception {
Path p = tempDir.resolve(name);
Files.write(p, bytes);
@@ -82,18 +97,87 @@ public class POIFSDeclaredSizeTest extends TikaTest {
}
}
+ /**
+ * Small objects may reserve up to four BAT blocks regardless of length: at that size the
+ * ratio to a few hundred bytes of header says nothing about whether the header is honest.
+ */
+ @Test
+ public void testSmallDeclarationsAreBelievedUpToTheBatFloor() throws Exception {
+ // 4 BAT blocks == 262_656 bytes declared by 512 bytes of content: exactly the floor
+ try (SeekableByteChannel channel = channelFor(header(4), "at-floor.ole")) {
+ assertEquals((1 + 4 * 128) * 512L,
+ POIFSContainerDetector.honestDeclaredSize(channel));
+ }
+ // 5 BAT blocks == 328_192: the first value past it
+ try (SeekableByteChannel channel = channelFor(header(5), "over-floor.ole")) {
+ assertEquals(-1, POIFSContainerDetector.honestDeclaredSize(channel));
+ }
+ }
+
+ /**
+ * The floor is counted in sectors, not bytes. A 4K-sector object cannot declare less than
+ * one 4MB BAT block however empty it is, so a byte-valued floor tuned for 512-byte sectors
+ * would reject every small one of them -- a case no file in the test corpus exercises.
+ */
@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));
+ public void testTheBatFloorScalesWithSectorSize() throws Exception {
+ // 4 BAT blocks of 4K sectors == 16_781_312 bytes: the floor, not a byte constant
+ try (SeekableByteChannel channel = channelFor(header(4, 12), "4k-at-floor.ole")) {
+ assertEquals((1 + 4 * 1024) * 4096L,
+ 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")) {
+ try (SeekableByteChannel channel = channelFor(header(5, 12), "4k-over-floor.ole")) {
assertEquals(-1, POIFSContainerDetector.honestDeclaredSize(channel));
}
}
+ /** Past the floor the bound is a multiple of the bytes actually in hand. */
+ @Test
+ public void testLargeDeclarationsAreBoundedByAmplification() throws Exception {
+ int actual = 128 * 1024; // ceiling is 16x this == 2_097_152
+ // 31 BAT blocks == 2_032_128 declared: inside the ceiling
+ try (SeekableByteChannel channel =
+ channelFor(headerPaddedTo(31, actual), "under-ceiling.ole")) {
+ assertEquals((1 + 31 * 128) * 512L,
+ POIFSContainerDetector.honestDeclaredSize(channel));
+ }
+ // 32 BAT blocks == 2_097_664 declared: the first value past it
+ try (SeekableByteChannel channel =
+ channelFor(headerPaddedTo(32, actual), "over-ceiling.ole")) {
+ assertEquals(-1, POIFSContainerDetector.honestDeclaredSize(channel));
+ }
+ }
+
+ /**
+ * The regression this bound exists to avoid: real writers reserve BAT capacity ahead of
+ * use, so these all declare well past their own length -- SolidWorks by 26 BAT blocks
+ * where 16 would do, the encrypted workbook by 12x. Under a one-BAT-block rule every one
+ * of them fell back to spooling the object to a temp file.
+ */
+ @ParameterizedTest
+ @ValueSource(strings = {
+ "testEXCEL_protected_passtika_2.xlsx",
+ "testPPT_comment.ppt",
+ "testPPT_macros.ppt",
+ "testPPT_oleWorkbook.ppt",
+ "testsolidworksAssembly2014SP0.SLDASM",
+ "testsolidworksDrawing2014SP0.SLDDRW",
+ "testsolidworksPart2013SP2.SLDPRT",
+ "testsolidworksPart2014SP0.SLDPRT"})
+ public void testOverDeclaringRealFilesAreBelieved(String name) throws Exception {
+ byte[] bytes;
+ try (InputStream is = getResourceAsStream("/test-documents/" + name)) {
+ bytes = is.readAllBytes();
+ }
+ try (SeekableByteChannel channel = channelFor(bytes, name)) {
+ long declared = POIFSContainerDetector.honestDeclaredSize(channel);
+ assertTrue(declared > 0, name + " must not be rejected: declared " + declared +
+ " against " + bytes.length + " bytes");
+ assertTrue(declared > bytes.length,
+ name + " is only a regression fixture while it over-declares");
+ }
+ }
+
@Test
public void testRealDocumentHeaderIsHonest() throws Exception {
byte[] bytes;
@@ -102,7 +186,7 @@ public class POIFSDeclaredSizeTest extends TikaTest {
}
try (SeekableByteChannel channel = channelFor(bytes, "real.doc")) {
long declared = POIFSContainerDetector.honestDeclaredSize(channel);
- assertTrue(declared >= bytes.length && declared <= bytes.length + 128 * 512L,
+ assertTrue(declared >= bytes.length && declared <= bytes.length * 16L,
"a real header declares about its own size: " + declared + " vs " + bytes.length);
}
}
@@ -114,6 +198,33 @@ public class POIFSDeclaredSizeTest extends TikaTest {
}
}
+ /**
+ * End to end, and the point of the whole change: an over-declaring file detected from
+ * memory is opened in memory and never touches disk. Before the amplification bound these
+ * fell to the file path, which materialised a temp file for every one of them.
+ */
+ @ParameterizedTest
+ @ValueSource(strings = {"testPPT_comment.ppt", "testsolidworksPart2013SP2.SLDPRT"})
+ public void testOverDeclaringFileIsOpenedFromMemory(String name) throws Exception {
+ byte[] bytes;
+ try (InputStream is = getResourceAsStream("/test-documents/" + name)) {
+ bytes = is.readAllBytes();
+ }
+ 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(bytes), tmp, metadata);
+ new POIFSContainerDetector().detect(tis, metadata, context);
+ assertNotNull(tis.getOpenContainer(), name + " must be opened from memory");
+ assertFalse(tis.hasFile(), name + " must not have been spooled to disk");
+ assertTrue(budget.getReservedBytes() > 0, "the copy must be charged while open");
+ }
+ assertEquals(0, budget.getReservedBytes(), "released when the stream closes");
+ }
+
/**
* 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