#860: Fix regression in reading broken PackBits stream.

common/common-io/src/main/java/com/twelvemonkeys/io/SubStream.java

@@ -54,7 +54,7 @@ public final class SubStream extends FilterInputStream {
      */
     public SubStream(final InputStream stream, final long length) {
         super(Validate.notNull(stream, "stream"));
-        bytesLeft = length;
+        bytesLeft = Validate.isTrue(length >= 0, length, "length < 0: %s");
     }
 
     /**
@@ -63,10 +63,11 @@ public final class SubStream extends FilterInputStream {
      */
     @Override
     public void close() throws IOException {
-        // NOTE: Do not close the underlying stream
+        // NOTE: Do not close the underlying stream, but consume it
         while (bytesLeft > 0) {
-            //noinspection ResultOfMethodCallIgnored
-            skip(bytesLeft);
+            if (skip(bytesLeft) <= 0 && read() < 0) {
+                break;
+            }
         }
     }
 

common/common-io/src/test/java/com/twelvemonkeys/io/SubStreamTest.java

@@ -0,0 +1,114 @@
+package com.twelvemonkeys.io;
+
+import org.junit.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Arrays;
+import java.util.Random;
+
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+
+/**
+ * SubStreamTest.
+ *
+ * @author <a href="mailto:harald.kuhr@gmail.com">Harald Kuhr</a>
+ * @author last modified by $Author: haraldk$
+ * @version $Id: SubStreamTest.java,v 1.0 07/11/2023 haraldk Exp$
+ */
+public class SubStreamTest {
+
+    private final Random rng = new Random(2918475687L);
+
+    @SuppressWarnings("resource")
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateNullStream() {
+        new SubStream(null, 42);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateNegativeLength() {
+        new SubStream(new ByteArrayInputStream(new byte[1]), -1);
+    }
+
+    @Test
+    public void testReadAll() throws IOException {
+        byte[] buf = new byte[128];
+        rng.nextBytes(buf);
+
+        try (InputStream stream = new SubStream(new ByteArrayInputStream(buf), buf.length)) {
+            for (byte b : buf) {
+                assertEquals(b, (byte) stream.read());
+            }
+
+            assertEquals(-1, stream.read());
+        }
+    }
+
+    @Test
+    public void testReadAllArray() throws IOException {
+        byte[] buf = new byte[128];
+        rng.nextBytes(buf);
+
+        try (InputStream stream = new SubStream(new ByteArrayInputStream(buf), buf.length)) {
+            byte[] temp = new byte[buf.length / 4];
+            for (int i = 0; i < 4; i++) {
+                assertEquals(temp.length, stream.read(temp)); // Depends on ByteArrayInputStream specifics...
+                assertArrayEquals(Arrays.copyOfRange(buf, i * temp.length, (i + 1) * temp.length), temp);
+            }
+
+            assertEquals(-1, stream.read());
+        }
+    }
+
+    @Test
+    public void testSkipAll() throws IOException {
+        byte[] buf = new byte[128];
+
+        try (InputStream stream = new SubStream(new ByteArrayInputStream(buf), buf.length)) {
+            assertEquals(128, stream.skip(buf.length)); // Depends on ByteArrayInputStream specifics...
+            assertEquals(-1, stream.read());
+        }
+    }
+
+    @SuppressWarnings("EmptyTryBlock")
+    @Test
+    public void testCloseConsumesAll() throws IOException {
+        ByteArrayInputStream stream = new ByteArrayInputStream(new byte[128]);
+
+        try (InputStream ignore = new SubStream(stream, 128)) {
+            // Nothing here...
+        }
+
+        assertEquals(0, stream.available());
+        assertEquals(-1, stream.read());
+    }
+
+    @SuppressWarnings("EmptyTryBlock")
+    @Test
+    public void testCloseConsumesAllLongStream() throws IOException {
+        ByteArrayInputStream stream = new ByteArrayInputStream(new byte[256]);
+
+        try (InputStream ignore = new SubStream(stream, 128)) {
+            // Nothing here...
+        }
+
+        assertEquals(128, stream.available());
+        assertEquals(0, stream.read());
+    }
+
+    @SuppressWarnings("EmptyTryBlock")
+    @Test(timeout = 500L)
+    public void testCloseConsumesAllShortStream() throws IOException {
+        ByteArrayInputStream stream = new ByteArrayInputStream(new byte[13]);
+
+        try (InputStream ignore = new SubStream(stream, 42)) {
+            // Nothing here...
+        }
+
+        assertEquals(0, stream.available());
+        assertEquals(-1, stream.read());
+    }
+}

imageio/imageio-psd/src/test/java/com/twelvemonkeys/imageio/plugins/psd/PSDImageReaderTest.java

@@ -47,6 +47,7 @@ import javax.imageio.stream.ImageInputStream;
 import java.awt.*;
 import java.awt.color.*;
 import java.awt.image.*;
+import java.io.EOFException;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -695,6 +696,30 @@ public class PSDImageReaderTest extends ImageReaderAbstractTest<PSDImageReader>
         }
     }
 
+    @Test(timeout = 1000)
+    public void testBrokenPackBitsThrowsEOFException() throws IOException {
+        PSDImageReader imageReader = createReader();
+
+        try (ImageInputStream stream = ImageIO.createImageInputStream(getClassLoaderResource("/broken-psd/short-packbits.psd"))) {
+            imageReader.setInput(stream);
+
+            assertEquals(1, imageReader.getNumImages(true));
+
+            assertEquals(427, imageReader.getWidth(0));
+            assertEquals(107, imageReader.getHeight(0));
+
+            try {
+                imageReader.read(0);
+
+                fail("Expected EOFException, is the test broken?");
+            }
+            catch (EOFException expected) {
+                assertTrue(expected.getMessage().contains("PackBits"));
+            }
+        }
+    }
+
+
     final static class FakeCMYKColorSpace extends ColorSpace {
         FakeCMYKColorSpace() {
             super(ColorSpace.TYPE_CMYK, 4);