On Thu, 17 Oct 2024 10:44:51 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 back missing putNextEntry call src/java.base/share/classes/java/util/zip/ZipEntry.java line 97: > 95: // for entries in order to not exceed 65,489 bytes minus 46 bytes for > the CEN > 96: // header length > 97: private static final int MAX_NAME_COMMENT_EXTRA_SIZE = Would it make sense to validate the actual combined length here, instead of just pretending that all other components of the combined lenghts are zero-length? We could introduce a method like this: /* * Validate that the combined length of the 46 byte CEN header, the name, * the extra field and the comment does not exceed 0xFFFF */ private void validateCombinedLength(String name, byte[] extra, String comment, String message) { int headerLength = ZipEntry.CENHDR; if (name != null) { headerLength += name.length(); } if (extra != null) { headerLength += extra.length; } if (comment != null) { headerLength += comment.length(); } if (headerLength > 0xFFFF) { throw new IllegalArgumentException(message); } } Then the constructor could use it like this: validateCombinedLength(name, null, null, "entry name too long"); setExtra: validateCombinedLength(name, extra, comment, "invalid extra field length"); setComment: validateCombinedLength(name, extra, comment, "entry comment too long"); ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21544#discussion_r1804594587
