On Fri, 20 Mar 2026 13:19:49 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.
>> 
>> 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 two additional 
> commits since the last revision:
> 
>  - Make ZIP Path field static final
>  - Typo in "with"

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 1254:

> 1252:             throw (InvalidPathException)
> 1253:                     new InvalidPathException(name, "unmappable 
> character in path name").initCause(e);
> 1254:         }

Are you sure a. cause of IAE make sense here? Okay for ZipOutputStream as the 
argument is a ZipEntry but less obviously here.

Also I assume it will be thrown for both malformed and unmappable cases.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30319#discussion_r2967177589

Reply via email to