RFR: 8272746: ZipFile can't open big file (NegativeArraySizeException) [v2]

[Lance Andersen]

On Fri, 24 Dec 2021 11:07:04 GMT, Masanori Yano <myano at openjdk.org> wrote:

>> Could you please review the JDK-8272746 bug fixes?
>> Since the array index is of type int, the overflow occurs when the value of end.cenlen is too large because of too many entries.
>> It is necessary to read a part of the CEN from the file to fix the problem fundamentally, but the way will require an extensive fix and degrade performance.
>> In practical terms, the size of the central directory rarely grows that large. So, I fixed it to check the size of the central directory and throw an exception if it is too large.
>
> Masanori Yano has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8272746: ZipFile can't open big file (NegativeArraySizeException)

Thank you for your work here.   Have you also run your test using Apache Commons?

I have run this test over 2000 times via Mach5 on various hardware with some runs taking over 12 minutes for the test to complete.  So unless you can refactor the test to be more efficient, we need to make the test a manual test as this is more of a corner case.

Please see the additional comments below

test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java line 28:

> 26:  * @summary ZipFile can't open big file (NegativeArraySizeException)
> 27:  * @requires (sun.arch.data.model == "64" & os.maxMemory > 8g)
> 28:  * @run main/othervm/timeout=3600 -Xmx8g TestTooManyEntries

Please convert to a manual test and use TestNG for the test

test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java line 58:

> 56:                 if (System.currentTimeMillis() >= nextLog) {
> 57:                     nextLog = 30_000 + System.currentTimeMillis();
> 58:                     System.out.printf("Processed %s%% directories (%s), file size is %sMb (%ss)%n",

I would clarify the message to use "entries" vs "directories". as the issue deals with the size of the CEN overall not whether the actual entry is for a file or represents a directory.

You could probably simplify the time check by  just outputting every  "n" entries written

test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java line 70:

> 68:             ZipFile zip = new ZipFile(hugeZipFile);
> 69:         } catch (ZipException ze) {
> 70:             passed = true;

you can use assertThrows here

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

Changes requested by lancea (Reviewer).

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