8186464: ZipFile cannot read some InfoZip ZIP64 zip files

[fitzsim]

We would like to backport JDK-8186464: ZipFile cannot read some InfoZip ZIP64 zip files that was fixed during the development of OpenJDK 10.

(This is a third attempt at #452, hopefully this time with clearer background and context.)

Background

In the OpenJDK 9 timeframe, the java.util.zip.ZipFile was rewritten to be pure Java, by:

JDK-8145260 To bring j.u.z.ZipFile's native implementation to Java to remove the expensive jni cost and mmap crash risk

This rewrite was never backported to OpenJDK 8 and would be too invasive and risky to backport in its entirety now.

This means that OpenJDK 8's java.util.zip.ZipFile implementation is backed by JNI C code, implemented in jdk8u/jdk/src/share/native/java/util/zip/zip_util.c (core algorithms) and jdk8u/jdk/src/share/native/java/util/zip/ZipFile.c (JNI wrapper C code).

ZipFile.c was deleted during development of OpenJDK 9, in the JDK-8145260 commit:

openjdk/jdk9u@2b91819

OpenJDK 8 also contains Zip algorithms in a pure Java demo library, under jdk8u/jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs, added during development of OpenJDK 7, by:

JDK-6990846 Demo: NIO.2 filesystem provider for zip/jar archives

During development of OpenJDK 9, this pure Java code was promoted from being a demo package to being a fully-supported package, by:

JDK-8038500: (zipfs) Upgrade ZIP provider to be a supported provider

It remains demo-only in OpenJDK 8, and there are no plans to backport the promotion change to OpenJDK 8.

Desired Java-to-C backport, example RPM patch

The original JDK-8186464 fix was made against the pure Java ZipFile.java implementation.

The same logic of the pure Java JDK-8186464 fixes applies readily to the core C algorithms in zip_util.c. Andrew has converted the Java code in the original fix to C code applied to zip_util.c.

OpenJDK 8 users have run into this issue in real-world applications so in our Red Hat RPMs, since 2020, we have been shipping and maintaining "downstream" a backport patch.

This Red Hat internal backport patch, in addition to fixing the C code, also updates the pure Java ZipFileSystem.java code which was affected by the same issue reported in JDK-8186464.

ZipFileSystem.java has demo status in OpenJDK 8, but was promoted to production status in OpenJDK 9. The pure Java ZipFileSystem.java part of the backport patch is somewhat incidental, since, as established in the background section, the production/important Zip algorithms in OpenJDK 8 are in the C implementation backing ZipFile.java. We suspect ZipFileSystem.java is little-used by OpenJDK 8 users.

These changes have been deployed to Red Hat customers since 2020, with minor updates from time-to-time as required to keep the patch applicable.

Here is the most recent patch, written and maintained by Andrew Hughes:

https://gitlab.com/redhat/centos-stream/rpms/java-1.8.0-openjdk/-/raw/openjdk-portable-centos-9/jdk8186464-rh1433262-zip64_failure.patch?ref_type=72646f68c8ccb1bdbee3d2249abccf90980eb7d6

Specific Backport Notes

ReadZip.java

On jdk11u-dev master branch, ReadZip.java has these changes beyond what is currently on jdk8u-dev master (ordered oldest to newest):

JDK-8145260: To bring j.u.z.ZipFile's native implementation to Java to remove the expensive jni cost and mmap crash risk [2]
JDK-6233323: ZipEntry.isDirectory() may return false incorrectly 8144977: Class.getResourceAsStream("directory") in JAR returns broken InputStream
JDK-8187443: Forest Consolidation: Move files to unified layout
JDK-8186464: ZipFile cannot read some InfoZip ZIP64 zip files
JDK-8199616: Fix @module declarations in tier1 tests

Meanwhile, jdk8u-dev master saw this change, which did not go to jdk11u-dev master:

JDK-8184993: Jar file verification failing with SecurityException: digest missing xxx

Backporting the 8186464-specific ReadZip.java hunks required adjustments throughout the file and a location change due to forest consolidation.

The net result is that the two tests of ZIP64 end records are the same in jdk8u-dev and jdk11u-dev, other than the use of 11 syntax conveniences Map.of and IOUtils.readAllBytes.

ZipFileSystem.java

On jdk11u-dev master, ZipFileSystem.java has been through three
location changes since it had the same location as it does today on
jdk8u-dev master:

JDK-6989148: (fs) zip provider should be available "out of the box"
=> ./jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipFileSystem.java

JDK-8038500: (zipfs) Upgrade ZIP provider to be a supported provider
=> ./jdk/src/share/classes/jdk/nio/zipfs/ZipFileSystem.java

JDK-8054834: Modular Source Code
=> ./jdk/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java

JDK-8187443: Forest Consolidation: Move files to unified layout
=> ./src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java

There are too many divergences between jdk8u-dev and jdk11u-dev to list, so this rework also had to be done fully by hand.

The net result is that with this jdk8u-dev patch the ZipFileSystem.findEND function identical to jdk11u-dev.

ZipFile.java

"Initial load"
=> ./jdk/src/share/classes/java/util/zip/ZipFile.java

JDK-8054834: Modular Source Code
=> ./jdk/src/java.base/share/classes/java/util/zip/ZipFile.java

JDK-8187443: Forest Consolidation: Move files to unified layout
=> ./src/java.base/share/classes/java/util/zip/ZipFile.java

As mentioned in the background section, this is the file that required unique treatment, a Java-to-C port, for jdk8u-dev.

Andrew ported this Java code (annotated for this pull request with [comment: ...] blocks:

    // Found ENDSIG header
    byte[] endbuf = Arrays.copyOfRange(buf, i, i + ENDHDR);
    end.centot = ENDTOT(endbuf);
    end.cenlen = ENDSIZ(endbuf);
    end.cenoff = ENDOFF(endbuf);
    end.endpos = pos + i;

    // [comment: ...]
    // [comment: elide unchanged comment handling logic, present in C implementation's verifyEND function]
    // [comment: ...]

    // [comment: findEND64 contains the following zip64 end of central directory search logic]

    // must check for a zip64 end record; it is always permitted to be present
    try {
        byte[] loc64 = new byte[ZIP64_LOCHDR];
        if (end.endpos < ZIP64_LOCHDR ||
            readFullyAt(loc64, 0, loc64.length, end.endpos - ZIP64_LOCHDR)
            != loc64.length || GETSIG(loc64) != ZIP64_LOCSIG) {
            return end;
        }
        long end64pos = ZIP64_LOCOFF(loc64);
        byte[] end64buf = new byte[ZIP64_ENDHDR];
        if (readFullyAt(end64buf, 0, end64buf.length, end64pos)
            != end64buf.length || GETSIG(end64buf) != ZIP64_ENDSIG) {
            return end;
        }

        // [comment: anchor comment 1, same in C implementation]

        // end64 candidate found,
        long cenlen64 = ZIP64_ENDSIZ(end64buf);
        long cenoff64 = ZIP64_ENDOFF(end64buf);
        long centot64 = ZIP64_ENDTOT(end64buf);
        // double-check
        if (cenlen64 != end.cenlen && end.cenlen != ZIP64_MAGICVAL ||
            cenoff64 != end.cenoff && end.cenoff != ZIP64_MAGICVAL ||
            centot64 != end.centot && end.centot != ZIP64_MAGICCOUNT) {
            return end;
        }

        // [comment: anchor comment 2, same in C implementation]

        // to use the end64 values
        end.cenlen = cenlen64;
        end.cenoff = cenoff64;
        end.centot = (int)centot64; // assume total < 2g
        end.endpos = end64pos;
    } catch (IOException x) {}    // no zip64 loc/end
    return end;

to this C code:

   /* Get position and length of central directory */
    cenlen = ENDSIZ(endbuf);
    cenoff = ENDOFF(endbuf);
    total  = ENDTOT(endbuf);
    unsigned char end64buf[ZIP64_ENDHDR];
    if ((end64pos = findEND64(zip, end64buf, endpos)) != -1) {

        // [comment: anchor comment 1, same in Java implementation]

        // end64 candidate found,
        cenlen64 = ZIP64_ENDSIZ(end64buf);
        cenoff64 = ZIP64_ENDOFF(end64buf);
        centot64 = ZIP64_ENDTOT(end64buf);
        // double-check

        // [comment: logic is reversed in C function, to use 64-bit vaules]

        if ((cenlen64 == cenlen || cenlen == ZIP64_MAGICVAL) &&
            (cenoff64 == cenoff || cenoff == ZIP64_MAGICVAL) &&
            (centot64 == total || total == ZIP64_MAGICCOUNT)) {

            // [comment: anchor comment 2, same in Java implementation]

            // to use the end64 values
            cenlen = cenlen64;
            cenoff = cenoff64;
            total64 = centot64;
            /* ZIP64 size, offset and total-count fields are unsigned 64-bit
             * values. Sizes and offsets that do not fit in signed jlong
             * (i.e., >= 2^63), or total values that do not fit in jint, are
             * not supported and indicate a corrupt or invalid zip file.
             */
            if (cenlen < 0 || cenoff < 0 || total64 < 0 || total64 > INT_MAX) {
                ZIP_FORMAT_ERROR("Zip64 END values exceed supported size");
            }
            total = (jint)total64;
            endpos = end64pos;
            endhdrlen = ZIP64_ENDHDR;
        }
    }

The older compiler on the Microsoft Windows x32 build machine needed the end64buf declaration to be at the start of the function; I made that change on top of Andrew's patch.

Testing

When working on #442 I also tested this same code with the test steps in the description of the JBS issue JDK-8186464: ZipFile cannot read some InfoZip ZIP64 zip files.

I re-confirmed that the changes to ReadZip.java exercise the new code zip_utils.c code and produce correct results.

CI runs showed no regressions due to this patch. There are unrelated certificate issues in some of the tests that cause the test runs to fail, for example:

security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#buypassclass3ca
security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#godaddyrootg2ca
security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#microsoftecc2017
security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#sslrooteccca
security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#sslrootrsaca
security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#starfieldrootg2ca

Newer OpenJDK versions and zip_util.c

zip_util.c still survives in newer versions up to and including tip (i.e., OpenJDK 27).

It is used to load Zip files from the bootstrap classpath (-Xbootclasspath), which happens during JVM bring-up, prior to Java features (such as the pure Java ZipFile implementation) being available.

Conceivably newer versions of zip_util.c could be patched in the same was as proposed here, however no one has ever needed to load InfoZip ZIP64 zip files on the bootstrap classpath.

There was not much interest when I proposed this in:

JDK-8334048: -Xbootclasspath can not read some ZIP64 zip files
openjdk/jdk#19678

(This could have subsequently become a true backport to OpenJDK 8 zip_util.c.)

In summary, given that no one needs this code in zip_util.c in new OpenJDK versions due to zip_util.c only being used in very specific circumstances, and given its importance in OpenJDK 8 as backing the sole production Zip API, I think it makes more sense just to patch zip_util.c only in OpenJDK 8 directly as we're proposing in this pull request.

---

• I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• JDK-8186464 needs maintainer approval

Issue

• JDK-8186464: ZipFile cannot read some InfoZip ZIP64 zip files (Bug - P3)

Contributors

• Andrew John Hughes <andrew@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/835/head:pull/835
$ git checkout pull/835

Update a local copy of the PR:
$ git checkout pull/835
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/835/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 835

View PR using the GUI difftool:
$ git pr show -t 835

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/835.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back fitzsim! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

This backport pull request has now been updated with issue from the original commit.

[fitzsim]

/contributor add Andrew John Hughes andrew@openjdk.org

[openjdk]

@fitzsim
Contributor Andrew John Hughes <andrew@openjdk.org> successfully added.

[fitzsim]

The latest CI run had the same certificate issues I mentioned in the description, and a new error running RedefineCrossEvent.java on Microsoft Windows x86 jdk/tier1 which seems unrelated to this change. I will mark this as ready for review now.

[mlbridge]

Webrevs

• 00: Full (2990ec54)