On Tue, 9 Jun 2026 10:55:03 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this change which proposes to address 
>> https://bugs.openjdk.org/browse/JDK-8385924?
>> 
>> For reasons explained in that issue, the `java.util.zip.GZIPInputStream` 
>> class currently behaves differently depending on which version of Java is in 
>> use. The change in this current PR proposes to introduce a system property 
>> which allows applications to decide whether they prefer the decades old 
>> behaviour of `GZIPInputStream` where it uses `InputStream.available()` for 
>> deciding whether to read a subsequent member in the stream, or whether the 
>> application prefers the more recent behaviour of `GZIPInputStream` where it 
>> skips the `available()` call and instead directly does a read operation for 
>> detecting a subsequent member.
>> 
>> The system property by default isn't set which means that `GZIPInputStream` 
>> will call `InputStream.available()` and thus matches its decades old 
>> behaviour. The introduction of this system property is a stopgap measure as 
>> noted in https://github.com/openjdk/jdk/pull/30925#discussion_r3318466104 :
>> 
>>> For JDK 27, introduce a system property that controls the reliable handling 
>>> of streams with more than one member. Long standing behavior does not 
>>> handle the case where the underlying stream doesn't have a useful 
>>> avaialble() method and it additionally swallows any exception when reading 
>>> ahead. The system property can be documented as an implNote in the 2-arg 
>>> constructor or class description. The property will need default to long 
>>> standing behavior (not the more recent/changed behavior).
>> 
>> As noted in that same comment, the long term solution involves enhancing the 
>> `GZIPInputStream` class to introduce a constructor which allows applications 
>> to decide how the `GZIPInputStream` should behave when dealing with multiple 
>> members:
>> 
>>> For JDK 28 or later, introduce a new constructor to create a 
>>> GZIPInputStream that works like it should have done in JDK 1.1 when the API 
>>> was originally introduced. We discussed two possible sets of parameters. We 
>>> also discussed maybe deprecating the two existing constructors due to the 
>>> issue of treating available==0 as EOF and the issue of swallowing 
>>> exceptions. 
>> 
>> That new API will need careful thought and time and thus the current 
>> proposal to introduce this system property in the meantime.
>> 
>> A new jtreg test has been introduced to verify the usage of this system 
>> property. A CSR will be proposed shortly. Existing tests and this new test 
>> continue to pass with this change.
>> 
>> ---------...
>
> 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 five additional 
> commits since the last revision:
> 
>  - adjust the implNote
>  - replace unset with "not set"
>  - minor code comment updates
>  - merge latest from master branch
>  - 8385924: GZIPInputStream.read() behaves differently on some Java versions

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

> 69:  * property in environments that process streams with a series of 
> members. By default,
> 70:  * the {@code jdk.util.gzip.tryReadAheadAfterTrailer} system property is 
> not set,
> 71:  * and {@code InputStream.available()} gets called.

This is quite good, just a bit uncomfortable with the "may not always be 
accurate". What would you think of this:

"GZIPInputStream depends on a high quality implementation of available() to 
reliably read a stream with a series of members. Consequently it may be 
necessary to set this property in environments that process streams with a 
series of members."

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31396#discussion_r3382003596

Reply via email to