RFR: 8340553: ZipEntry field validation does not take into account the size of a CEN header [v9]
Claes Redestad
redestad at openjdk.org
Sat Oct 19 15:41:48 UTC 2024
On Fri, 18 Oct 2024 21:24:18 GMT, Lance Andersen <lancea at 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
More information about the core-libs-dev
mailing list