BOMInputStream.read() should return -1 after the stream is closed

Author: garydgregory

src/changes/changes.xml

@@ -70,6 +70,7 @@ The <action> type attribute can be add,update,fix,remove.
       <action dev="ggregory" type="add"                due-to="Gary Gregory">InfiniteCircularInputStream.available() should return 0 when the stream is closed.</action>
       <action dev="ggregory" type="add"                due-to="Gary Gregory">ChecksumInputStream(InputStream, Checksum, long, long) should fail-fast on null Checksum input.</action>
       <action dev="ggregory" type="add"                due-to="Gary Gregory">BufferedFileChannelInputStream.read() should return -1 after the stream is closed.</action>
+      <action dev="ggregory" type="add"                due-to="Gary Gregory">BOMInputStream.read() should return -1 after the stream is closed.</action>
       <!-- UPDATE -->
       <action dev="ggregory" type="update"             due-to="Dependabot">Bump tests commons.bytebuddy.version from 1.14.13 to 1.14.17 #615, #621, #631, #635.</action>
       <action dev="ggregory" type="update"             due-to="Dependabot">Bump tests commons-codec:commons-codec from 1.16.1 to 1.17.0.</action>

src/main/java/org/apache/commons/io/input/BOMInputStream.java

@@ -419,6 +419,9 @@ private boolean matches(final ByteOrderMark bom) {
      */
     @Override
     public int read() throws IOException {
+        if (isClosed()) {
+            return EOF;
+        }
         final int b = readFirstBytes();
         return b >= 0 ? b : in.read();
     }

src/main/java/org/apache/commons/io/input/ProxyInputStream.java

@@ -39,20 +39,22 @@
  * <li>notify a subclass that an exception was caught through {@link #handleIOException(IOException)}</li>
  * <li>{@link #unwrap()} itself</li>
  * </ul>
- * <p>
- * This class does not add any state (no additional instance variables).
- * </p>
  */
 public abstract class ProxyInputStream extends FilterInputStream {
 
+    /**
+     * Tracks whether {@link #close()} has been called or not.
+     */
+    private boolean closed;
+
     /**
      * Constructs a new ProxyInputStream.
      *
      * @param proxy  the InputStream to delegate to
      */
     public ProxyInputStream(final InputStream proxy) {
-        super(proxy);
         // the proxy is stored in a protected superclass variable named 'in'
+        super(proxy);
     }
 
     /**
@@ -87,7 +89,7 @@ protected void afterRead(final int n) throws IOException {
      */
     @Override
     public int available() throws IOException {
-        if (in != null) {
+        if (in != null && !isClosed()) {
             try {
                 return in.available();
             } catch (final IOException e) {
@@ -130,6 +132,7 @@ protected void beforeRead(final int n) throws IOException {
     @Override
     public void close() throws IOException {
         IOUtils.close(in, this::handleIOException);
+        closed = true;
     }
 
     /**
@@ -147,6 +150,15 @@ protected void handleIOException(final IOException e) throws IOException {
         throw e;
     }
 
+    /**
+     * Tests whether this instance is closed.
+     *
+     * @return whether this instance is closed.
+     */
+    boolean isClosed() {
+        return closed;
+    }
+
     /**
      * Invokes the delegate's {@code mark(int)} method.
      *

src/test/java/org/apache/commons/io/input/BOMInputStreamTest.java

@@ -37,6 +37,7 @@
 import javax.xml.parsers.ParserConfigurationException;
 
 import org.apache.commons.io.ByteOrderMark;
+import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.SystemProperties;
 import org.junit.jupiter.api.Test;
 import org.w3c.dom.Document;
@@ -207,13 +208,35 @@ private void readFile(final BOMInputStream bomInputStream) throws Exception {
     }
 
     @Test
-    public void testAvailableWithBOM() throws Exception {
+    public void testAvailableWithBOMAfterOpen() throws Exception {
         final byte[] data = { 'A', 'B', 'C', 'D' };
         try (InputStream in = BOMInputStream.builder().setInputStream(createUtf8Input(data, true)).get()) {
             assertEquals(7, in.available());
         }
     }
 
+    @Test
+    public void testAvailableWithBOMAfterClose() throws Exception {
+        final byte[] data = { 'A', 'B', 'C', 'D' };
+        final InputStream shadow;
+        try (InputStream in = BOMInputStream.builder().setInputStream(createUtf8Input(data, true)).get()) {
+            assertEquals(7, in.available());
+            shadow = in;
+        }
+        assertEquals(0, shadow.available());
+    }
+
+    @Test
+    public void testReadAfterClose() throws Exception {
+        final byte[] data = { 'A', 'B', 'C', 'D' };
+        final InputStream shadow;
+        try (InputStream in = BOMInputStream.builder().setInputStream(createUtf8Input(data, true)).get()) {
+            assertEquals(7, in.available());
+            shadow = in;
+        }
+        assertEquals(IOUtils.EOF, shadow.read());
+    }
+
     @Test
     public void testAvailableWithoutBOM() throws Exception {
         final byte[] data = { 'A', 'B', 'C', 'D' };

src/test/java/org/apache/commons/io/input/ProxyInputStreamTest.java

@@ -56,8 +56,14 @@ protected InputStream createProxySource() {
         return CharSequenceInputStream.builder().setCharSequence("abc").get();
     }
 
-    protected void testEos(final T inputStream) {
-        // empty
+    @SuppressWarnings("resource")
+    @Test
+    public void testAvailableAfterClose() throws IOException {
+        final T shadow;
+        try (T inputStream = createFixture()) {
+            shadow = inputStream;
+        }
+        assertEquals(0, shadow.available());
     }
 
     @Test
@@ -82,6 +88,10 @@ public void testAvailableNull() throws IOException {
         }
     }
 
+    protected void testEos(final T inputStream) {
+        // empty
+    }
+
     @Test
     public void testRead() throws IOException {
         try (T inputStream = createFixture()) {
@@ -97,6 +107,16 @@ public void testRead() throws IOException {
         }
     }
 
+    @SuppressWarnings("resource")
+    @Test
+    public void testReadAfterClose() throws IOException {
+        final T shadow;
+        try (T inputStream = createFixture()) {
+            shadow = inputStream;
+        }
+        assertEquals(IOUtils.EOF, shadow.read());
+    }
+
     @Test
     public void testReadArrayAtMiddleFully() throws IOException {
         try (T inputStream = createFixture()) {