On Mon, 2 Mar 2026 18:43:01 GMT, Jaikiran Pai <[email protected]> wrote:
>> Can I please get a review of this change which addresses the issue noted in >> https://bugs.openjdk.org/browse/JDK-8369181? >> >> As noted in that issue, if `finish()` is called on a `InflaterOutputStream` >> that was constructed without passing a `Inflater`, then any subsequent >> `write()`s on that `InflaterOutputStream` result in an >> `IllegalStateException`, because we close the `Inflater` in `finish()`. The >> commit in this PR, fixes the issue by throwing the specified `IOException` >> in place of the `IllegalStateException`. >> >> At the same time, the documentation of `finish()` has been enhanced to >> clarify the current behaviour, through a `@implSpec`. >> >> Alternative approaches of deprecating finish() and/or not closing the >> default Inflater were considered, but given the current long standing >> implementation of finish(), it was decided to merely specify the current >> behaviour of closing the default Inflater in finish(). >> >> A new jtreg test has been added to reproduce the issue and verify the fix. >> tier1, tier2, tier3 testing of this change completed without any related >> issues. >> >> I'll file a CSR shortly for this change. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > writes any ... instead of writes out any ... src/java.base/share/classes/java/util/zip/InflaterOutputStream.java line 228: > 226: * > 227: * @implSpec This method calls {@link #flush()} to write any pending > buffered > 228: * decompressed data. Do we really want to explicitly specify (enforce?) "finish() -> flush()" here. @implSpec is normative, this may be read as an implementation requirement for finish(). If a subclass override finish() without calling flush() but still provides equivalent output semantics, this wording might overly constraining? Or I'm overthinking this? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29935#discussion_r2874067797
