Fix infinite loop in AbstractByteArrayOutputStream. #758
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When an AbstractByteArrayOutputStream is initialized with a 0 size buffer and writeImpl(InputStream) is called an infinite loop may occur and eventually cause an OOM exception. This is due to passing 0 as the maximum bytes to read in InputStream#read which returns 0, then a new 0 size buffer is created, and then InputStream#read is called again therefore repeating the loop forever. Fix this infinite loop by updating needNewBuffer to use DEFAULT_SIZE as the initial buffer size instead of 0. This will allow InputStream#read to properly populate the buffer as expected and terminate the loop when EOF is reached. Set initial value of currentBufferIndex to -1 so it is properly initialized to 0 when needNewBuffer is called; unrelated to the infinite loop bug.
qwerty4030
commented
Jul 3, 2025
| @@ -92,7 +92,7 @@ protected interface InputStreamConstructor<T extends InputStream> { | |||
| private byte[] currentBuffer; | |||
|
|
|||
| /** The index of the current buffer. */ | |||
| private int currentBufferIndex; | |||
| private int currentBufferIndex = -1; | |||
There was a problem hiding this comment.
unrelated to the bug but I found this value was wrong while investigating. I dont think its causing any issues because it appears to only be used after reset is called which properly sets this value.
qwerty4030
commented
Jul 3, 2025
| @@ -147,7 +147,8 @@ protected void needNewBuffer(final int newCount) { | |||
| // Creating new buffer | |||
| final int newBufferSize; | |||
| if (currentBuffer == null) { | |||
| newBufferSize = newCount; | |||
| // prevents 0 size buffers | |||
| newBufferSize = newCount > 0 ? newCount : DEFAULT_SIZE; | |||
There was a problem hiding this comment.
simplest fix I found. Another option is to not allow ANY buffers smaller than DEFAULT_SIZE. Some things I ran into trying other fixes:
- if the logic allows an initial 0 size buffer; the next buffer must be >0 size. Then when
toBufferedInputStreamis called the tests fail because.available()returns 0 (for the initial buffer) instead of the actual length of the data in the next buffer. Also seems like a waste to even callInputStream.readwith a 0 size buffer. - Technically there is nothing wrong with a BAOS with an initial size of 0, so we don't want to throw an exception. The docs say the buffer is increased as needed so hopefully it's fine to default it to
DEFAULT_SIZEin this case.
qwerty4030
commented
Jul 3, 2025
| final @TempDir Path temporaryFolder) throws IOException { | ||
| final Path emptyFile = Files.createTempFile(temporaryFolder, getClass().getSimpleName(), "-empty.txt"); | ||
|
|
||
| try (InputStream is = Files.newInputStream(emptyFile)) { |
There was a problem hiding this comment.
interesting corner case if you have an empty ByteArrayInputStream and you call read with a 0 size buffer it will return EOF BUT if you have an empty FileInputStream in that same scenario it will return 0.
|
TY @qwerty4030 Good find 👍, merged. |
garydgregory
added a commit
that referenced
this pull request
Jul 4, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix infinite loop in AbstractByteArrayOutputStream.
When an AbstractByteArrayOutputStream is initialized with a 0 size buffer and writeImpl(InputStream) is called an infinite loop may occur and eventually cause an OOM exception.
This is due to passing 0 as the maximum bytes to read in InputStream#read which returns 0, then a new 0 size buffer is created, and then InputStream#read is called again therefore repeating the loop forever.
Fix this infinite loop by updating needNewBuffer to use DEFAULT_SIZE as the initial buffer size instead of 0.
This will allow InputStream#read to properly populate the buffer as expected and terminate the loop when EOF is reached.
Set initial value of currentBufferIndex to -1 so it is properly initialized to 0 when needNewBuffer is called; unrelated to the infinite loop bug.