On Sat, 19 Oct 2024 15:52:59 GMT, Lance Andersen <lan...@openjdk.org> wrote:
>> 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?
>
> Sure, I can add an additional comment here if you like for future
> maintainers. I don't want to consider converting name/comments earlier than
> needed as part of this PR as that start to creep out of the scope of the
> original issue being addressed, especially given this is a corner case(a
> corpus search of 90K+ jars only found 520 Zip entries with a header size
> between 500 - 1000 bytes and everything else < 500 bytes).
Good
Yes, converting early might have other compatibility concerns, too.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21544#discussion_r1807417503
