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