fix: respect read limit in BoundedInputStream.available() (#779)

ppkarwasz · garydgregory · web-flow · commit 86119d3c3377 · 2025-09-08T11:02:22.000-04:00
* fix: correct `BoundedInputStream.getRemaining()` for unbounded streams

`BoundedInputStream.getRemaining()` previously returned `0` when no read limit was set, which was misleading.
It now returns `Long.MAX_VALUE` to more clearly represent an unbounded stream.

The Javadoc was also updated to clarify that the method reflects only the configured limit and does **not** report the actual number of bytes available in the underlying stream.

* fix: respect read limit in `BoundedInputStream.available()`

`BoundedInputStream.available()` previously returned the underlying stream’s availability without applying the configured read limit. It now caps the reported value by the remaining bytes allowed by the bound.

Additional changes:

* Removed the `onMaxLength` call from `available()`, since `available()` does not consume data and cannot exceed the maximum count.
* Reworked the `testAvailableAfterClose` and `testReadAfterClose` tests:

  * Old versions relied too heavily on `CharSequenceInputStream`’s specific behavior.
  * New parameterized tests cover all three typical cases of the underlying stream after close: no-op, return special value, or throw an exception.
* Added a dedicated unit test for the corrected `available()` behavior.

* fix: formatting

* fix: Javadoc

* fix: Tests

* fix: test descriptions

* fix: replace toIntExact with cast

* Use consistent wording

* Use consistent wording

---------

Co-authored-by: Gary Gregory &lt;garydgregory@users.noreply.github.com&gt;
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -50,6 +50,8 @@ The <action> type attribute can be add,update,fix,remove.
       <action type="fix" dev="ggregory"                due-to="Gary Gregory">When testing on Java 21 and up, enable -XX:+EnableDynamicAgentLoading.</action>
       <action type="fix" dev="ggregory"                due-to="Gary Gregory">When testing on Java 24 and up, don't fail FileUtilsListFilesTest for a different behavior in the JRE.</action>
       <action type="fix" dev="ggregory"                due-to="Stanislav Fort, Gary Gregory">ValidatingObjectInputStream does not validate dynamic proxy interfaces.</action>
+      <action type="fix" dev="pkarwasz"                due-to="Piotr P. Karwasz">BoundedInputStream.getRemaining() now reports Long.MAX_VALUE instead of 0 when no limit is set.</action>
+      <action type="fix" dev="pkarwasz"                due-to="Piotr P. Karwasz">BoundedInputStream.available() correctly accounts for the maximum read limit.</action>
       <!-- ADD -->
       <action dev="ggregory" type="add"                due-to="strangelookingnerd, Gary Gregory">FileUtils#byteCountToDisplaySize() supports Zettabyte, Yottabyte, Ronnabyte and Quettabyte #763.</action>
       <action dev="ggregory" type="add"                due-to="strangelookingnerd, Gary Gregory">Add org.apache.commons.io.FileUtils.ONE_RB #763.</action>
diff --git a/src/main/java/org/apache/commons/io/input/BoundedInputStream.java b/src/main/java/org/apache/commons/io/input/BoundedInputStream.java
@@ -370,16 +370,11 @@ protected synchronized void afterRead(final int n) throws IOException {
         super.afterRead(n);
     }
 
-    /**
-     * {@inheritDoc}
-     */
     @Override
     public int available() throws IOException {
-        if (isMaxCount()) {
-            onMaxLength(maxCount, getCount());
-            return 0;
-        }
-        return in.available();
+        // Safe cast: value is between 0 and Integer.MAX_VALUE
+        final int remaining = (int) Math.min(getRemaining(), Integer.MAX_VALUE);
+        return Math.min(super.available(), remaining);
     }
 
     /**
@@ -405,9 +400,9 @@ public synchronized long getCount() {
     }
 
     /**
-     * Gets the max count of bytes to read.
+     * Gets the maximum number of bytes to read.
      *
-     * @return The max count of bytes to read.
+     * @return The maximum number of bytes to read, or {@value IOUtils#EOF} if unbounded.
      * @since 2.16.0
      */
     public long getMaxCount() {
@@ -427,13 +422,21 @@ public long getMaxLength() {
     }
 
     /**
-     * Gets how many bytes remain to read.
+     * Gets the number of bytes remaining to read before the maximum is reached.
+     *
+     * <p>
+     * This method does <strong>not</strong> report the bytes available in the
+     * underlying stream; it only reflects the remaining allowance imposed by this
+     * {@code BoundedInputStream}.
+     * </p>
      *
-     * @return bytes how many bytes remain to read.
+     * @return The number of bytes remaining to read before the maximum is reached,
+     *         or {@link Long#MAX_VALUE} if no bound is set.
      * @since 2.16.0
      */
     public long getRemaining() {
-        return Math.max(0, getMaxCount() - getCount());
+        final long maxCount = getMaxCount();
+        return maxCount == EOF ? Long.MAX_VALUE : Math.max(0, maxCount - getCount());
     }
 
     private boolean isMaxCount() {
diff --git a/src/test/java/org/apache/commons/io/input/BoundedInputStreamTest.java b/src/test/java/org/apache/commons/io/input/BoundedInputStreamTest.java
@@ -23,18 +23,27 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertThrowsExactly;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Stream;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.test.CustomIOException;
 import org.apache.commons.lang3.mutable.MutableInt;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
 /**
@@ -80,21 +89,74 @@ void testAfterReadConsumer() throws Exception {
         // @formatter:on
     }
 
-    @SuppressWarnings("resource")
-    @Test
-    void testAvailableAfterClose() throws Exception {
+    static Stream<Arguments> testAvailableAfterClose() throws IOException {
+        // Case 1: behaves like ByteArrayInputStream — close() is a no-op, available() still returns a value (e.g., 42).
+        final InputStream noOpClose = mock(InputStream.class);
+        when(noOpClose.available()).thenReturn(42, 42);
+
+        // Case 2: returns 0 after close (Commons memory-backed streams that ignore close but report 0 when exhausted).
+        final InputStream returnsZeroAfterClose = mock(InputStream.class);
+        when(returnsZeroAfterClose.available()).thenReturn(42, 0);
+
+        // Case 3: throws IOException after close (e.g., FileInputStream-like behavior).
+        final InputStream throwsAfterClose = mock(InputStream.class);
+        when(throwsAfterClose.available()).thenReturn(42).thenThrow(new IOException("Stream closed"));
+
+        return Stream.of(
+                Arguments.of("underlying stream still returns 42 after close", noOpClose, 42),
+                Arguments.of("underlying stream returns 0 after close", returnsZeroAfterClose, 42),
+                Arguments.of("underlying stream throws IOException after close", throwsAfterClose, 42));
+    }
+
+    @ParameterizedTest(name = "{index} — {0}")
+    @MethodSource
+    void testAvailableAfterClose(String caseName, InputStream delegate, int expectedBeforeClose)
+            throws Exception {
         final InputStream shadow;
-        try (InputStream in = BoundedInputStream.builder().setCharSequence("Hi").get()) {
-            assertTrue(in.available() > 0);
-            shadow = in;
-        }
-        assertEquals(0, shadow.available());
+        try (InputStream in = BoundedInputStream.builder()
+                .setInputStream(delegate)
+                .setPropagateClose(true)
+                .get()) {
+            // Before close: pass-through behavior
+            assertEquals(expectedBeforeClose, in.available(), caseName + " (before close)");
+            shadow = in; // keep reference to call after close
+        }
+        // Verify the underlying stream was closed
+        verify(delegate, times(1)).close();
+        // After close: behavior depends on the underlying stream
+        assertEquals(0, shadow.available(), caseName + " (after close)");
+        // Interactions: available called only once before close.
+        verify(delegate, times(1)).available();
+        verifyNoMoreInteractions(delegate);
     }
 
-    @Test
-    void testAvailableAfterOpen() throws Exception {
-        try (InputStream in = BoundedInputStream.builder().setCharSequence("Hi").get()) {
-            assertTrue(in.available() > 0);
+    static Stream<Arguments> testAvailableUpperLimit() {
+        final byte[] helloWorld = "Hello World".getBytes(StandardCharsets.UTF_8);
+        return Stream.of(
+                // Limited by maxCount
+                Arguments.of(new ByteArrayInputStream(helloWorld), helloWorld.length - 1, helloWorld.length - 1, 0),
+                // Limited by data length
+                Arguments.of(new ByteArrayInputStream(helloWorld), helloWorld.length + 1, helloWorld.length, 0),
+                // Limited by Integer.MAX_VALUE
+                Arguments.of(
+                        new NullInputStream(Long.MAX_VALUE), Long.MAX_VALUE, Integer.MAX_VALUE, Integer.MAX_VALUE));
+    }
+
+    @ParameterizedTest
+    @MethodSource
+    void testAvailableUpperLimit(InputStream input, long maxCount, int expectedBeforeSkip, int expectedAfterSkip)
+            throws Exception {
+        try (BoundedInputStream bounded = BoundedInputStream.builder()
+                .setInputStream(input)
+                .setMaxCount(maxCount)
+                .get()) {
+            assertEquals(
+                    expectedBeforeSkip, bounded.available(), "available should be limited by maxCount and data length");
+            IOUtils.skip(bounded, expectedBeforeSkip);
+            assertEquals(
+                    expectedAfterSkip,
+                    bounded.available(),
+                    "after skipping available should be limited by maxCount and data length");
         }
     }
 
@@ -444,15 +506,58 @@ void testPublicConstructors() throws IOException {
         }
     }
 
-    @SuppressWarnings("resource")
-    @Test
-    void testReadAfterClose() throws Exception {
-        final InputStream shadow;
-        try (InputStream in = BoundedInputStream.builder().setCharSequence("Hi").get()) {
-            assertTrue(in.available() > 0);
-            shadow = in;
+    static Stream<Arguments> testReadAfterClose() throws IOException {
+        // Case 1: no-op close (ByteArrayInputStream-like): read() still returns a value after close
+        final InputStream noOpClose = mock(InputStream.class);
+        when(noOpClose.read()).thenReturn(42);
+
+        // Case 2: returns EOF (-1) after close
+        final InputStream returnsEofAfterClose = mock(InputStream.class);
+        when(returnsEofAfterClose.read()).thenReturn(IOUtils.EOF);
+
+        // Case 3: throws IOException after close (FileInputStream-like)
+        final InputStream throwsAfterClose = mock(InputStream.class);
+        final IOException closed = new IOException("Stream closed");
+        when(throwsAfterClose.read()).thenThrow(closed);
+
+        return Stream.of(
+                Arguments.of("underlying stream still reads data after close", noOpClose, 42),
+                Arguments.of("underlying stream returns EOF after close", returnsEofAfterClose, IOUtils.EOF),
+                Arguments.of("underlying stream throws IOException after close", throwsAfterClose, closed));
+    }
+
+    @ParameterizedTest(name = "{index} — {0}")
+    @MethodSource("testReadAfterClose")
+    void testReadAfterClose(
+            String caseName,
+            InputStream delegate,
+            Object expectedAfterClose // Integer (value) or IOException (expected thrown)
+            ) throws Exception {
+
+        final InputStream bounded;
+        try (InputStream in = BoundedInputStream.builder()
+                .setInputStream(delegate)
+                .setPropagateClose(true)
+                .get()) {
+            bounded = in; // call read() only after close
+        }
+
+        // Underlying stream should be closed exactly once
+        verify(delegate, times(1)).close();
+
+        if (expectedAfterClose instanceof Integer) {
+            assertEquals(expectedAfterClose, bounded.read(), caseName + " (after close)");
+        } else if (expectedAfterClose instanceof IOException) {
+            final IOException actual = assertThrows(IOException.class, bounded::read, caseName + " (after close)");
+            // verify it's the exact instance we configured
+            assertSame(expectedAfterClose, actual, caseName + " (exception instance)");
+        } else {
+            fail("Unexpected expectedAfterClose type: " + expectedAfterClose);
         }
-        assertEquals(IOUtils.EOF, shadow.read());
+
+        // We only performed one read() (after close)
+        verify(delegate, times(1)).read();
+        verifyNoMoreInteractions(delegate);
     }
 
     @Test
@@ -494,6 +599,46 @@ void testReadArray() throws Exception {
         }
     }
 
+    static Stream<Arguments> testRemaining() {
+        return Stream.of(
+                // Unbounded: any negative maxCount is treated as "no limit".
+                Arguments.of("unbounded (EOF constant)", IOUtils.EOF, Long.MAX_VALUE),
+                Arguments.of("unbounded (arbitrary negative)", Long.MIN_VALUE, Long.MAX_VALUE),
+
+                // Bounded: remaining equals the configured limit, regardless of underlying data size.
+                Arguments.of("bounded (zero)", 0L, 0L),
+                Arguments.of("bounded (small)", 1024L, 1024L),
+                Arguments.of("bounded (Integer.MAX_VALUE)", Integer.MAX_VALUE, (long) Integer.MAX_VALUE),
+
+                // Bounded but extremely large: still not 'unbounded'.
+                Arguments.of("bounded (Long.MAX_VALUE)", Long.MAX_VALUE, Long.MAX_VALUE));
+    }
+
+    @ParameterizedTest(name = "{index}: {0} -> initial remaining {2}")
+    @MethodSource
+    void testRemaining(final String caseName, final long maxCount, final long expectedInitialRemaining)
+            throws Exception {
+        final byte[] data = "Hello World".getBytes(StandardCharsets.UTF_8); // 11 bytes
+
+        try (BoundedInputStream in = BoundedInputStream.builder()
+                .setByteArray(data)
+                .setMaxCount(maxCount)
+                .get()) {
+            // Initial remaining respects the imposed limit (or is Long.MAX_VALUE if unbounded).
+            assertEquals(expectedInitialRemaining, in.getRemaining(), caseName + " (initial)");
+
+            // Skip more than the data length to exercise both bounded and unbounded paths.
+            final long skipped = IOUtils.skip(in, 42);
+
+            // For unbounded streams (EOF == -1), remaining stays the same.
+            // For bounded, it decreases by 'skipped'.
+            final long expectedAfterSkip =
+                    in.getMaxCount() == IOUtils.EOF ? expectedInitialRemaining : expectedInitialRemaining - skipped;
+
+            assertEquals(expectedAfterSkip, in.getRemaining(), caseName + " (after skip)");
+        }
+    }
+
     @Test
     void testReadSingle() throws Exception {
         final byte[] helloWorld = "Hello World".getBytes(StandardCharsets.UTF_8);
