On Tue, 7 Apr 2026 09:10:41 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 incrementally with one additional
> commit since the last revision:
>
> Avoid "Charset of this ZipOutputStream" since that isn't mentioned in the
> class description
src/java.base/share/classes/java/util/zip/ZipOutputStream.java line 151:
> 149: * comment is greater than 0xFFFF bytes or if the {@code
> comment}
> 150: * contains characters that cannot be mapped by the
> {@code Charset}
> 151: * used to encode entry names and comments
This is OK but perhaps we can tune further as mentioning "entry names" does not
quite feel right in the setComments javadoc
Would it be beneficial to clarify the charset used in the class description?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30338#discussion_r3047154004
