[jdk8u-dev] RFR: 8186464: ZipFile cannot read some InfoZip ZIP64 zip files

[Andrew John Hughes]

On Mon, 8 Apr 2024 16:09:30 GMT, fitzsim <duke at openjdk.org> wrote:

>> This is a re-do of #445 as we reached a deadlock where I could not make myself commit author, as I was not the PR author, and Thomas could not make me the author as he was not a Committer (see [SKARA-2173](https://bugs.openjdk.org/browse/SKARA-2173)). The content remains the same.
>> 
>> What follows is Thomas' introduction from the original PR:
>> 
>> This patch was applied to the Red Hat 1.8.0 RPMs in June 2020, so it has been deployed to Red Hat customers for over three years.
>> 
>> I verified that the patch applies cleanly to jdk8u-dev master. I confirmed that with the fix portion of the patch reverted, the ReadZip.java test portion of the patch produces this exception:
>> 
>> ~~~
>> java.lang.RuntimeException: zipfile: zip64 end failed
>> 	at ReadZip.main(ReadZip.java:209)
>> 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>> 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>> 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>> 	at java.lang.reflect.Method.invoke(Method.java:498)
>> 	at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>> 	at java.lang.Thread.run(Thread.java:750)
>> ~~~
>> 
>> With the fix applied the test passes:
>> 
>> ~~~
>> Passed: java/util/zip/ZipFile/ReadZip.java
>> ~~~
>> 
>> With the patch applied on top of jdk8u-dev master tip, 3dc011b7ff955f6c1334058f300708412b21a3ad, `make test` on Fedora 38 x86-64 passes, with:
>> ~~~
>> Test results: passed: 3,122
>> ~~~
>> I also retested the test cases in [JDK-8186464](https://bugs.openjdk.org/browse/JDK-8186464) and confirmed that without this backport, they fail, and with the backport they succeed.
>> 
>> Thank you,
>> Thomas
>
> Here is some public discussion: https://bugzilla.redhat.com/show_bug.cgi?id=1433262#c21 and https://bugzilla.redhat.com/show_bug.cgi?id=1433262#c22

> @fitzsim Yes, please remove the `zip_util.c` changes. Then we can proceed with reviewing. Different question: Is there a reason why you didn't submit this PR for review?

If you remove the `zip_util.c` changes, you remove the core of the fix. The `zip_util.c` changes are equivalent to the `ZipFile.java` in the 9+ version of this patch. 8u does not have [JDK-8145260](https://bugs.openjdk.org/browse/JDK-8145260): "To bring j.u.z.ZipFile's native implementation to Java to remove the expensive jni cost and mmap crash risk" and this is not a suitable candidate for backport to a stable release.

The test and zipfs changes are the same. The reason I didn't upstream this at the time of development was I was unsure that the test was adequately testing the changes, particularly the ZipFileSystem ones. In 9+, the ZipFileSystem is part of the JDK, thanks to [JDK-8038500](https://bugs.openjdk.org/browse/JDK-8038500): "(zipfs) Upgrade ZIP provider to be a supported provider" and so I presume is tested by the `FileSystem fs = FileSystems.newFileSystem(uri, Map.of())` code in a way it isn't when it's demo code.

Thomas kindly agreed to verify the tests and upstream the patch. I think the 'clean patch' confusion arose from the RPM patch applying cleanly, not the 11u version.

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

PR Comment: https://git.openjdk.org/jdk8u-dev/pull/452#issuecomment-2078410441