On Wed, 23 Sep 2020 15:06:55 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

> Can I please get a review and a sponsor for a fix for 
> https://bugs.openjdk.java.net/browse/JDK-8242882?
> 
> As noted in that JBS issue, if the size of the Manifest entry in the jar 
> happens to be very large (such that it exceeds
> the `Integer.MAX_VALUE`), then the current code in `JarFile#getBytes` can 
> lead to a `NegativeArraySizeException`.  This
> is due to the: if (len != -1 && len <= 65535)  block which evaluates to 
> `true` when the size of the manifest entry is
> larger than `Integer.MAX_VALUE`. As a result, this then ends up calling the 
> code which can lead to the
> `NegativeArraySizeException`.  The commit in this PR fixes that issue by 
> changing those `if/else` blocks to prevent
> this issue and instead use a code path that leads to the 
> `InputStream#readAllBytes()` which internally has the
> necessary checks to throw the expected `OutOfMemoryError`.  This commit also 
> includes a jtreg test case which
> reproduces the issue and verifies the fix.

Changes requested by bchristi (Reviewer).

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.

-------------

PR: https://git.openjdk.java.net/jdk/pull/323

Reply via email to