On Fri, 20 Mar 2026 13:52:06 GMT, Jaikiran Pai <[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.
>>
>> A CSR has been drafted. Its specification section will be updated after an
>> initial round of review of the specification text in this PR.
>
> src/java.base/share/classes/java/util/zip/ZipOutputStream.java line 150:
>
>> 148: * passed to the constructor of this ZipOutputStream
>> instance.
>> 149: */
>> 150: public void setComment(String comment) {
>
> Given what this method currently does and the class level specification which
> states that "unless otherwise noted" a NullPointerException gets thrown if
> null if passed for method params, I think we might have to update this
> documentation like:
>
>
> /**
> * Sets the ZIP file comment.
> *
> * @implSpec Passing {@code null} for {@code comment} will result in
> * the ZIP file having no comment.
> *
> * @param comment the comment string, can be {@code null}
> * @throws IllegalArgumentException if the length of the specified
> * ZIP file comment is greater than 0xFFFF bytes or if the
> * {@code comment} contains characters that cannot be mapped
> * by the {@code Charset} of this {@code ZipOutputStream}
> */
I am unsure if accepting `null` should be a `@implSpec` or just a formal API
specification.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30338#discussion_r2965933879
