RFR: 8303866: Allow ZipInputStream.readEnd to parse small Zip64 ZIP files

[Lance Andersen]

On Sun, 12 Feb 2023 15:41:55 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:

> ZipInputStream.readEnd currently assumes a Zip64 data descriptor if the number of compressed or uncompressed bytes read from the inflater is larger than the Zip64 magic value.
> 
> While the ZIP format  mandates that the data descriptor `SHOULD be stored in ZIP64 format (as 8 byte values) when a file's size exceeds 0xFFFFFFFF`, it also states that `ZIP64 format MAY be used regardless of the size of a file`. For such small entries, the above assumption does not hold.
> 
> This PR augments ZipInputStream.readEnd to also assume 8-byte sizes if the ZipEntry includes a Zip64 extra information field. This brings ZipInputStream into alignment with the APPNOTE format spec:
> 
> 
> When extracting, if the zip64 extended information extra 
> field is present for the file the compressed and 
> uncompressed sizes will be 8 byte values.
> 
> 
> While small Zip64 files with 8-byte data descriptors are not commonly found in the wild, it is possible to create one using the Info-ZIP command line `-fd` flag:
> 
> `echo hello | zip -fd > hello.zip`
> 
> The PR also adds a test verifying that such a small Zip64 file can be parsed by ZipInputStream.

Hi Eirik,

Thank you for your continued work in this area.

I have not had a chance to test this yet but will early next week.  Are you aware of any tools that would create this scenario as to the best of my knowledge we have not encountered one that does as of yet?


The comments below are mainly to provide more clarity for future maintainers.

src/java.base/share/classes/java/util/zip/ZipInputStream.java line 578:

> 576:         if ((flag & 8) == 8) {
> 577:             /* "Data Descriptor" present */
> 578:             if (hasZip64Extra(e)

A comment would be useful here as well

src/java.base/share/classes/java/util/zip/ZipInputStream.java line 627:

> 625:         }
> 626:     }
> 627:     // Returns true if the ZipEntry has a ZIP64 extended information extra field

Please add a comment which clarifies the purpose of this method(such as what you are traversing...etc) and cite the section of the APP.NOTE that is being referenced for future maintainers (similar to the description you created)

src/java.base/share/classes/java/util/zip/ZipInputStream.java line 640:

> 638:                     break; // Invalid size
> 639:                 }
> 640:                 i += size + (2 * Short.BYTES);

Probably  could assign ` 2 * Short.BYTES ` to a variable or create a constant.

test/jdk/java/util/zip/ZipInputStream/Zip64DataDescriptor.java line 50:

> 48: public class Zip64DataDescriptor {
> 49: 
> 50:     private byte[] zip;

Please add a comment to the purpose of the field

test/jdk/java/util/zip/ZipInputStream/Zip64DataDescriptor.java line 89:

> 87:                 0000b20000000000000001000000504b050600000000010001005c000000
> 88:                 560000000000""";
> 89: 

It would be useful to describe how the Zip was created in case it needs to be recreated at a future date

test/jdk/java/util/zip/ZipInputStream/Zip64DataDescriptor.java line 140:

> 138:     }
> 139: 
> 140:     private void setExtraSize(short invalidSize) {

Please add a comment describing the method (and any others without).  As we add new tests, we are trying to. follow this practice the best that we can.

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

PR Review: https://git.openjdk.org/jdk/pull/12524#pullrequestreview-1305594238
PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1111822893
PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1111822289
PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1111821880
PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1120663321
PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1119159694
PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1120661558