Add missing read argument validation in IOUtils (#795)

* Add missing `read` argument validation in `IOUtils`

In #790 I introduced `IOUtils#checkIndexFromLength` calls to validate arguments across the codebase. Ironically, the `IOUtils` class itself was left out.

This PR addresses that omission by adding argument validation to `IOUtils#read` and `IOUtils#readFully`.

Key points:

* Ensures consistency with the rest of Commons IO by validating `offset` and `length`.
* Fixes inconsistent exception behavior:

  * Previously, `length < 0` resulted in an `IllegalArgumentException`.
  * `offset < 0` did not trigger validation and failed later with an `IndexOutOfBoundsException`.
* With this change, both invalid cases are handled consistently and upfront.

* fix: failing tests

Author: ppkarwasz

src/main/java/org/apache/commons/io/IOUtils.java

@@ -2054,6 +2054,7 @@ public static LineIterator lineIterator(final Reader reader) {
      * @param input where to read input from.
      * @param buffer destination.
      * @return actual length read; may be less than requested if EOF was reached.
+     * @throws NullPointerException if {@code input} or {@code buffer} is null.
      * @throws IOException if a read error occurs.
      * @since 2.2
      */
@@ -2074,40 +2075,19 @@ public static int read(final InputStream input, final byte[] buffer) throws IOEx
      * @param offset initial offset into buffer.
      * @param length length to read, must be >= 0.
      * @return actual length read; may be less than requested if EOF was reached.
-     * @throws IllegalArgumentException if length is negative.
+     * @throws NullPointerException     if {@code input} or {@code buffer} is null.
+     * @throws IndexOutOfBoundsException if {@code offset} or {@code length} is negative, or if
+     *                                   {@code offset + length} is greater than {@code buffer.length}.
      * @throws IOException              if a read error occurs.
      * @since 2.2
      */
     public static int read(final InputStream input, final byte[] buffer, final int offset, final int length)
             throws IOException {
-        if (length == 0) {
-            return 0;
-        }
-        return read(input::read, buffer, offset, length);
-    }
-
-    /**
-     * Reads bytes from an input. This implementation guarantees that it will read as many bytes as possible before giving up; this may not always be the case
-     * for subclasses of {@link InputStream}.
-     *
-     * @param input  How to read input.
-     * @param buffer destination.
-     * @param offset initial offset into buffer.
-     * @param length length to read, must be >= 0.
-     * @return actual length read; may be less than requested if EOF was reached.
-     * @throws IllegalArgumentException if length is negative.
-     * @throws IOException              if a read error occurs.
-     * @since 2.2
-     */
-    static int read(final IOTriFunction<byte[], Integer, Integer, Integer> input, final byte[] buffer, final int offset, final int length)
-            throws IOException {
-        if (length < 0) {
-            throw new IllegalArgumentException("Length must not be negative: " + length);
-        }
+        checkFromIndexSize(buffer, offset, length);
         int remaining = length;
         while (remaining > 0) {
             final int location = length - remaining;
-            final int count = input.apply(buffer, offset + location, remaining);
+            final int count = input.read(buffer, offset + location, remaining);
             if (EOF == count) {
                 break;
             }
@@ -2172,15 +2152,15 @@ public static int read(final Reader reader, final char[] buffer) throws IOExcept
      * @param offset initial offset into buffer.
      * @param length length to read, must be &gt;= 0.
      * @return actual length read; may be less than requested if EOF was reached.
-     * @throws IllegalArgumentException if length is negative.
+     * @throws NullPointerException     if {@code reader} or {@code buffer} is null.
+     * @throws IndexOutOfBoundsException if {@code offset} or {@code length} is negative, or if
+     *                                   {@code offset + length} is greater than {@code buffer.length}.
      * @throws IOException              if a read error occurs.
      * @since 2.2
      */
     public static int read(final Reader reader, final char[] buffer, final int offset, final int length)
             throws IOException {
-        if (length < 0) {
-            throw new IllegalArgumentException("Length must not be negative: " + length);
-        }
+        checkFromIndexSize(buffer, offset, length);
         int remaining = length;
         while (remaining > 0) {
             final int location = length - remaining;
@@ -2202,9 +2182,9 @@ public static int read(final Reader reader, final char[] buffer, final int offse
      *
      * @param input where to read input from.
      * @param buffer destination.
-     * @throws IOException              if there is a problem reading the file.
-     * @throws IllegalArgumentException if length is negative.
+     * @throws NullPointerException     if {@code input} or {@code buffer} is null.
      * @throws EOFException             if the number of bytes read was incorrect.
+     * @throws IOException              if there is a problem reading the file.
      * @since 2.2
      */
     public static void readFully(final InputStream input, final byte[] buffer) throws IOException {
@@ -2222,9 +2202,11 @@ public static void readFully(final InputStream input, final byte[] buffer) throw
      * @param buffer destination.
      * @param offset initial offset into buffer.
      * @param length length to read, must be &gt;= 0.
-     * @throws IOException              if there is a problem reading the file.
-     * @throws IllegalArgumentException if length is negative.
+     * @throws NullPointerException     if {@code input} or {@code buffer} is null.
+     * @throws IndexOutOfBoundsException if {@code offset} or {@code length} is negative, or if
+     *                                   {@code offset + length} is greater than {@code buffer.length}.
      * @throws EOFException             if the number of bytes read was incorrect.
+     * @throws IOException              if there is a problem reading the file.
      * @since 2.2
      */
     public static void readFully(final InputStream input, final byte[] buffer, final int offset, final int length)
@@ -2286,9 +2268,9 @@ public static void readFully(final ReadableByteChannel input, final ByteBuffer b
      *
      * @param reader where to read input from.
      * @param buffer destination.
-     * @throws IOException              if there is a problem reading the file.
-     * @throws IllegalArgumentException if length is negative.
+     * @throws NullPointerException     if {@code reader} or {@code buffer} is null.
      * @throws EOFException             if the number of characters read was incorrect.
+     * @throws IOException              if there is a problem reading the file.
      * @since 2.2
      */
     public static void readFully(final Reader reader, final char[] buffer) throws IOException {
@@ -2306,9 +2288,11 @@ public static void readFully(final Reader reader, final char[] buffer) throws IO
      * @param buffer destination.
      * @param offset initial offset into buffer.
      * @param length length to read, must be &gt;= 0.
-     * @throws IOException              if there is a problem reading the file.
-     * @throws IllegalArgumentException if length is negative.
+     * @throws NullPointerException     if {@code reader} or {@code buffer} is null.
+     * @throws IndexOutOfBoundsException if {@code offset} or {@code length} is negative, or if
+     *                                   {@code offset + length} is greater than {@code buffer.length}.
      * @throws EOFException             if the number of characters read was incorrect.
+     * @throws IOException              if there is a problem reading the file.
      * @since 2.2
      */
     public static void readFully(final Reader reader, final char[] buffer, final int offset, final int length)

src/test/java/org/apache/commons/io/IOUtilsTest.java

@@ -1195,6 +1195,31 @@ void testRead_ReadableByteChannel() throws Exception {
         }
     }
 
+    static Stream<Arguments> invalidRead_InputStream_Offset_ArgumentsProvider() {
+        final InputStream input = new ByteArrayInputStream(new byte[10]);
+        final byte[] b = new byte[10];
+        return Stream.of(
+            // input is null
+            Arguments.of(null, b, 0, 1, NullPointerException.class),
+            // b is null
+            Arguments.of(input, null, 0, 1, NullPointerException.class),
+            // off is negative
+            Arguments.of(input, b, -1, 1, IndexOutOfBoundsException.class),
+            // len is negative
+            Arguments.of(input, b, 0, -1, IndexOutOfBoundsException.class),
+            // off + len is too big
+            Arguments.of(input, b, 1, 10, IndexOutOfBoundsException.class),
+            // off + len is too big
+            Arguments.of(input, b, 10, 1, IndexOutOfBoundsException.class)
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("invalidRead_InputStream_Offset_ArgumentsProvider")
+    void testRead_InputStream_Offset_ArgumentsValidation(InputStream input, byte[] b, int off, int len, Class<? extends Throwable> expected) {
+        assertThrows(expected, () -> IOUtils.read(input, b, off, len));
+    }
+
     @Test
     void testReadFully_InputStream__ReturnByteArray() throws Exception {
         final byte[] bytes = "abcd1234".getBytes(StandardCharsets.UTF_8);
@@ -1213,7 +1238,7 @@ void testReadFully_InputStream_ByteArray() throws Exception {
         final byte[] buffer = new byte[size];
         final InputStream input = new ByteArrayInputStream(new byte[size]);
 
-        assertThrows(IllegalArgumentException.class, () -> IOUtils.readFully(input, buffer, 0, -1), "Should have failed with IllegalArgumentException");
+        assertThrows(IndexOutOfBoundsException.class, () -> IOUtils.readFully(input, buffer, 0, -1), "Should have failed with IndexOutOfBoundsException");
 
         IOUtils.readFully(input, buffer, 0, 0);
         IOUtils.readFully(input, buffer, 0, size - 1);
@@ -1260,7 +1285,7 @@ void testReadFully_Reader() throws Exception {
 
         IOUtils.readFully(input, buffer, 0, 0);
         IOUtils.readFully(input, buffer, 0, size - 3);
-        assertThrows(IllegalArgumentException.class, () -> IOUtils.readFully(input, buffer, 0, -1), "Should have failed with IllegalArgumentException");
+        assertThrows(IndexOutOfBoundsException.class, () -> IOUtils.readFully(input, buffer, 0, -1), "Should have failed with IndexOutOfBoundsException");
         assertThrows(EOFException.class, () -> IOUtils.readFully(input, buffer, 0, 5), "Should have failed with EOFException");
         IOUtils.closeQuietly(input);
     }
@@ -1274,6 +1299,12 @@ void testReadFully_Reader_Offset() throws Exception {
         IOUtils.closeQuietly(reader);
     }
 
+    @ParameterizedTest
+    @MethodSource("invalidRead_InputStream_Offset_ArgumentsProvider")
+    void testReadFully_InputStream_Offset_ArgumentsValidation(InputStream input, byte[] b, int off, int len, Class<? extends Throwable> expected) {
+        assertThrows(expected, () -> IOUtils.read(input, b, off, len));
+    }
+
     @Test
     void testReadLines_CharSequence() throws IOException {
         final File file = TestUtils.newFile(temporaryFolder, "lines.txt");