ppkarwasz commented on code in PR #785:
URL: https://github.com/apache/commons-io/pull/785#discussion_r2371334564
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -2680,15 +2680,14 @@ public static BufferedReader toBufferedReader(final
Reader reader, final int siz
*
* @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 IOException If an I/O error occurs while reading
or if the maximum array size is exceeded.
* @throws NullPointerException If {@code inputStream} is {@code null}.
*/
public static byte[] toByteArray(final InputStream inputStream) throws
IOException {
// Using SOFT_MAX_ARRAY_LENGTH guarantees that size() will not overflow
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));
+ throw new IOException(String.format("Cannot read more than %,d
into a byte array", SOFT_MAX_ARRAY_LENGTH));
Review Comment:
In my opinion `IllegalArgumentException` should be thrown for conditions
that the caller is supposed to verify himself. Asking the user to verify this
condition seems like a big requirement. @ecki, what do you think?
However, I think that:
- We can add the condition as a fail-fast option and throw an `IOException`.
The condition should be `size() - position() > SOFT_MAX_ARRAY_LENGTH`.
- Depending on the remaining size of the `FileInputStream`, we can also use
`FileChannel.map()` to map the remaining bytes into direct memory and then copy
them into Java heap memory to a newly created `byte[]`.
- We could actually use this technique for all `toByteArray` methods. In
this case, we should reword the `toByteArray(InputStream, int, int)` Javadoc to
say that `chunkSize` is the maximum size of **temporary** byte arrays used to
load the data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
