Add toByteArray(InputStream input, int size, int bufferSize)

[ppkarwasz]

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.

• I used AI to create the Javadoc.
• Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
• 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.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• 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.

[garydgregory]

Hi @ppkarwasz

I have lots of comments! 😉

[ppkarwasz]

@garydgregory,

I removed a lot of details from the Javadoc and kept only these:

• For toByteArray(InputStream):
  

  commons-io/src/main/java/org/apache/commons/io/IOUtils.java

  Lines 2643 to 2644 in c6c79a3

  	* <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>

• For toByteArray(InputStream, int/long):
  

  commons-io/src/main/java/org/apache/commons/io/IOUtils.java

  Lines 2666 to 2667 in c6c79a3

  	* <p>The method allocates a single array of the requested size and fills it directly
  	* from the stream.</p>

• For toByteArray(InputStream, int, int):
  

  commons-io/src/main/java/org/apache/commons/io/IOUtils.java

  Lines 2708 to 2709 in c6c79a3

  	* <p>The method accumulates the data in temporary buffers of size at most {@code bufferSize}
  	* and returns a single array containing the entire contents once the end of the stream is reached.</p>

I believe that these behaviors will not change and should remain in the method contracts, what do you think?

[garydgregory]

Hi @ppkarwasz ,
I have comments scattered about.
TY!

[ppkarwasz]

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 Add toByteArray(InputStream input, int size, int bufferSize) #776 (comment))? The toByteArray(InputStream, int, int) method explicitly promises chunks up to chunkSize, but the others do not.

[garydgregory]

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))? 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?

[garydgregory]

Hello @ppkarwasz

I added some comments.

[ppkarwasz]

JDK 11+ uses 16 KiB.

Where do you see that?

In the source code of InputStream. This is the value used by readAllBytes/readNBytes.

[MarkEWaite]

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. The major regression was:

• [JENKINS-76295] SSH agents stopped working on Jenkins 2.537 jenkinsci/jenkins#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 released Jenkins 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.

I've attached a tar archive of the git repository where I've been able to duplicate the issue.

gh-16845.tar.gz

[ppkarwasz]

Hi @MarkEWaite,

I can’t reproduce it locally, but the stack trace points to SFTPInputStream#read (source):

@Override
public int read(byte[] b, int off, int len) throws IOException {
    int r = SFTPClient.this.read(h, offset, b, off, len);
    if (r < 0) return -1;
    offset += r;
    return r;
}

This method doesn’t fully follow the InputStream#read contract, particularly when len == 0 or when len exceeds the SFTP packet-size limit (2^15). The change in this PR alters the read pattern and exposes this bug.

Given that toByteArray uses an 8192-byte buffer, my guess is that a read() call with len == 0 triggers the exception.

This looks like an issue in Trilead rather than Commons IO. I submitted a minimal fix in jenkinsci/trilead-ssh2#273