On Tue, 5 May 2026 12:26:45 GMT, Jaikiran Pai <[email protected]> wrote:
>> Can I please get a review of this doc and test-only change for >> https://bugs.openjdk.org/browse/JDK-8322256? >> >> The `java.util.zip.GZIPInputStream` has been in the JDK since Java 1.1. >> However, its specification hasn't been clear on how it behaves, especially >> when a `InputStream` consists of more than one GZIP member. >> >> The commit in this PR updates the specification of this class to match its >> current (long standing) implementation. >> >> A new jtreg test has been introduced to verify this behaviour. >> >> I'll draft a CSR shortly. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Jaikiran Pai 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 14 additional > commits since the last revision: > > - Lance's review > - merge latest from master branch > - introduce test to verify read() throws IOException when invoked on closed > stream > - specify existing behaviour of throwing IOException when stream is already > closed > - merge latest from master branch > - specify that GZIPInputStream is not thread-safe > - refer to section 2.2 of the RFC > - remove additional sentence from constructor documentation > - Lance's review - clarify implicit vs explicit close() > - merge latest from master branch > - ... and 4 more: https://git.openjdk.org/jdk/compare/81bb87f9...f3b4d725 Hi Jai, Overall looks better. A couple of minor suggestions to consider src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 53: > 51: * <p> > 52: * When {@linkplain #read(byte[], int, int) reading}, this class may read > ahead in the underlying > 53: * stream while completing a member or determining whether another member > follows. Consequently, > When {@linkplain #read(byte[], int, int) reading}, this class may read ahead > in the underlying I think the above needs a bit of word smithing as "When reading, this class may read...." as it feels "reading" and then "read" seem a bit redundant. If you prefer to not use "When processing the GZIP Stream", which is fine, we just need a different opening to the sentence. src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 56: > 54: * an unspecified number of bytes beyond a member’s trailer may be > consumed. If the bytes read > 55: * ahead do not constitute a valid header for a subsequent member, the > stream is considered to > 56: * have reached end-of-stream, and {@code read()} returns {@code -1}. > {@code read()} returns I think this would be better something like "the read method" given readTrailer(), which is called via read(byte[], int, int) ------------- PR Review: https://git.openjdk.org/jdk/pull/30925#pullrequestreview-4228150129 PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3188560690 PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3188590401
