TIKA-4878: never treat a declared Content-Length as a measurement (#3131)

Author: tballison

CHANGES.txt

@@ -1,5 +1,12 @@
 Release 4.1.0 - unreleased
 
+   * A declared Content-Length is no longer treated as a measurement: the
+     zip-bomb ratio counts only measured input bytes (a container-declared
+     size on an embedded document could inflate its denominator), and a
+     re-openable source no longer reserves cache budget or sizes its buffer
+     from the declared length (a lying one could push a small payload to
+     disk or churn the shared budget). Neither is in a release: the
+     exposure arrived with TIKA-4868 and TIKA-4873 (TIKA-4878).
    * Embedded objects in Office documents are re-opened from their container
      instead of cached: every OOXML part (pictures, media, attachments), the
      OLE 2.0 package inside an OOXML part, the CONTENTS entry of an OLE 2.0

tika-core/src/main/java/org/apache/tika/io/ReopenableSource.java

@@ -221,23 +221,19 @@ private void maybeReleaseRetained() {
     /**
      * Drains a fresh stream into memory if it fits within the per-object floor plus what
      * can be reserved from the shared budget, retaining the buffer (and its reservation)
-     * until {@link #close()}. The declared length is a sizing hint only -- it can lie, so
-     * the cap is enforced during the read. Does not disturb this source's read position.
+     * until {@link #close()}. The declared length is the file's claim: it may skip an
+     * attempt that cannot succeed (a lie there costs a spill, nothing more), but it never
+     * sizes a reservation or an allocation -- the buffer starts at the floor at most and
+     * grows, reserving, on what is actually read. Does not disturb this source's read
+     * position.
      */
     private boolean tryBufferInMemory() throws IOException {
         if (length > MAX_ARRAY_SIZE || (length > IN_MEMORY_FLOOR && budget == null)) {
             return false;
         }
         long reservedHere = 0;
         // Reservation invariant: reservedHere == max(0, data.length - IN_MEMORY_FLOOR)
-        if (length > IN_MEMORY_FLOOR) {
-            long extra = length - IN_MEMORY_FLOOR;
-            if (budget.tryReserve(extra) != extra) {
-                return false;
-            }
-            reservedHere = extra;
-        }
-        byte[] data = new byte[length > 0 ? (int) length : 8192];
+        byte[] data = new byte[(int) Math.max(8192, Math.min(length, IN_MEMORY_FLOOR))];
         int total = 0;
         boolean fits = false;
         try (InputStream in = opener.get()) {

tika-core/src/main/java/org/apache/tika/sax/SecureContentHandler.java

@@ -212,7 +212,9 @@ public void throwIfCauseOf(SAXException e) throws TikaException {
 
     private long getByteCount() throws SAXException {
         try {
-            if (stream.hasLength()) {
+            //a declared Content-Length is the file's claim; only a measured length
+            //or the bytes actually read can serve as the ratio's denominator
+            if (stream.hasReliableLength()) {
                 return stream.getLength();
             } else {
                 return stream.getPosition();

tika-core/src/test/java/org/apache/tika/io/ReopenableSourceTest.java

@@ -245,6 +245,29 @@ public void testChannelBudgetExhaustedSpills() throws Exception {
         }
     }
 
+    /**
+     * A declared length far above the content, against a budget that could never
+     * grant it: the claim must not be what gets reserved, or a 500-byte payload is
+     * pushed to disk by a number the file made up.
+     */
+    @Test
+    public void testLyingDeclaredLengthDoesNotReserveOrSpill() throws Exception {
+        byte[] data = data(500);
+        AtomicInteger opens = new AtomicInteger();
+        CacheMemoryBudget budget = new CacheMemoryBudget(1024);
+        try (ReopenableSource source = new ReopenableSource(countingOpener(data, opens), tmp,
+                50L * 1024 * 1024, null)) {
+            source.enableRewind(budget);
+            try (SeekableByteChannel channel = source.getSeekableByteChannel()) {
+                assertInstanceOf(MemorySeekableByteChannel.class, channel);
+                assertArrayEquals(data, readFully(channel));
+            }
+            assertFalse(source.hasPath());
+            assertEquals(0, budget.getReservedBytes(), "nothing reserved for a 500 byte payload");
+            assertEquals(500, source.getLength());
+        }
+    }
+
     @Test
     public void testLyingDeclaredLengthCorrected() throws Exception {
         byte[] data = data(500);

tika-core/src/test/java/org/apache/tika/sax/SecureContentHandlerTest.java

@@ -29,7 +29,10 @@
 import org.xml.sax.helpers.DefaultHandler;
 
 import org.apache.tika.exception.TikaException;
+import org.apache.tika.io.TemporaryResources;
 import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.HttpHeaders;
+import org.apache.tika.metadata.Metadata;
 
 /**
  * Tests for the {@link SecureContentHandler} class.
@@ -108,6 +111,31 @@ public void testManyCharactersPerByte() throws IOException {
         }
     }
 
+    /**
+     * A one-shot stream carrying a declared Content-Length far above what it holds:
+     * the claim must not become the input byte count, or the ratio never trips.
+     */
+    @Test
+    public void testDeclaredLengthDoesNotInflateByteCount() throws IOException {
+        Metadata metadata = new Metadata();
+        //a plausible lie: large enough that no ratio trips, small enough that
+        //byteCount * ratio does not overflow and trip for the wrong reason
+        metadata.set(HttpHeaders.CONTENT_LENGTH, Long.toString(100L * 1024 * 1024 * 1024));
+        try (TikaInputStream lying = TikaInputStream.get(new NullInputStream(MANY_BYTES),
+                new TemporaryResources(), metadata)) {
+            SecureContentHandler lyingHandler =
+                    new SecureContentHandler(new DefaultHandler(), lying);
+            char[] ch = new char[1000];
+            for (int i = 0; i < MANY_BYTES; i++) {
+                lying.read();
+                lyingHandler.characters(ch, 0, ch.length);
+            }
+            fail("Expected SAXException not thrown");
+        } catch (SAXException e) {
+            // expected
+        }
+    }
+
     @Test
     public void testSomeCharactersWithoutInput() throws IOException {
         try {