On Fri, 24 Dec 2021 11:07:04 GMT, Masanori Yano <my...@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
