On Mon, 6 Apr 2026 05:11:28 GMT, Eirik Bjørsnøs <[email protected]> wrote:
>> Please review this PR which updates the specification of
>> `ZipOutputStream.setComment(String)` to match the long-standing behavior of
>> throwing `IllegalArgumentException` when a comment contains characters which
>> are unmappable using the `Charset` passed in the constructor.
>>
>> A new tests is added to reproduce calling setComment with unmappable
>> characters and verify that it throws IAE.
>>
>> PR work revealed that setComment does not explicitly specify behavior if the
>> comment string is null or an empty string. The method spectifications of
>> `ZipOutputStream::setComment` and `ZipEntry::setComment` are updated to
>> clarify the consequences of such string values.
>>
>> A CSR has been drafted. Its specification section will be updated after an
>> initial round of review of the specification text in this PR.
>
> Eirik Bjørsnøs has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains nine additional
> commits since the last revision:
>
> - Merge branch 'master' into zos-setcomment-unmappable
> - Update copyright year
> - Add null / empty string note to ZipEntry::setComment. Use "an empty
> string" consistently.
> - Lift null and empty string note into the method description
> - Change "the empty string" to "an empty string"
> - Remove unused import
> - Add note to @param comment specification that the comment may be set to
> null or the empty string to produce no ZIP file comment
> - Update spec according to review suggestion
> - Update ZipOutputStream.setComment to declare that it throws
> IllegalArgumentException for unmappable characters in comment
src/java.base/share/classes/java/util/zip/ZipOutputStream.java line 151:
> 149: * ZIP file comment is greater than 0xFFFF bytes or if the
> 150: * {@code comment} contains characters that cannot be
> mapped
> 151: * by the {@code Charset} of this {@code ZipOutputStream}
"the Charset of this ZipOutputStream". Maybe we can do better and say the
Charset used to encode entry names and comments. This is because there is
nothing in the ZipOutputStream class description that says anything about
Charsets, one only learns about it when looking at the 2-arg constructor.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30338#discussion_r3043231976
