On Thu, 19 Mar 2026 17:24:39 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.
>
> Curiously, `ZipFileOutputStream::setComment` is not specified to throw for an
> unmappable comment. Long standing unspecified behavior is to throw
> IllegalArgumentException. To prevent regressions, a test is added to verify
> this. An alternative path here would be to update the specification with a
> CSR.
src/java.base/share/classes/java/util/zip/ZipOutputStream.java line 268:
> 266: if (e.comment != null) {
> 267: try {
> 268: zc.getBytes(e.comment);
Can you check the other usages of ZipCode.getBytes to see if this merits an
overload of getBytes that throws ZipException rather than IllegalArgument.
These malformed-input and unmappable-character cases will cause
CharacterCodingException to be thrown so it must be caught to throw
IllegalArgument, then caught to throw ZipException, and we should be able to do
better.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30319#discussion_r2961852371
