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

Reply via email to