You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2025/09/04 21:19:48 UTC
[PR] feat: Add incremental `toByteArray` method [commons-io]
ppkarwasz opened a new pull request, #776:
URL: https://github.com/apache/commons-io/pull/776
This introduces `toByteArray(InputStream input, int size, int bufferSize)`, which reads the stream in chunks of `bufferSize` instead of allocating the full array up front.
By reading incrementally, the method:
* Validates that the stream actually contains `size` bytes before completing the allocation.
* Prevents excessive memory usage if a corrupted or malicious `size` value is provided.
* Offers safer handling for untrusted input compared to the direct-allocation variant.
- [x] I used AI to create the Javadoc.
- [x] Run a successful build using the default [Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command line by itself.
- [x] Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best-practice.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325125868
##########
src/main/java/org/apache/commons/io/RandomAccessFiles.java:
##########
@@ -76,7 +76,11 @@ private static long length(final RandomAccessFile raf) throws IOException {
* other I/O error occurs.
*/
public static byte[] read(final RandomAccessFile input, final long position, final int length) throws IOException {
+ Objects.requireNonNull(input, "input");
input.seek(position);
+ if (length < 0) {
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/d748e99d390eef117d54bf175dcaa569e2b9106d
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #776:
URL: https://github.com/apache/commons-io/pull/776#issuecomment-3264154631
> Hi @garydgregory,
>
> I’ve refactored this proposal a bit further:
>
> * Extended chunked reading to the legacy `toByteArray(InputStream, int/long)` methods as well.
> * Revised the Javadoc to clarify the contract. As you mentioned earlier, we should **not** guide users based on how _trusted_ the size parameter is. I’ve also removed explicit references to `OutOfMemoryError`, which can always occur. Instead, the docs now emphasize that memory allocation is **proportional to the number of bytes actually read** (previously it was _de facto_ proportional to the `size` argument).
>
> Open questions:
>
> * **Chunking threshold**: Currently set to 128 KiB, matching CPython. JDK 11+ uses 16 KiB. We could also consider raising our default buffer size (currently 8 KiB, and in some places as low as 1 KiB, which feels outdated).
> * **Use of `available()`**: Should we consult `available()` in `toByteArray(InputStream, int)` and `toByteArray(InputStream)` (see [feat: Add incremental `toByteArray` method #776 (comment)](https://github.com/apache/commons-io/pull/776#discussion_r2326598113))? The `toByteArray(InputStream, int, int)` method explicitly promises chunks up to `chunkSize`, but the others do not.
CPython is irrelevant IMO. Java's `Files.BUFFER_SIZE` class in Java 21 is 8K, so 8K is consistent as the default.
> JDK 11+ uses 16 KiB.
Where do you see that?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2329690275
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(
+ String.format("Cannot read more than %,d into a byte array", MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input, size, DEFAULT_CHUNK_SIZE);
Review Comment:
I aligned this method with the JDK methods [`InputStream.readNBytes()`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/InputStream.html#readNBytes(int)), which performs a similar task, but is only available since JDK 11.
Our Javadoc in this method does not prevent us from making the switch:
> Gets the contents of an `InputStream` as a byte[]. Use this method instead of `toByteArray(InputStream)` when `InputStream` size is known.
I think that with the right value for the threshold, the change might be profitable for users, what about using `Math.max(in.available(), DEFAULT_CHUNK_SIZE)` as threshold? This should guarantee that loading from local sources will **not** double-buffer (`FileInputStream` reports the remaining bytes in the file as “available”). What do you think?
Personally I would rather not rely on the new behavior in my projects, since there is not guarantee users don't downgrade Commons IO to `2.20.0`, so I am mostly interested in `toByteArray(InputStream, int, int)`, but we might consider improving this method too.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ecki (via GitHub)" <gi...@apache.org>.
ecki commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2338655036
##########
src/changes/changes.xml:
##########
@@ -57,6 +57,7 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], int, int, long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(int, long).</action>
+ <action dev="pkarwasz" type="add" due-to="Piotr P. Karwasz">Add IOUtils.toByteArray(InputStream, int, int) for safer incremental reading with size validation.</action>
Review Comment:
Ah yeah chunked or maybe dynamic, if the idea it works for partial results.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2329706213
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/29f365be5d485dce7d03f0d0384dbf36fd40cc9d
This was folded by my formatter and I usually don't change anything, unless there is a CheckStyle rule that fails.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ecki (via GitHub)" <gi...@apache.org>.
ecki commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2338651435
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
Review Comment:
I mean on reaching the byte array limit. Would be a cleaner signalling compared to returning with size one bigger than a limit
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2340346452
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2711,68 @@ 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>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</p>
+ *
+ * <p>This method processes the input stream in successive chunks of up to
+ * {@code chunkSize} bytes.</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 chunkSize The chunk 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 chunkSize <= 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 chunkSize) throws IOException {
+ Objects.requireNonNull(input, "input");
+ if (chunkSize <= 0) {
+ throw new IllegalArgumentException("Chunk size must be greater than zero: " + chunkSize);
+ }
+ if (size <= chunkSize) {
+ // throws if size < 0
+ return toByteArray(input::read, size);
+ }
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(input, size, chunkSize);
+ if (output.size() != size) {
+ throw new EOFException("Unexpected read size, current: " + output.size() + ", expected: " + size);
Review Comment:
Hi @ppkarwasz
We now have two calls to the new `copyToOutputStream` method, and in both cases, we check the return value and throw an exception (with different exceptions and styles in building the message). What about moving the check and exception throwing inside the private method, which could throw IAE.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325249490
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
+ * </p>
+ *
+ * <p>
+ * <strong>Important:</strong> This method does <em>not</em> defend against corrupted
+ * or untrusted {@code size} values.
+ * For untrusted input, use {@link #toByteArray(InputStream, int, int)} instead,
+ * which validates that the stream contains at least {@code size} bytes before allocating the target array.
+ * </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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
@SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ Objects.requireNonNull(input, "input");
+ if (size < 0) {
+ throw new IllegalArgumentException("Size must be equal or greater than zero: " + size);
+ }
+ return toByteArray(input::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>
+ * This is a convenience overload of {@link #toByteArray(InputStream, int, int)} that accepts a
+ * {@code long} size parameter. The value is checked to ensure it does not exceed
+ * {@link Integer#MAX_VALUE} before being safely converted to {@code int}.
+ * </p>
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/7362d3e34801662b029ea6ef4e005dfb1194c57b
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/7362d3e34801662b029ea6ef4e005dfb1194c57b
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325240290
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
+ * </p>
+ *
+ * <p>
Review Comment:
I removed details about which methods can be used for defensive programming in https://github.com/apache/commons-io/pull/776/commits/7362d3e34801662b029ea6ef4e005dfb1194c57b
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2337088732
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(
+ String.format("Cannot read more than %,d into a byte array", MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
Review Comment:
Yes, I borrowed the **proportional** keyword, which seems to be the magical word to dismiss DoS reports.
Of course `<bytes read> + Integer.MAX_VALUE` is also proportional to the number of bytes read. :wink:
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ecki (via GitHub)" <gi...@apache.org>.
ecki commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2336532313
##########
src/changes/changes.xml:
##########
@@ -57,6 +57,7 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], int, int, long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(int, long).</action>
+ <action dev="pkarwasz" type="add" due-to="Piotr P. Karwasz">Add IOUtils.toByteArray(InputStream, int, int) for safer incremental reading with size validation.</action>
Review Comment:
Btw what’s incremental about reading a while stream into a single array?
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
Review Comment:
For me “proportional” could also mean 2x so maybe “is the same”? And “large input streams” is also relatife with 2gb. Maybe say it directly “Only streams which fit into a single byte array with roughly 2GB limit can be processed with this method”?
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
Review Comment:
Overall it looks like a simplification!
##########
src/test/java/org/apache/commons/io/IOUtilsTest.java:
##########
@@ -1659,6 +1662,48 @@ void testToByteArray_InputStream_SizeZero() throws Exception {
}
}
+ @ParameterizedTest
+ @MethodSource
+ void testToByteArray_InputStream_Size_BufferSize_Succeeds(byte[] data, int size, int bufferSize) throws IOException {
+ final ByteArrayInputStream input = new ByteArrayInputStream(data);
+ final byte[] expected = Arrays.copyOf(data, size);
+ final byte[] actual = IOUtils.toByteArray(input, size, bufferSize);
+ assertArrayEquals(expected, actual);
+ }
+
+ private static Stream<Arguments> testToByteArray_InputStream_Size_BufferSize_Succeeds() {
+ final byte[] data = new byte[1024];
+ for (int i = 0; i < 1024; i++) {
+ data[i] = (byte) i;
+ }
+ return Stream.of(
+ // Eager reading
+ Arguments.of(data.clone(), 512, 1024),
Review Comment:
Those clones and array copies are pretty defensive, but I guess it’s fine here the test only uses small ones
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -221,6 +221,14 @@ public class IOUtils {
*/
private static final char[] SCRATCH_CHAR_BUFFER_WO = charArray();
+ /**
+ * The maximum size of an array in many Java VMs.
+ * <p>
+ * The constant is copied from OpenJDK's {@link jdk.internal.util.ArraysSupport#SOFT_MAX_ARRAY_LENGTH}.
+ * </p>
+ */
+ private static final int SOFT_MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;
Review Comment:
I wonder what’s soft and what’s -8 is about, but I guess the question is for openjdk (for the review it’s only important that it does not overflow, see below)
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
Review Comment:
Hm, the UABOS.size() feels a bit overloaded here and why does the write() not throw?
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
Review Comment:
Maybe add a comment that the soft max is guranteed to not overflow on +1?
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", SOFT_MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>This variant provides no safeguards against allocating very large arrays.
+ * For large streams, prefer {@link #toByteArray(InputStream, int, int)},
+ * which enforces stricter memory usage constraints.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>This variant provides no safeguards against allocating very large arrays.
Review Comment:
Why not? If size is small it allocates no large array?
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
Review Comment:
IAE is a bit strange but since it’s the existing APi it’s fine to keep that.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2330658794
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(
+ String.format("Cannot read more than %,d into a byte array", MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input, size, DEFAULT_CHUNK_SIZE);
Review Comment:
I think this method's behavior should remain unchanged. The current goal is to use the new method in Commons Compress, so let's focus on that IMO.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2337133889
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
Review Comment:
Yes, it is the current API.
@garydgregory, should we change it in a separate PR, so it throws an `IOException`?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add toByteArray(InputStream input, int size, int bufferSize) [commons-io]
Posted by "MarkEWaite (via GitHub)" <gi...@apache.org>.
MarkEWaite commented on PR #776:
URL: https://github.com/apache/commons-io/pull/776#issuecomment-3604799112
As far as I can tell from `git bisect` with builds of commons-io bundled into Jenkins core, this is the pull request that caused a major regression in [Jenkins 2.537](https://www.jenkins.io/changelog/2.537/). The major regression was:
* https://github.com/jenkinsci/jenkins/issues/16845
Jenkins agents failed to connect with SSH until we reverted Apache Commons IO 2.21.0 and returned to Apache Commons IO 2.20.0. We release [Jenkins 2.538](https://www.jenkins.io/changelog/2.538/) less than 24 hours after 2.537 due to the severity of the issue.
The stack trace reported in the failure is:
```
[SSH] Starting sftp client.
[SSH] Copying latest remoting.jar...
java.io.IOException: Could not copy remoting.jar into '/home/jenkins/.jenkins-cd-control' on agent
at PluginClassLoader for ssh-slaves//hudson.plugins.sshslaves.SSHLauncher.copyAgentJar(SSHLauncher.java:738)
at PluginClassLoader for ssh-slaves//hudson.plugins.sshslaves.SSHLauncher.lambda$launch$0(SSHLauncher.java:462)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.IllegalArgumentException: invalid len argument
at PluginClassLoader for trilead-api//com.trilead.ssh2.SFTPv3Client.read(SFTPv3Client.java:1250)
at PluginClassLoader for trilead-api//com.trilead.ssh2.jenkins.SFTPClient$SFTPInputStream.read(SFTPClient.java:172)
at org.apache.commons.io.input.ProxyInputStream.read(ProxyInputStream.java:337)
at org.apache.commons.io.input.BoundedInputStream.read(BoundedInputStream.java:536)
at org.apache.commons.io.output.AbstractByteArrayOutputStream.writeImpl(AbstractByteArrayOutputStream.java:405)
at org.apache.commons.io.output.UnsynchronizedByteArrayOutputStream.write(UnsynchronizedByteArrayOutputStream.java:227)
at org.apache.commons.io.IOUtils.copyToOutputStream(IOUtils.java:1958)
at org.apache.commons.io.IOUtils.toByteArray(IOUtils.java:2918)
at PluginClassLoader for ssh-slaves//hudson.plugins.sshslaves.SSHLauncher.readInputStreamIntoByteArrayAndClose(SSHLauncher.java:796)
at PluginClassLoader for ssh-slaves//hudson.plugins.sshslaves.SSHLauncher.copyAgentJar(SSHLauncher.java:705)
... 5 more
Launch failed - cleaning up connection
[SSH] Connection closed.
```
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2340632143
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2711,68 @@ 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>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</p>
+ *
+ * <p>This method processes the input stream in successive chunks of up to
+ * {@code chunkSize} bytes.</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 chunkSize The chunk 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 chunkSize <= 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 chunkSize) throws IOException {
+ Objects.requireNonNull(input, "input");
+ if (chunkSize <= 0) {
+ throw new IllegalArgumentException("Chunk size must be greater than zero: " + chunkSize);
+ }
+ if (size <= chunkSize) {
+ // throws if size < 0
+ return toByteArray(input::read, size);
+ }
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(input, size, chunkSize);
+ if (output.size() != size) {
+ throw new EOFException("Unexpected read size, current: " + output.size() + ", expected: " + size);
Review Comment:
Those two calls have slightly different checks:
- `toByteArray(InputStream)` checks if the size is **at most** `SOFT_MAX_ARRAY_LENGTH` and throws an IAE if the check fails.
- `toByteArray(InputStream, int, int)` checks if the size is **equal** `size` and throws an EOF if the check fails.
To move the check inside the method, I would need to pass an `IOConsumer` as parameter. In practice this would not change the location of the check code.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2326572852
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2702,48 @@ 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>The method accumulates the data in temporary buffers of size at most {@code bufferSize}
Review Comment:
Replaced with guidance about OOME in https://github.com/apache/commons-io/pull/776/commits/97d37a94de8c935ea8e4961e354a1cd1d5c49b6a
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325862574
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The method accumulates the data in temporary buffers and returns a single array
+ * containing the entire contents once the end of the stream is reached.</p>
+ *
+ * @param input the {@link InputStream} to read; must not be {@code null}.
+ * @return a new byte array.
+ * @throws IllegalArgumentException if the size of the stream is greater than {@code Integer.MAX_VALUE}.
+ * @throws IOException if an I/O error occurs while reading.
+ * @throws NullPointerException if {@code input} is {@code null}.
*/
- public static byte[] toByteArray(final InputStream inputStream) throws IOException {
+ public static byte[] toByteArray(final InputStream input) throws IOException {
// We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
}, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
+ copy(input, thresholdOutput);
return ubaOutput.toByteArray();
}
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The method allocates a single array of the requested size and fills it directly
Review Comment:
This comment is not needed IMO, the user cannot affect the behavior by passing in different values. It's a low-level implementation detail.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The method accumulates the data in temporary buffers and returns a single array
Review Comment:
This comment is not needed IMO, the user cannot affect the behavior about it by passing in different values. It's a low-level implementation detail.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The method accumulates the data in temporary buffers and returns a single array
+ * containing the entire contents once the end of the stream is reached.</p>
+ *
+ * @param input the {@link InputStream} to read; must not be {@code null}.
+ * @return a new byte array.
+ * @throws IllegalArgumentException if the size of the stream is greater than {@code Integer.MAX_VALUE}.
+ * @throws IOException if an I/O error occurs while reading.
+ * @throws NullPointerException if {@code input} is {@code null}.
*/
- public static byte[] toByteArray(final InputStream inputStream) throws IOException {
+ public static byte[] toByteArray(final InputStream input) throws IOException {
// We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
}, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
+ copy(input, thresholdOutput);
return ubaOutput.toByteArray();
}
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The method allocates a single array of the requested size and fills it directly
+ * from the stream.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
return toByteArray(Objects.requireNonNull(input, "input")::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>The method allocates a single array of the requested size and fills it directly
Review Comment:
This comment is not needed IMO, the user cannot affect the behavior by passing in different values. It's a low-level implementation detail.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The method accumulates the data in temporary buffers and returns a single array
+ * containing the entire contents once the end of the stream is reached.</p>
+ *
+ * @param input the {@link InputStream} to read; must not be {@code null}.
+ * @return a new byte array.
+ * @throws IllegalArgumentException if the size of the stream is greater than {@code Integer.MAX_VALUE}.
+ * @throws IOException if an I/O error occurs while reading.
+ * @throws NullPointerException if {@code input} is {@code null}.
*/
- public static byte[] toByteArray(final InputStream inputStream) throws IOException {
+ public static byte[] toByteArray(final InputStream input) throws IOException {
Review Comment:
Please leave the parameter name unchanged. We can normalize the other names later to "inputStream" for consistency.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2702,48 @@ 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>The method accumulates the data in temporary buffers of size at most {@code bufferSize}
Review Comment:
I think it would be nice to explain _why_ this method is useful, instead of _how_ it works. For example, "When reading from an untrusted input stream, you reduce the risk of an OOME by calling this API with...".
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
Review Comment:
What follows is just about the finer aspects of documenting methods, but it's still nice to talk about IMO:
The text can be simpler and more direct, for example, `java.nio.file.Files.read(InputStream, int)` says "Reads all the bytes from an input stream." After all, the only kinds of bytes in an input stream are the "remaining" kind, and it only makes sense to return a "new" array. So I would say: "Reads all the bytes from an input stream in a byte array".
As an aside, Java seems to have called out the "remaining" concept formally in one API name: "Iterator.forEachRemaining(...)", but that might have been to avoid overriding or shadowing the "forEach(...)".
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2326572685
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The method accumulates the data in temporary buffers and returns a single array
+ * containing the entire contents once the end of the stream is reached.</p>
+ *
+ * @param input the {@link InputStream} to read; must not be {@code null}.
+ * @return a new byte array.
+ * @throws IllegalArgumentException if the size of the stream is greater than {@code Integer.MAX_VALUE}.
+ * @throws IOException if an I/O error occurs while reading.
+ * @throws NullPointerException if {@code input} is {@code null}.
*/
- public static byte[] toByteArray(final InputStream inputStream) throws IOException {
+ public static byte[] toByteArray(final InputStream input) throws IOException {
// We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
}, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
+ copy(input, thresholdOutput);
return ubaOutput.toByteArray();
}
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The method allocates a single array of the requested size and fills it directly
Review Comment:
Removed in https://github.com/apache/commons-io/pull/776/commits/97d37a94de8c935ea8e4961e354a1cd1d5c49b6a
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The method accumulates the data in temporary buffers and returns a single array
+ * containing the entire contents once the end of the stream is reached.</p>
+ *
+ * @param input the {@link InputStream} to read; must not be {@code null}.
+ * @return a new byte array.
+ * @throws IllegalArgumentException if the size of the stream is greater than {@code Integer.MAX_VALUE}.
+ * @throws IOException if an I/O error occurs while reading.
+ * @throws NullPointerException if {@code input} is {@code null}.
*/
- public static byte[] toByteArray(final InputStream inputStream) throws IOException {
+ public static byte[] toByteArray(final InputStream input) throws IOException {
// We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
}, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
+ copy(input, thresholdOutput);
return ubaOutput.toByteArray();
}
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The method allocates a single array of the requested size and fills it directly
+ * from the stream.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
return toByteArray(Objects.requireNonNull(input, "input")::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>The method allocates a single array of the requested size and fills it directly
Review Comment:
Removed in https://github.com/apache/commons-io/pull/776/commits/97d37a94de8c935ea8e4961e354a1cd1d5c49b6a
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #776:
URL: https://github.com/apache/commons-io/pull/776#issuecomment-3263013610
Hi @garydgregory,
I’ve refactored this proposal a bit further:
* Extended chunked reading to the legacy `toByteArray(InputStream, int/long)` methods as well.
* Revised the Javadoc to clarify the contract. As you mentioned earlier, we should **not** guide users based on how *trusted* the size parameter is. I’ve also removed explicit references to `OutOfMemoryError`, which can always occur. Instead, the docs now emphasize that memory allocation is **proportional to the number of bytes actually read** (previously it was _de facto_ proportional to the `size` argument).
Open questions:
* **Chunking threshold**: Currently set to 128 KiB, matching CPython. JDK 11+ uses 16 KiB. We could also consider raising our default buffer size (currently 8 KiB, and in some places as low as 1 KiB, which feels outdated).
* **Use of `available()`**: Should we consult `available()` in `toByteArray(InputStream, int)` and `toByteArray(InputStream)` (see https://github.com/apache/commons-io/pull/776#discussion_r2326598113)? The `toByteArray(InputStream, int, int)` method explicitly promises chunks up to `chunkSize`, but the others do not.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2337169218
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
Review Comment:
Yes, `size()` is quite a generic method name, however `UBAOS.size()` is in line with what `ByteArrayOutputStream.size()` from the JDK does.
Which `write` are you referring to? The one used in `copyToOutputStream`? I does throw `IOException` on error.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2339057678
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", SOFT_MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>This variant provides no safeguards against allocating very large arrays.
+ * For large streams, prefer {@link #toByteArray(InputStream, int, int)},
+ * which enforces stricter memory usage constraints.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>This variant provides no safeguards against allocating very large arrays.
Review Comment:
Thanks! Fixed in https://github.com/apache/commons-io/pull/776/commits/e6e2a4d6e1dafb74ef8162116183862fdd9f8d70
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #776:
URL: https://github.com/apache/commons-io/pull/776#issuecomment-3258605946
@garydgregory,
I removed a lot of details from the Javadoc and let only these:
- For `toByteArray(InputStream)`:
https://github.com/apache/commons-io/blob/c6c79a3dc4fcfcc84a82189f9ae41e3a051a5d35/src/main/java/org/apache/commons/io/IOUtils.java#L2643-L2644
- For `toByteArray(InputStream, int/long)`:
https://github.com/apache/commons-io/blob/c6c79a3dc4fcfcc84a82189f9ae41e3a051a5d35/src/main/java/org/apache/commons/io/IOUtils.java#L2666-L2667
- For `toByteArray(InputStream, int, int)`:
https://github.com/apache/commons-io/blob/c6c79a3dc4fcfcc84a82189f9ae41e3a051a5d35/src/main/java/org/apache/commons/io/IOUtils.java#L2708-L2709
I believe that these behaviors will not change and should remain in the method contracts, what do you think?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325251002
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2724,62 @@ 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>
+ * This variant validates that the stream actually contains {@code size} bytes.
+ * It is suitable for untrusted input because it prevents oversized allocations when the provided {@code size}
Review Comment:
Removed in https://github.com/apache/commons-io/pull/776/commits/7362d3e34801662b029ea6ef4e005dfb1194c57b
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325125429
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2707,9 +2790,6 @@ public static byte[] toByteArray(final InputStream input, final long size) throw
* @throws IllegalArgumentException if {@code size} is less than zero.
*/
static byte[] toByteArray(final IOTriFunction<byte[], Integer, Integer, Integer> input, final int size) throws IOException {
- if (size < 0) {
Review Comment:
This method is package-private, so it’s reasonable to push the responsibility for validating array size onto the callers. At one point most callers already performed this check, so I added it to the one missing call site and removed it here.
After reviewing it again, I agree the check belongs here, so I’ll restore it and remove the duplicates in the callers. Fixed in https://github.com/apache/commons-io/pull/776/commits/d748e99d390eef117d54bf175dcaa569e2b9106d
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2323630769
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2707,9 +2790,6 @@ public static byte[] toByteArray(final InputStream input, final long size) throw
* @throws IllegalArgumentException if {@code size} is less than zero.
*/
static byte[] toByteArray(final IOTriFunction<byte[], Integer, Integer, Integer> input, final int size) throws IOException {
- if (size < 0) {
Review Comment:
This check should remain here; a negative array size never makes sense here.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
Review Comment:
Saying "one pass" feels off since the underlying worker method _loops_ until EOF or the right amount is read. I don't think we should describe the _how_ of this method.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
+ * </p>
+ *
+ * <p>
+ * <strong>Important:</strong> This method does <em>not</em> defend against corrupted
+ * or untrusted {@code size} values.
+ * For untrusted input, use {@link #toByteArray(InputStream, int, int)} instead,
+ * which validates that the stream contains at least {@code size} bytes before allocating the target array.
+ * </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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
@SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ Objects.requireNonNull(input, "input");
+ if (size < 0) {
+ throw new IllegalArgumentException("Size must be equal or greater than zero: " + size);
+ }
+ return toByteArray(input::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>
+ * This is a convenience overload of {@link #toByteArray(InputStream, int, int)} that accepts a
Review Comment:
Let's not define convenience versus not; the convenience is only due to the implementation, which feels wrong, we should probably (and separately) fix the implementation to use `Math.toExactInt()`.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2724,62 @@ 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>
+ * This variant validates that the stream actually contains {@code size} bytes.
+ * It is suitable for untrusted input because it prevents oversized allocations when the provided {@code size}
Review Comment:
I don't think we should start to document which APIs are designed for trusted versus untrusted input. The documentation should just describe the intended functionality.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
Review Comment:
You can remove this since it duplicates the exception documentation.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
Review Comment:
Let's not say we assume a parameter is correct; after all, we don't say when we assume a parameter incorrect in other cases. That's what exceptions are for, and sometimes the return value.
##########
src/main/java/org/apache/commons/io/RandomAccessFiles.java:
##########
@@ -76,7 +76,11 @@ private static long length(final RandomAccessFile raf) throws IOException {
* other I/O error occurs.
*/
public static byte[] read(final RandomAccessFile input, final long position, final int length) throws IOException {
+ Objects.requireNonNull(input, "input");
input.seek(position);
+ if (length < 0) {
Review Comment:
This change is unrelated to the feature, and I don't understand the benefit of moving the check from the method actually using the value to here.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
+ * </p>
+ *
+ * <p>
+ * <strong>Important:</strong> This method does <em>not</em> defend against corrupted
+ * or untrusted {@code size} values.
+ * For untrusted input, use {@link #toByteArray(InputStream, int, int)} instead,
+ * which validates that the stream contains at least {@code size} bytes before allocating the target array.
+ * </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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
@SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ Objects.requireNonNull(input, "input");
+ if (size < 0) {
+ throw new IllegalArgumentException("Size must be equal or greater than zero: " + size);
+ }
+ return toByteArray(input::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>
+ * This is a convenience overload of {@link #toByteArray(InputStream, int, int)} that accepts a
+ * {@code long} size parameter. The value is checked to ensure it does not exceed
+ * {@link Integer#MAX_VALUE} before being safely converted to {@code int}.
+ * </p>
Review Comment:
This whole paragraph (above) is wrong.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
+ * </p>
+ *
+ * <p>
+ * <strong>Important:</strong> This method does <em>not</em> defend against corrupted
+ * or untrusted {@code size} values.
+ * For untrusted input, use {@link #toByteArray(InputStream, int, int)} instead,
+ * which validates that the stream contains at least {@code size} bytes before allocating the target array.
+ * </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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
@SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ Objects.requireNonNull(input, "input");
+ if (size < 0) {
Review Comment:
This check should remain in the underlying method.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2724,62 @@ 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>
+ * This variant validates that the stream actually contains {@code size} bytes.
+ * It is suitable for untrusted input because it prevents oversized allocations when the provided {@code size}
+ * is corrupted or malicious.
+ * </p>
+ *
+ * <ul>
+ * <li>If {@code size <= bufferSize}, the array is allocated directly and filled in a single pass.</li>
+ * <li>
+ * If {@code size > bufferSize}, the stream is read incrementally using a buffer of length {@code bufferSize}.
+ * This avoids allocating an excessively large array up front,
+ * but may temporarily double memory usage due to buffering.
+ * </li>
+ * </ul>
+ *
+ * @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 (size < 0) {
+ throw new IllegalArgumentException("Size must be equal or greater than zero: " + size);
+ }
+ if (bufferSize <= 0) {
+ throw new IllegalArgumentException("Chunk size must be greater than zero: " + bufferSize);
Review Comment:
The message text and parameter names should match: Either "buffer" or "chunk", not both.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
+ * </p>
+ *
+ * <p>
Review Comment:
This feels like FUD to me, and I would remove the whole paragraph. I don't think we want to start documenting which methods fall into which defensive category, especially when the actual issue would be in the call site, not this implementation.
##########
src/changes/changes.xml:
##########
@@ -57,6 +57,7 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], int, int, long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(int, long).</action>
+ <action dev="pkarwasz" type="add" due-to="Piotr P. Karwasz">Added toByteArray(InputStream, int, int) for safer incremental reading with size validation.</action>
Review Comment:
"Added" -> "Add", like all other descriptions that add something.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2724,62 @@ 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>
+ * This variant validates that the stream actually contains {@code size} bytes.
+ * It is suitable for untrusted input because it prevents oversized allocations when the provided {@code size}
+ * is corrupted or malicious.
+ * </p>
+ *
+ * <ul>
+ * <li>If {@code size <= bufferSize}, the array is allocated directly and filled in a single pass.</li>
+ * <li>
+ * If {@code size > bufferSize}, the stream is read incrementally using a buffer of length {@code bufferSize}.
+ * This avoids allocating an excessively large array up front,
+ * but may temporarily double memory usage due to buffering.
+ * </li>
+ * </ul>
+ *
+ * @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 (size < 0) {
Review Comment:
You can remove this check since the worker method does it (in master).
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2329705120
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -221,6 +221,21 @@ public class IOUtils {
*/
private static final char[] SCRATCH_CHAR_BUFFER_WO = charArray();
+ /**
+ * The maximum size of an array in many Java VMs.
+ */
+ private static final int MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;
+
+ /*
+ * Default maximum chunk size used when copying large streams into a byte array.
+ * <p>
+ * This value is somewhat arbitrary, currently aligned with the value used by
+ * <a href="https://github.com/python/cpython/blob/3.14/Lib/_pyio.py">Python</a>
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/38a6a2c6b7dcb3cf0646a54ce4a762913f6b7627
I didn't use `{@value}`, since as far as I remember, the `javadoc` tool in JDK 8 fails to reference values, which are not public constants.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2329688265
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -221,6 +221,21 @@ public class IOUtils {
*/
private static final char[] SCRATCH_CHAR_BUFFER_WO = charArray();
+ /**
+ * The maximum size of an array in many Java VMs.
+ */
+ private static final int MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;
+
+ /*
+ * Default maximum chunk size used when copying large streams into a byte array.
+ * <p>
+ * This value is somewhat arbitrary, currently aligned with the value used by
+ * <a href="https://github.com/python/cpython/blob/3.14/Lib/_pyio.py">Python</a>
+ * for copying streams.
+ * </p>
+ */
+ private static final int DEFAULT_CHUNK_SIZE = 128 * 1024;
Review Comment:
Food for thought: `Files.BUFFER_SIZE` is 8 KiB – maybe we shall stick to Java defaults? Note that 8 KiB will theoretically allow more threads consuming in parallel.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -221,6 +221,21 @@ public class IOUtils {
*/
private static final char[] SCRATCH_CHAR_BUFFER_WO = charArray();
+ /**
+ * The maximum size of an array in many Java VMs.
+ */
+ private static final int MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;
Review Comment:
A reference to `ArraysSupport#SOFT_MAX_ARRAY_LENGTH` in the Javadoc would be handy for maintainers.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2714,13 +2800,9 @@ static byte[] toByteArray(final IOTriFunction<byte[], Integer, Integer, Integer>
return EMPTY_BYTE_ARRAY;
}
final byte[] data = byteArray(size);
- int offset = 0;
- int read;
- while (offset < size && (read = input.apply(data, offset, size - offset)) != EOF) {
- offset += read;
- }
- if (offset != size) {
- throw new IOException("Unexpected read size, current: " + offset + ", expected: " + size);
+ final int read = read(input, data, 0, size);
Review Comment:
AFAICT, this _improvement_ feels unrelated with this PR. You might consider carrying this out in a separate PR.
##########
src/test/java/org/apache/commons/io/IOUtilsTest.java:
##########
Review Comment:
I don't see a test that verifies the exception on insufficient data. Am I mistaken?
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(
+ String.format("Cannot read more than %,d into a byte array", MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
Review Comment:
Okay... I see we're using `UnsynchronizedByteArrayOutputStream`, which allocates along the way. Now I understand why you avoided to provide an elaborate explanation on the memory cap. I was about to suggest you to shape the Javadoc similar to `InputStream::readNBytes`, but then I realized that you already did by using the `proportional` term here. I see you've done your homework. :+1:
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(
+ String.format("Cannot read more than %,d into a byte array", MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input, size, DEFAULT_CHUNK_SIZE);
Review Comment:
I don't think this necessarily implies double buffering: we can avoid the ephemeral buffer if `size <= chunkSize` – and this is what @ppkarwasz already does:
https://github.com/apache/commons-io/pull/776/files#diff-b106f51e7daf5b40ed10dccc4622f425f3e82afc52fb736547a8f28efa6478e4R2750-R2753
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(
+ String.format("Cannot read more than %,d into a byte array", MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
Review Comment:
Can you explain what is meant with `proportional` here? As a user reading this to understand the API, I find it more confusing than helpful. Can't we simply say something as follows?
> This method allocates at most `size` bytes, plus {@value MAX_ARRAY_LENGTH} bytes for the temporary buffer used to consume the stream.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
Review Comment:
Can this ever happen? If not, I'd replace `if` with `assert`.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325240868
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/7362d3e34801662b029ea6ef4e005dfb1194c57b
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2337189758
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
Review Comment:
The total memory allocation for this method is actually about **twice** the real size of the stream, since data is first loaded into chunks and only then copied to a `byte` array of the appropriate size.
I fixed the mention of “large input streams” in https://github.com/apache/commons-io/pull/776/commits/eb6c2bccb61dda3b586e7f5828ae7cf009970a70
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/eb6c2bccb61dda3b586e7f5828ae7cf009970a70
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2339769508
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
Review Comment:
I believe that under the hood both the current code (below) and the proposed change reads 1 byte more than the limit.
https://github.com/apache/commons-io/blob/4dc17b6cb8c794cd15acb2bc99fe30d63306d5f9/src/main/java/org/apache/commons/io/IOUtils.java#L2651-L2657
I prefer the new approach because:
* It writes directly into `UnsynchronizedByteArrayOutputStream` buffers without requiring additional buffers in the `copy` call.
* the control flow is more explicity (`copy(limit + 1) -> size check -> throw`),
* and it avoids the overhead of extra wrappers and callbacks.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2342470057
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
Review Comment:
Sounds good. Let's finish this one first.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add toByteArray(InputStream input, int size, int bufferSize) [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #776:
URL: https://github.com/apache/commons-io/pull/776
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2329702187
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -221,6 +221,21 @@ public class IOUtils {
*/
private static final char[] SCRATCH_CHAR_BUFFER_WO = charArray();
+ /**
+ * The maximum size of an array in many Java VMs.
+ */
+ private static final int MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/38a6a2c6b7dcb3cf0646a54ce4a762913f6b7627
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325129002
##########
src/changes/changes.xml:
##########
@@ -57,6 +57,7 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], int, int, long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(int, long).</action>
+ <action dev="pkarwasz" type="add" due-to="Piotr P. Karwasz">Added toByteArray(InputStream, int, int) for safer incremental reading with size validation.</action>
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/33a4fe15a7c05a3b850b6a9872ac6dd3b7c0aaf6
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325248753
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
+ * </p>
+ *
+ * <p>
+ * <strong>Important:</strong> This method does <em>not</em> defend against corrupted
+ * or untrusted {@code size} values.
+ * For untrusted input, use {@link #toByteArray(InputStream, int, int)} instead,
+ * which validates that the stream contains at least {@code size} bytes before allocating the target array.
+ * </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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
@SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ Objects.requireNonNull(input, "input");
+ if (size < 0) {
+ throw new IllegalArgumentException("Size must be equal or greater than zero: " + size);
+ }
+ return toByteArray(input::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>
+ * This is a convenience overload of {@link #toByteArray(InputStream, int, int)} that accepts a
Review Comment:
Since this method throws an unchecked exception if the `long` does not fit into an `int`, most callers have to check the parameter, before calling this. So this methods looks like a convenient composition of a size check and `toByteArray(InputStream, int, int)`.
Fixed in https://github.com/apache/commons-io/pull/776/commits/7362d3e34801662b029ea6ef4e005dfb1194c57b
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325257858
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2724,62 @@ 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>
+ * This variant validates that the stream actually contains {@code size} bytes.
+ * It is suitable for untrusted input because it prevents oversized allocations when the provided {@code size}
+ * is corrupted or malicious.
+ * </p>
+ *
+ * <ul>
+ * <li>If {@code size <= bufferSize}, the array is allocated directly and filled in a single pass.</li>
+ * <li>
+ * If {@code size > bufferSize}, the stream is read incrementally using a buffer of length {@code bufferSize}.
+ * This avoids allocating an excessively large array up front,
+ * but may temporarily double memory usage due to buffering.
+ * </li>
+ * </ul>
+ *
+ * @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 (size < 0) {
+ throw new IllegalArgumentException("Size must be equal or greater than zero: " + size);
+ }
+ if (bufferSize <= 0) {
+ throw new IllegalArgumentException("Chunk size must be greater than zero: " + bufferSize);
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/c6c79a3dc4fcfcc84a82189f9ae41e3a051a5d35
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325255942
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2724,62 @@ 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>
+ * This variant validates that the stream actually contains {@code size} bytes.
+ * It is suitable for untrusted input because it prevents oversized allocations when the provided {@code size}
+ * is corrupted or malicious.
+ * </p>
+ *
+ * <ul>
+ * <li>If {@code size <= bufferSize}, the array is allocated directly and filled in a single pass.</li>
+ * <li>
+ * If {@code size > bufferSize}, the stream is read incrementally using a buffer of length {@code bufferSize}.
+ * This avoids allocating an excessively large array up front,
+ * but may temporarily double memory usage due to buffering.
+ * </li>
+ * </ul>
+ *
+ * @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 (size < 0) {
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/fe39b777c447f2eeb185ae2520ca6b547d84e719
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2336501535
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", SOFT_MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>This variant provides no safeguards against allocating very large arrays.
+ * For large streams, prefer {@link #toByteArray(InputStream, int, int)},
+ * which enforces stricter memory usage constraints.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input::read, size);
Review Comment:
With this change, we are losing the better NPE message on Java < 14.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2326572400
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The method accumulates the data in temporary buffers and returns a single array
+ * containing the entire contents once the end of the stream is reached.</p>
+ *
+ * @param input the {@link InputStream} to read; must not be {@code null}.
+ * @return a new byte array.
+ * @throws IllegalArgumentException if the size of the stream is greater than {@code Integer.MAX_VALUE}.
+ * @throws IOException if an I/O error occurs while reading.
+ * @throws NullPointerException if {@code input} is {@code null}.
*/
- public static byte[] toByteArray(final InputStream inputStream) throws IOException {
+ public static byte[] toByteArray(final InputStream input) throws IOException {
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/9439095923e901ef3e4eac9ae4f06c00268a280c
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2328895181
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -221,6 +221,21 @@ public class IOUtils {
*/
private static final char[] SCRATCH_CHAR_BUFFER_WO = charArray();
+ /**
+ * The maximum size of an array in many Java VMs.
+ */
+ private static final int MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;
Review Comment:
Java 12's `ArraySupport` class calls this `SOFT_MAX_ARRAY_LENGTH`, so I think we should use the same name.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(
+ String.format("Cannot read more than %,d into a byte array", MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input, size, DEFAULT_CHUNK_SIZE);
Review Comment:
Woa, this is surprising; now we are double-buffering and slowing down existing call sites that don't want this new feature? It seems to like we should _only_ do the double buffering when a call site chooses to use the new API.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -221,6 +221,21 @@ public class IOUtils {
*/
private static final char[] SCRATCH_CHAR_BUFFER_WO = charArray();
+ /**
+ * The maximum size of an array in many Java VMs.
+ */
+ private static final int MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;
+
+ /*
+ * Default maximum chunk size used when copying large streams into a byte array.
+ * <p>
+ * This value is somewhat arbitrary, currently aligned with the value used by
+ * <a href="https://github.com/python/cpython/blob/3.14/Lib/_pyio.py">Python</a>
Review Comment:
CPython is completely irrelevant here; it should not be mentioned, as we certainly do not want to give the impression that we are aligning anything with CPython. I also like to mention the value with `{@value}` in this kind of Javadoc.
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2720,69 @@ 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>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</p>
+ *
+ * <p>This method processes the input stream in successive chunks of up to
+ * {@code chunkSize} bytes.</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 chunkSize The chunk 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 chunkSize <= 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 chunkSize) throws IOException {
+ Objects.requireNonNull(input, "input");
+ if (chunkSize <= 0) {
+ throw new IllegalArgumentException("Chunk size must be greater than zero: " + chunkSize);
+ }
+ if (size <= chunkSize) {
+ // throws if size < 0
+ return toByteArray(input::read, size);
+ }
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(input, size, chunkSize);
+ if (output.size() != size) {
+ throw new EOFException("Unexpected read size, current: " + output.size() + ", expected: " + size);
+ }
+ return output.toByteArray();
+ }
+
+ /**
+ * Copies up to {@code size} bytes from the given {@link InputStream} into a new {@link UnsynchronizedByteArrayOutputStream}.
+ *
Review Comment:
Could you remove extra line?
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
Review Comment:
You can use line lengths up to 160 per our Checkstyle, so the code doesn't end up reading like a newspaper 😉
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325238418
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/7362d3e34801662b029ea6ef4e005dfb1194c57b
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ecki (via GitHub)" <gi...@apache.org>.
ecki commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2338658902
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", SOFT_MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>This variant provides no safeguards against allocating very large arrays.
+ * For large streams, prefer {@link #toByteArray(InputStream, int, int)},
+ * which enforces stricter memory usage constraints.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>This variant provides no safeguards against allocating very large arrays.
Review Comment:
How about “This version always allocates the whole requested array size, for a dynamic growing variant use ..”?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ecki (via GitHub)" <gi...@apache.org>.
ecki commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2338658902
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", SOFT_MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>This variant provides no safeguards against allocating very large arrays.
+ * For large streams, prefer {@link #toByteArray(InputStream, int, int)},
+ * which enforces stricter memory usage constraints.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>This variant provides no safeguards against allocating very large arrays.
Review Comment:
How about “This variant always allocates the whole requested array size, for a dynamic growing variant use ..”?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2329705832
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2720,69 @@ 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>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</p>
+ *
+ * <p>This method processes the input stream in successive chunks of up to
+ * {@code chunkSize} bytes.</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 chunkSize The chunk 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 chunkSize <= 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 chunkSize) throws IOException {
+ Objects.requireNonNull(input, "input");
+ if (chunkSize <= 0) {
+ throw new IllegalArgumentException("Chunk size must be greater than zero: " + chunkSize);
+ }
+ if (size <= chunkSize) {
+ // throws if size < 0
+ return toByteArray(input::read, size);
+ }
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(input, size, chunkSize);
+ if (output.size() != size) {
+ throw new EOFException("Unexpected read size, current: " + output.size() + ", expected: " + size);
+ }
+ return output.toByteArray();
+ }
+
+ /**
+ * Copies up to {@code size} bytes from the given {@link InputStream} into a new {@link UnsynchronizedByteArrayOutputStream}.
+ *
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/29f365be5d485dce7d03f0d0384dbf36fd40cc9d
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/29f365be5d485dce7d03f0d0384dbf36fd40cc9d
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #776:
URL: https://github.com/apache/commons-io/pull/776#issuecomment-3265381781
> > JDK 11+ uses 16 KiB.
>
> Where do you see that?
In the [source code of `InputStream`](https://github.com/openjdk/jdk/blob/bea2b029a77e126171d17c3a44baec6d5cafed4a/src/java.base/share/classes/java/io/InputStream.java#L58). This is the value used by `readAllBytes/readNBytes`.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2336381952
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -221,6 +221,21 @@ public class IOUtils {
*/
private static final char[] SCRATCH_CHAR_BUFFER_WO = charArray();
+ /**
+ * The maximum size of an array in many Java VMs.
+ */
+ private static final int MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;
+
+ /*
+ * Default maximum chunk size used when copying large streams into a byte array.
+ * <p>
+ * This value is somewhat arbitrary, currently aligned with the value used by
+ * <a href="https://github.com/python/cpython/blob/3.14/Lib/_pyio.py">Python</a>
+ * for copying streams.
+ * </p>
+ */
+ private static final int DEFAULT_CHUNK_SIZE = 128 * 1024;
Review Comment:
I switched to 8 KiB in https://github.com/apache/commons-io/pull/776/commits/4aba097d871332ca1ac537abb5f6880c0bf0ffc3
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2337119654
##########
src/test/java/org/apache/commons/io/IOUtilsTest.java:
##########
Review Comment:
This test case shows the exception on insufficient data:
https://github.com/apache/commons-io/blob/624de9fa1a2d376e944219249c56639e02da441b/src/test/java/org/apache/commons/io/IOUtilsTest.java#L1703-L1704
Surefire is configured with 25 MiB of heap (probably should be 256 MiB), so having a smaller chunk size prevents an OOME here.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2326573543
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/cbfa307f526f8c6fac4744fea135de16edbdb1b0
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2342470436
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
Review Comment:
OK.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2339058928
##########
src/changes/changes.xml:
##########
@@ -57,6 +57,7 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], int, int, long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(int, long).</action>
+ <action dev="pkarwasz" type="add" due-to="Piotr P. Karwasz">Add IOUtils.toByteArray(InputStream, int, int) for safer incremental reading with size validation.</action>
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/e6e2a4d6e1dafb74ef8162116183862fdd9f8d70
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2337101006
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", SOFT_MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>This variant provides no safeguards against allocating very large arrays.
+ * For large streams, prefer {@link #toByteArray(InputStream, int, int)},
+ * which enforces stricter memory usage constraints.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input::read, size);
Review Comment:
Nice catch! :100:
Fixed in https://github.com/apache/commons-io/pull/776/commits/15b249da4f4e6bc8238dfce9ae34f4d21f7f88a4
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2325130242
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2659,35 +2660,61 @@ public static byte[] toByteArray(final InputStream inputStream) throws IOExcepti
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>
+ * This variant allocates the target array immediately and attempts to fill it in one pass.
+ * It assumes that {@code size} is correct.
+ * If the stream ends prematurely, an {@link EOFException} is thrown.
+ * </p>
+ *
+ * <p>
+ * <strong>Important:</strong> This method does <em>not</em> defend against corrupted
+ * or untrusted {@code size} values.
+ * For untrusted input, use {@link #toByteArray(InputStream, int, int)} instead,
+ * which validates that the stream contains at least {@code size} bytes before allocating the target array.
+ * </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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
@SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ Objects.requireNonNull(input, "input");
+ if (size < 0) {
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/d748e99d390eef117d54bf175dcaa569e2b9106d
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2326572559
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2638,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all remaining bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The method accumulates the data in temporary buffers and returns a single array
Review Comment:
Removed in https://github.com/apache/commons-io/pull/776/commits/97d37a94de8c935ea8e4961e354a1cd1d5c49b6a
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2336380869
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(
+ String.format("Cannot read more than %,d into a byte array", MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input, size, DEFAULT_CHUNK_SIZE);
Review Comment:
I restored the previous behavior in https://github.com/apache/commons-io/pull/776/commits/3afae073c62d55f241f3893612e85ce650b79bbe, but I added a recommendation to use `toByteArray(InputStream, int, int)` for large arrays:
https://github.com/apache/commons-io/blob/4aba097d871332ca1ac537abb5f6880c0bf0ffc3/src/main/java/org/apache/commons/io/IOUtils.java#L2691-L2693
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2336478064
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2652,65 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output =
+ copyToOutputStream(inputStream, MAX_ARRAY_LENGTH + 1, DEFAULT_CHUNK_SIZE);
+ if (output.size() > MAX_ARRAY_LENGTH) {
Review Comment:
Yes, it's possible since `MAX_ARRAY_LENGTH` is `Integer.MAX_VALUE - 8`.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2337102998
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2714,13 +2800,9 @@ static byte[] toByteArray(final IOTriFunction<byte[], Integer, Integer, Integer>
return EMPTY_BYTE_ARRAY;
}
final byte[] data = byteArray(size);
- int offset = 0;
- int read;
- while (offset < size && (read = input.apply(data, offset, size - offset)) != EOF) {
- offset += read;
- }
- if (offset != size) {
- throw new IOException("Unexpected read size, current: " + offset + ", expected: " + size);
+ final int read = read(input, data, 0, size);
Review Comment:
Fixed in https://github.com/apache/commons-io/pull/776/commits/624de9fa1a2d376e944219249c56639e02da441b
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2337206751
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2637,57 +2645,61 @@ public static BufferedReader toBufferedReader(final Reader reader, final int siz
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}.
- * <p>
- * This method buffers the input internally, so there is no need to use a {@link BufferedInputStream}.
- * </p>
+ * Reads all the bytes from an input stream in a byte array.
*
- * @param inputStream the {@link InputStream} to read.
- * @return the requested byte array.
- * @throws NullPointerException if the InputStream is {@code null}.
- * @throws IOException if an I/O error occurs or reading more than {@link Integer#MAX_VALUE} occurs.
+ * <p>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read, which is only limited by {@link Integer#MAX_VALUE}. This makes it unsuitable for
+ * processing large input streams, unless sufficient heap space is available.</p>
+ *
+ * @param inputStream The {@link InputStream} to read; must not be {@code null}.
+ * @return A new byte array.
+ * @throws IllegalArgumentException If the size of the stream is greater than the maximum array size.
+ * @throws IOException If an I/O error occurs while reading.
+ * @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws IOException {
- // We use a ThresholdingOutputStream to avoid reading AND writing more than Integer.MAX_VALUE.
- try (UnsynchronizedByteArrayOutputStream ubaOutput = UnsynchronizedByteArrayOutputStream.builder().get();
- ThresholdingOutputStream thresholdOutput = new ThresholdingOutputStream(Integer.MAX_VALUE, os -> {
- throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", Integer.MAX_VALUE));
- }, os -> ubaOutput)) {
- copy(inputStream, thresholdOutput);
- return ubaOutput.toByteArray();
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(inputStream, SOFT_MAX_ARRAY_LENGTH + 1, DEFAULT_BUFFER_SIZE);
+ if (output.size() > SOFT_MAX_ARRAY_LENGTH) {
+ throw new IllegalArgumentException(String.format("Cannot read more than %,d into a byte array", SOFT_MAX_ARRAY_LENGTH));
}
+ return output.toByteArray();
}
/**
- * Gets the contents of an {@link InputStream} as a {@code byte[]}. Use this method instead of
- * {@link #toByteArray(InputStream)} when {@link InputStream} size is known.
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= length of input stream.
- * @return byte [] of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is smaller than parameter {@code size}.
- * @throws IllegalArgumentException if {@code size} is less than zero.
+ * <p>This variant provides no safeguards against allocating very large arrays.
+ * For large streams, prefer {@link #toByteArray(InputStream, int, int)},
+ * which enforces stricter memory usage constraints.</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}.
+ * @return a new byte array of length {@code size}.
+ * @throws IllegalArgumentException if {@code size} is negative.
+ * @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.1
*/
- @SuppressWarnings("resource")
public static byte[] toByteArray(final InputStream input, final int size) throws IOException {
- return toByteArray(Objects.requireNonNull(input, "input")::read, size);
+ return toByteArray(input::read, size);
}
/**
- * Gets contents of an {@link InputStream} as a {@code byte[]}.
- * Use this method instead of {@link #toByteArray(InputStream)}
- * when {@link InputStream} size is known.
- * <strong>NOTE:</strong> the method checks that the length can safely be cast to an int without truncation
- * before using {@link IOUtils#toByteArray(InputStream, int)} to read into the byte array.
- * (Arrays can have no more than Integer.MAX_VALUE entries anyway.)
+ * Reads exactly {@code size} bytes from the given {@link InputStream} into a new {@code byte[]}.
*
- * @param input the {@link InputStream} to read.
- * @param size the size of {@link InputStream} to read, where 0 < {@code size} <= min(Integer.MAX_VALUE, length of input stream).
- * @return byte [] the requested byte array, of length {@code size}.
- * @throws IOException if an I/O error occurs or {@link InputStream} length is less than {@code size}.
- * @throws IllegalArgumentException if size is less than zero or size is greater than Integer.MAX_VALUE.
- * @see IOUtils#toByteArray(InputStream, int)
+ * <p>This variant provides no safeguards against allocating very large arrays.
Review Comment:
The difference between this method and `toByteArray(InputStream, size, chunkSize)` is that this method always allocates a byte array of size `size`, while the other allocates `2 * min(size, remaining bytes in the stream)`.
These are the safeguards I am mentioning in the Javadoc: if the stream is corrupted this method could allocate 2 GiB of memory and cause an OOME, while the other method will allocate up to twice what is left in the stream. How could I express this better?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2337176712
##########
src/changes/changes.xml:
##########
@@ -57,6 +57,7 @@ The <action> type attribute can be add,update,fix,remove.
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], int, int, long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(byte[], long).</action>
<action dev="ggregory" type="add" due-to="Gary Gregory">Add org.apache.commons.io.output.ProxyOutputStream.writeRepeat(int, long).</action>
+ <action dev="pkarwasz" type="add" due-to="Piotr P. Karwasz">Add IOUtils.toByteArray(InputStream, int, int) for safer incremental reading with size validation.</action>
Review Comment:
Should I say “chunked”?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: Add incremental `toByteArray` method [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #776:
URL: https://github.com/apache/commons-io/pull/776#discussion_r2342465755
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2697,6 +2711,68 @@ 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>The memory used by this method is <strong>proportional</strong> to the number
+ * of bytes read and limited by the specified {@code size}. This makes it suitable for
+ * processing large input streams, provided that <strong>sufficient</strong> heap space is
+ * available.</p>
+ *
+ * <p>This method processes the input stream in successive chunks of up to
+ * {@code chunkSize} bytes.</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 chunkSize The chunk 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 chunkSize <= 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 chunkSize) throws IOException {
+ Objects.requireNonNull(input, "input");
+ if (chunkSize <= 0) {
+ throw new IllegalArgumentException("Chunk size must be greater than zero: " + chunkSize);
+ }
+ if (size <= chunkSize) {
+ // throws if size < 0
+ return toByteArray(input::read, size);
+ }
+ final UnsynchronizedByteArrayOutputStream output = copyToOutputStream(input, size, chunkSize);
+ if (output.size() != size) {
+ throw new EOFException("Unexpected read size, current: " + output.size() + ", expected: " + size);
Review Comment:
OK.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org