On Mon, 8 Jun 2026 17:28:28 GMT, Alan Bateman <[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. >> >> ---------... > > src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 69: > >> 67: * is performed on the underlying stream for a subsequent member. By >> default, >> 68: * the {@code jdk.util.gzip.tryReadAheadAfterTrailer} system property is >> unset, and >> 69: * {@code InputStream.available()} gets called. > > The implNote will likely move to the 2-arg constructor when a new constructor > is proposed. > > I think your wording is quite good. A suggestion is to replace > > "and a read is performed on the underlying stream for a subsequent member." > > with > > "and the implementation instead attempts to read a subsequent member in the > stream. It may be necessary to set this property in environments that process > streams with a series of members." > > When the new constructor arrives then it will be possible to update the > guidance. I can live with this proposal, although I'd still prefer to make the actual issue a little bit more explicit, e.g.: "and the implementation instead attempts to read a subsequent member in the stream. It may be necessary to set this property in environments that need to reliably process streams with a series of members, independently on the underlying input stream's implementation of `available()`." ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31396#discussion_r3375289917
