On Sun, 26 Apr 2026 06:27:04 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 three additional 
> commits since the last revision:
> 
>  - merge latest from master branch
>  - minor doc update
>  - 8322256: Define and document GZIPInputStream concatenated stream semantics

Hi Jai,

Sorry for the delay in reviewing.  I made a pass through with some 
questions/suggestions

src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 43:

> 41:  * a header, followed by data that is compressed using the {@code 
> deflate} algorithm, and then
> 42:  * a trailer. The members simply appear one after another in the file, 
> with no additional
> 43:  * information before, between, or after them. An input stream consisting 
> of more than one member

Should the above reference 2.2 of gzip spec?

2.2. File format

      A gzip file consists of a series of "members" (compressed data
      sets).  The format of each member is specified in the following
      section.  The members simply appear one after another in the file,
      with no additional information before, between, or after them.

src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 49:

> 47:  *
> 48:  * @apiNote
> 49:  * The {@link #close} method should be called to release resources used 
> by this stream.

Given this supports AutoClosable, should we expand this note?

src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 137:

> 135:     /**
> 136:      * Creates a new input stream with a default buffer size. The given 
> {@code in}
> 137:      * stream is treated as containing {@linkplain ##gzip_file_format 
> GZIP format data}.

Do we need to restate the stream is expecting GZIP formatted data as I think if 
state this in the file format section it might not need repeating?

src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 176:

> 174:      * from one particular member i.e. a call to {@code read()} will not 
> result in {@code buf}
> 175:      * containing decompressed data from two or more members of a 
> concatenated GZIP stream.
> 176:      *

Did you give any thought of discussing concatenated gzip files right after the 
file format discussion?

Reason I ask as it might be a bit cleaner to have the format discussion and 
processing together>

-------------

PR Review: https://git.openjdk.org/jdk/pull/30925#pullrequestreview-4197721400
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3161689620
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3161641583
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3161720435
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3161741556

Reply via email to