On Thu, 24 Sep 2020 16:36:28 GMT, Brent Christian <[email protected]> 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
