IO-429: Check for long streams in IOUtils.toByteArray#175
IO-429: Check for long streams in IOUtils.toByteArray#175leskin-in wants to merge 2 commits intoapache:masterfrom leskin-in:IO-429-IOUtils-toByteArray-Overflow
Conversation
|
Adding a test for this change (e.g. to A solution I can think of is to use It is also possible to implement a custom |
|
Why would you use a real file? Just test with a |
|
@garydgregory thank you for your comment. I have implemented a test using Note the new test case requires lots of memory, so heap size limit is increased. I do not know whether this is appropriate. The chosen value is the lowest possible, which I obtained empirically on my laptop. IO-161 introduced heap size limit, and it has remained unchanged since then. But the reasons for this (and why |
pom.xml
Outdated
| <reuseForks>false</reuseForks> | ||
| <!-- limit memory size see IO-161 --> | ||
| <argLine>${argLine} -Xmx25M</argLine> | ||
| <argLine>${argLine} -Xmx4223M</argLine> |
There was a problem hiding this comment.
Not acceptable, leave as is for now.
There was a problem hiding this comment.
| try (CircularInputStream cin = new CircularInputStream(new byte[]{65, 65, 65}, ((long)Integer.MAX_VALUE) + 1L)) { | ||
| IOUtils.toByteArray(cin); | ||
| fail("IllegalArgumentException expected"); | ||
| } catch (final IllegalArgumentException exc) { |
| try (final ByteArrayOutputStream output = new ByteArrayOutputStream()) { | ||
| copy(input, output); | ||
| if (copy(input, output) == -1) { | ||
| throw new IllegalArgumentException("Stream cannot be longer than Integer max value bytes"); |
There was a problem hiding this comment.
@leskin-in
This condition will not happen for a ByteArrayInputStream, instead of you'll get a IndexOutOfBoundsException or am I missing something?
There was a problem hiding this comment.
That is correct if an InputStream is a ByteArrayInputStream. However, this method accepts generic InputStream, which may wrap more data. In that case, the implementation of copy() would return -1.
However, ByteArrayOutputStream, which is created in this method as an intermediate buffer, does not check for its underlying buffer overflow at write(). The checks in write() only ensure sanity of arguments passed to it; they are valid in case of copyLarge() (ultimately called by copy() mentioned above).
The ByteArrayOutputStream, however, can store more than Integer.MAX_VALUE bytes because it can use multiple underlying byte arrays.
|
@leskin-in |
Throw an IllegalArgumentException when an InputStream provided to IOUtils.toByteArray() is longer than Integer.MAX_VALUE bytes. Conversion of such long arrays is not possible, as arrays with long indices are forbidden by Java language specification.
@garydgregory, thank you for reminding about this PR. I have rebased it against the current |
Add a test case testToByteArray_InputStreamTooLong() to IOUtilsTestCase to test how InputStreams with more than Integer.MAX_VALUE bytes are processed by IOUtils.toByteArray().
|
@leskin-in |
|
@leskin-in Note that GitHub Actions are currently down. |
|
@garydgregory , that is a nice solution. As Should this PR be closed then? |
|
A different solution is in git master. |
2 participants
Throw an
IllegalArgumentExceptionwhen anInputStreamprovided toIOUtils.toByteArray()is longer thanInteger.MAX_VALUEbytes. Processing of such long arrays is not possible, as arrays withlongindices are forbidden by Java language specification.