On Tue, 14 Nov 2023 11:49:11 GMT, Lance Andersen <[email protected]> wrote:
>> Eirik Bjorsnos has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add a @bug reference in the test
>
> src/java.base/share/classes/java/util/zip/ZipInputStream.java line 581:
>
>> 579: if ((flag & 8) == 8) {
>> 580: /* "Data Descriptor" present */
>> 581: if (hasZip64Extra(e) ||
>
> You probably want to consider updating `readLOC` to make sure the extralen is
> != 0 if the appropriate fields are set to either 0xFFFF or 0xFFFFFFFF or
> update `hasZip64Extra` to do the validation
I think I prefer keeping this PR maintaining a strict focus on expanding the
set of readable files to include those that use Zip64 extra fields for < 2GB
entries with data descriptors.
Would you be ok with that?
Adding validation to `readLOC` is a fair effort, but I would prefer this to be
done in a separate PR, similar to your work on adding Zip64 validation to
ZipFile.
I wouldn't mind looking into that, but perhaps you would like to handle it,
given your comment above about spending some time on `ZipInputStream` in the
following days?
> src/java.base/share/classes/java/util/zip/ZipInputStream.java line 689:
>
>> 687: return switch (blockSize) {
>> 688: case 8, 16 -> true;
>> 689: default -> false;
>
> Also from 4.5.3:
>
>> This entry in the Local header MUST include BOTH original and compressed
>> file size fields
>
> So I believe the minimum value is 16 given both fields must be present
So it seems 16 is the only valid size for a Zip64 block in a LOC header.
Updated the code to reflect that and also added a comment referencing that
original and uncompressed sizes are both required in a LOC header.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1394742755
PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1394743998
