On Sun, 1 Nov 2020 20:09:32 GMT, Lance Andersen <lan...@openjdk.org> wrote:
> Hi,
>
> Please review the fix for JDK-8255380 which addresses an issue when the Zip
> file is > 4GB. Zip FS when processing the CEN extra data does not take into
> account the fact that there is no specific order to how the extra data fields
> are written. Info-ZIP writes the fields in a different order than Zip FS
> which presents a problem when evaluating the Info-ZIP extended timestamp and
> the LOC offset is 0XFFFFFFFF therefore the LOC offset needs to be read from
> the EXTID_ZIP64 extra data prior to attempting to read the LOC extra data
> field.
>
> The fix will defer reading of the LOC extra data field, if needed until all
> of the CEN extra data has been processed.
>
> Using jdk.nio.zipfs.ZipInfo, you can see the ordering difference of the CEND
> extra data fields when using Zip FS and info-zip.
>
> Info-zip is included with Mac OS so the test uses ProcessBuilder to execute
> zip on Mac OS and Linux.
>
> Mach5 tests jdk-tier1, jdk-tier2, and jdk-tier3 run cleanly.
>
> Best,
> Lance
LGTM. Some nits inline.
I guess the test can be cause for issues in some test systems since it needs to
write out the 4Gb+ file. Is this why you've only enabled it on linux and mac?
Perhaps someone might have ideas on how to improve this.
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 2993:
> 2991: // We need to read the LOC extra data and the LOC offset
> was obtained
> 2992: // from the EXTID_ZIP64 field.
> 2993: if(hasZip64LocOffset) {
Suggestion:
if (hasZip64LocOffset) {
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 2978:
> 2976: // prior to reading the LOC extra data field in
> order to obtain
> 2977: // the Info-ZIP Extended Timestamp.
> 2978: if(locoff != ZIP64_MINVAL) {
Suggestion:
if (locoff != ZIP64_MINVAL) {
-------------
Marked as reviewed by redestad (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/987
