On Thu, 24 Sep 2020 16:36:28 GMT, Brent Christian <bchri...@openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address the review comments and introduce an array size check in >> JarFile.getBytes() method itself > > src/java.base/share/classes/java/util/jar/JarFile.java line 791: > >> 789: private byte[] getBytes(ZipEntry ze) throws IOException { >> 790: try (InputStream is = super.getInputStream(ze)) { >> 791: int len = (int)ze.getSize(); > > I think the main issue is the casting of the 'long' value from > ZipEntry.getSize() into an 'int'. I think checking if > the size is > the maximum array size and throwing an OOME here might be a > sufficient fix on its own. Hello Brent, Thank you for the review and sorry about the delayed response - I was involved in a few other things so couldn't get to this sooner. I have taken your input and updated this patch to address this part. Specifically, I have introduced a new `MAX_BUFFER_SIZE` within the `JarFile`. This `MAX_BUFFER_SIZE` is an actual copy of the field (and value) of `java.io.InputStream#MAX_BUFFER_SIZE`. I have done a minor change to the javadoc of this field as compared to what is in the javadoc of its `InputStream` counterpart. I did this so that the OOM exception message being thrown matches the comment in this javadoc (the `InputStream` has a mismatch in its javadoc and the actual message that gets thrown). Additionally, in this patch, the `if (len != -1 ...)` has been changed to `if (len >= 0 ...)` to prevent the code from entering this block when `Zip64` format is involved where (from what I can gather) an uncompressed entry size value can have 2^64 (unsigned) bytes. ------------- PR: https://git.openjdk.java.net/jdk/pull/323