On Sat, 19 Oct 2024 15:38:26 GMT, Claes Redestad <[email protected]> wrote:
>> 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?
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).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21544#discussion_r1807391884
