On Fri, 18 Oct 2024 20:06:14 GMT, Lance Andersen <lan...@openjdk.org> wrote:

>> Please review the changes for 
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a 
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) 
>> which addresses that
>> 
>> - ZipEntry(String)
>> - ZipEntry::setComment
>> - ZipEntry::setExtra
>> 
>> currently validate that the max possiible field size is 0xFFFF(65535) 
>> instead of  0xFFD1(65489) not taking into account the size of the CEN header 
>> which is 46 bytes per the PKWare APP.NOTE 4.4.10, 4.4.11, & 4.4.12 
>> 
>> The CSR has been approved.
>> Mach5 tiers1-3 run clean as do the relevant JCK tests
>
> Lance Andersen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   change field -> fields

A couple more suggestions for the ZipEntry test.

test/jdk/java/util/zip/ZipEntry/MaxZipEntryFieldSizeTest.java line 28:

> 26:  * @summary Verify that ZipEntry(String), ZipEntry::setComment, and
> 27:  * ZipEntry::setExtra throws a IllegalArgumentException when the
> 28:  * combined length of the fields, including the size of the CEN Header,

A test where the combined length exceeds the limit, but no one field alone 
exceeds the limit would be a nice addition.

test/jdk/java/util/zip/ZipEntry/MaxZipEntryFieldSizeTest.java line 60:

> 58:     static final short UNKNOWN_ZIP_TAG = (short) 0x9902;
> 59:     // ZIP file to be used by the tests
> 60:     static final Path ZIP_FILE = Path.of("ZipEntryFieldSize.zip");

Seems unused?

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

PR Review: https://git.openjdk.org/jdk/pull/21544#pullrequestreview-2378915951
PR Review Comment: https://git.openjdk.org/jdk/pull/21544#discussion_r1806987734
PR Review Comment: https://git.openjdk.org/jdk/pull/21544#discussion_r1806985521

Reply via email to