On Sat, 16 May 2026 07:21:09 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 19 additional > commits since the last revision: > > - Alan's review - update the class level doc > - merge latest from master branch > - merge latest from master branch > - merge latest from master branch > - minor adjustments > - 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 > - ... and 9 more: https://git.openjdk.org/jdk/compare/139e2cf4...40a57962 src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 42: > 40: * The GZIP file format is specified by RFC 1952. The format, as > specified in section 2.2 of > 41: * the RFC, consists of a series of "members" that appear one after > another in the stream with > 42: * no additional information before, between, or after. Each member > consists of a header, "or after" -> "or after them" ? src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 43: > 41: * the RFC, consists of a series of "members" that appear one after > another in the stream with > 42: * no additional information before, between, or after. Each member > consists of a header, > 43: * followed by data that is compressed using the {@code deflate} > algorithm, and then a trailer. can "deflate" link. src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 52: > 50: * in a single read operation. > 51: * <p> > 52: * When {@linkplain #read(byte[], int, int) processing the GZIP stream}, > this class may read I think it might be better to combine this paragraph with the previous as they are both speaking about the read method. If you combine them then it could first speak about "processing the GZIP stream" by reading data from at most one member. I suspect it will flow better if you could make that structural change. src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 54: > 52: * When {@linkplain #read(byte[], int, int) processing the GZIP stream}, > this class may read > 53: * ahead in the underlying stream while completing a member or > determining whether another > 54: * member follows. Consequently, an unspecified number of bytes beyond a > member’s trailer The previous text has set the expectation that the read method yields data from at most one member. Now we find out that the read method any read into the next member (or into non-member bytes that follow a member). The picture you are trying to present here is that a GZIPInputStream may read ahead under the covers and we need to find the right words to explain it. src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 55: > 53: * ahead in the underlying stream while completing a member or > determining whether another > 54: * member follows. Consequently, an unspecified number of bytes beyond a > member’s trailer > 55: * may be consumed. If the bytes read ahead do not constitute a valid > header for a "do not constitute a valid header". Do you mean the IDentification bytes or what validation is really done here? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3257570721 PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3257597332 PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3257582688 PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3257612344 PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3257632452
