RFR: 8340553: ZipEntry field validation does not take into account the size of a CEN header [v4]

Eirik Bjørsnøs eirbjo at openjdk.org
Fri Oct 18 13:32:38 UTC 2024


On Fri, 18 Oct 2024 12:58:17 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:
> 
>   Added additional validation to ZipEntry

Looks good. See comment about `headerSize` overflow plus some other minor comments.

Would this change in the `ZipEntry` API description require a re-approval of the CSR, even if just a formality?

src/java.base/share/classes/java/util/zip/ZipEntry.java line 106:

> 104:      * @throws NullPointerException if the entry name is null
> 105:      * @throws IllegalArgumentException if the combined length of the entry name
> 106:      * and {@linkplain #CENHDR CEN Header size} exceeds 65,535 bytes.

English is not my native language, but should 'CEN header size" be preceded by the definite article "the", here and in setExtra/setComment ?

src/java.base/share/classes/java/util/zip/ZipEntry.java line 652:

> 650:      * @param comment the comment string
> 651:      * @throws IllegalArgumentException if the combined length
> 652:      * of the specified entry comment, {@linkplain #getName() entry name},

'entry name' is preceded by the definite article 'the' in `setExtra`, but not here. Would be good to be consistent.

-------------

PR Review: https://git.openjdk.org/jdk/pull/21544#pullrequestreview-2378084998
PR Review Comment: https://git.openjdk.org/jdk/pull/21544#discussion_r1806487837
PR Review Comment: https://git.openjdk.org/jdk/pull/21544#discussion_r1806492209


More information about the core-libs-dev mailing list