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