RFR: 8242882: opening jar file with large manifest might throw NegativeArraySizeException [v2]

[Jaikiran Pai]

On Wed, 30 Sep 2020 17:21:14 GMT, Brent Christian <bchristi at 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 161:
> 
>> 159:      * OutOfMemoryError: Required array size too large
>> 160:      */
>> 161:     private static final int MAX_BUFFER_SIZE = Integer.MAX_VALUE - 8;
> 
> "/**" comments are generally only used for public documentation.  For use here, probably a single line // comment would
> be sufficient to explain what this value is.
> This constant is also named, "MAX_ARRAY_SIZE" in various places, which seems more applicable to this case.

Hello Brent,

I've updated the PR with your suggested changes for this member variable name and the comment.

> src/java.base/share/classes/java/util/jar/JarFile.java line 801:
> 
>> 799:             if (len > MAX_BUFFER_SIZE) {
>> 800:                 throw new OutOfMemoryError("Required array size too large");
>> 801:             }
> 
> I would just add a new `long zeSize` to read and check `ze.getSize()`, and then (int) cast it into `len`, as before.
> Then I think no changes would be needed past L802, `int bytesRead;`

Done. Changed it based on your input.

> test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 78:
> 
>> 76:             bw.write("OOM-Test: ");
>> 77:             for (long i = 0; i < 2147483648L; i++) {
>> 78:                 bw.write("a");
> 
> As you probably noticed, this test takes a little while to run.  One way to speed it up a little would be to write more
> characters at a time.  While we're at it, we may as well make the Manifest well-formed by breaking it into 72-byte
> lines.  See "Line length" under:
> https://docs.oracle.com/en/java/javase/15/docs/specs/jar/jar.html#notes-on-manifest-and-signature-files  Just write
> enough lines to exceed Integer.MAX_VALUE bytes.

I decided to slightly change the way this large manifest file was being created. I borrowed the idea from
`Zip64SizeTest`[1] to create the file and set its length to a large value. I hope that is OK. If not, let me know, I
will change this part.

[1] https://github.com/openjdk/jdk/blob/master/test/jdk/java/util/zip/ZipFile/Zip64SizeTest.java#L121

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

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