RFR: 8336025: Improve ZipOutputSream validation of MAX CEN Header field limits [v2]
Eirik Bjørsnøs
eirbjo at openjdk.org
Mon Sep 16 09:42:06 UTC 2024
On Sun, 15 Sep 2024 13:11:26 GMT, Lance Andersen <lancea at openjdk.org> wrote:
>> Please review the following PR which addresses that ZipOutputStream should validate the CEN header fields similar to what was done via [JDK-8316141](https://bugs.openjdk.org/browse/JDK-8316141)
>>
>> As part of this change, the javadoc for ZipEntry has been updated to indicate that the CEN Header(46 bytes) + entry name length + comment length + extra data length must not exceed 0xfffff.
>>
>> Mach5 tiers 1-3 runs were clean. The zip and jar JCK tests also continue to pass
>
> Lance Andersen has updated the pull request incrementally with one additional commit since the last revision:
>
> Update @link ->@linkplain
src/java.base/share/classes/java/util/zip/ZipEntry.java line 41:
> 39: * This class is used to represent a ZIP file entry.
> 40: * <P>
> 41: * The combined length of the entry name, the extra field data, the
"bytes" is the last word here, so technically this is correct. But an external reader may easily think this refers to the string lengths, not the byte encoded lengths.
So maybe being slightly more explicit that the encoded length depends on the chosen charset would reduce ambiguity?
src/java.base/share/classes/java/util/zip/ZipOutputStream.java line 642:
> 640: if (e.comment != null) {
> 641: commentBytes = zc.getBytes(e.comment);
> 642: clen = Math.min(commentBytes.length, 0xffff);
Moving the headerLength validation earlier in the method would remove the need to this comment truncation, right?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21003#discussion_r1760825827
PR Review Comment: https://git.openjdk.org/jdk/pull/21003#discussion_r1760828839
More information about the core-libs-dev
mailing list