On Mon, 23 Mar 2026 15:04:58 GMT, Eirik Bjørsnøs <[email protected]> wrote:
>> Please review this PR which improves validation of unmappable characters in
>> names in the `ZipFileSystem` and `ZipFileOutputStream` APIs.
>>
>> Currently, `ZipFileSystem::getPath` and `ZipFileOutputStream:putNextEntry`
>> both throw `IllegalArgumentException` when rejecting a path or entry name
>> which cannot be encoded with the given charset.
>>
>> This PR fixes `ZipFileSystem::getPath` to instead throw
>> `InvalidPathException` as specified. Similarly,
>> `ZipOutputStream::putNextEntry` is updated to throw `ZipException` a
>> specified.
>>
>> Related, `ZipOutputStream::putNextEntry` is updated to reject unmappable
>> ZipEntry comments in a similar fashion.
>>
>> This change effectively means that `ZipOutputStream` now encodes names and
>> comments twice, once in `putNextEntry` and second time in `writeCEN` when
>> the stream is closed. An alternative would be to capture the encoded byte
>> arrays in the `XEntry`, however this would increase retained heap memory for
>> large number of entries.
>>
>> During work on this PR, I noticed that ZipFS ZipCoder.toString and
>> ZipCoder.getBytes implementations differ from those in java.util.zip. This
>> PR aligns ZipFS with java.util.zip.
>>
>> New tests are added in the ZipFS and ZipFileOutputStream area to verify that
>> these APIs throw exceptions according to their specifications when faced
>> with unmappable characters.
>
> Eirik Bjørsnøs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Avoid encoding name into bytes twice in putNextEntry, then again in writeLOC
Thank you Eirik. Overall Looks good.
As Alan mentioned we will need a CSR to address the change
src/java.base/share/classes/java/util/zip/ZipOutputStream.java line 264:
> 262: if (e.comment != null) {
> 263: checkEncodable(e.comment, "unmappable character in ZIP entry
> comment");
> 264: }
I think this is OK. I have not come across many Zip files which include a
comment within a CEN Header file comment
-------------
Marked as reviewed by lancea (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/30319#pullrequestreview-3994265843
PR Review Comment: https://git.openjdk.org/jdk/pull/30319#discussion_r2977228399
