On Fri, 18 Oct 2024 21:24:18 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: > > Add Combined test src/java.base/share/classes/java/util/zip/ZipEntry.java line 723: > 721: * @return true if valid CEN Header size; false otherwise > 722: */ > 723: static boolean isCENHeaderValid(String name, byte[] extra, String > comment) { I think it should be spelled out better that `isCENHeaderValid` can give false positives since both `name` and `comment` may turn into more than `String::length` bytes after conversion. So while it's a reasonable sanity check to fail-fast you still need an exact check after conversion like in `ZipOutputStream::writeCEN`. Or we could consider converting `name` and `comment` to bytes eagerly and get an exact check up front? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21544#discussion_r1807388643
