ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2326590575


##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2693,52 @@ public static byte[] toByteArray(final InputStream 
input, final long size) throw
         return toByteArray(input, (int) size);
     }
 
+    /**
+     * Reads exactly {@code size} bytes from the given {@link InputStream} 
into a new {@code byte[]}.
+     *
+     * <p>When reading from an untrusted stream, this variant lowers the risk 
of
+     * {@link OutOfMemoryError} by allocating data in buffers of up to {@code 
bufferSize}
+     * bytes rather than in one large array.</p>
+     *
+     * <p>Note, however, that this approach requires additional temporary 
memory
+     * compared to {@link #toByteArray(InputStream, int)}.</p>

Review Comment:
   If we document `OutOfMemoryError` risk in the Javadoc…



##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2693,52 @@ public static byte[] toByteArray(final InputStream 
input, final long size) throw
         return toByteArray(input, (int) size);
     }
 
+    /**
+     * Reads exactly {@code size} bytes from the given {@link InputStream} 
into a new {@code byte[]}.
+     *
+     * <p>When reading from an untrusted stream, this variant lowers the risk 
of
+     * {@link OutOfMemoryError} by allocating data in buffers of up to {@code 
bufferSize}
+     * bytes rather than in one large array.</p>
+     *
+     * <p>Note, however, that this approach requires additional temporary 
memory
+     * compared to {@link #toByteArray(InputStream, int)}.</p>
+     *
+     * @param input      the {@link InputStream} to read; must not be {@code 
null}.
+     * @param size       the exact number of bytes to read; must be {@code >= 
0}.
+     *                   The actual bytes read are validated to equal {@code 
size}.
+     * @param bufferSize the buffer size for incremental reading; must be 
{@code > 0}.
+     * @return a new byte array of length {@code size}.
+     * @throws IllegalArgumentException if {@code size} is negative or {@code 
bufferSize <= 0}.
+     * @throws EOFException             if the stream ends before {@code size} 
bytes are read.
+     * @throws IOException              if an I/O error occurs while reading.
+     * @throws NullPointerException     if {@code input} is {@code null}.
+     * @since 2.21.0
+     */
+    public static byte[] toByteArray(final InputStream input, final int size, 
final int bufferSize) throws IOException {
+        Objects.requireNonNull(input, "input");
+        if (bufferSize <= 0) {
+            throw new IllegalArgumentException("Buffer size must be greater 
than zero: " + bufferSize);
+        }
+        if (size <= bufferSize) {
+            // throws if size < 0
+            return toByteArray(input::read, size);
+        }

Review Comment:
   …should we also extend the "fast path" case to situations where the 
requested size is already available in the stream?
   
   ```suggestion
           // Fast path: use direct allocation if the size fits in one buffer
           // or if at least `size` bytes are already available without 
blocking.
           if (size <= Math.max(bufferSize, input.available())) {
               // throws if size < 0
               return toByteArray(input::read, size);
           }
   ```
   
   In this case what would you suggest I add to the Javadoc?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to