On Fri, 17 Jul 2026 10:00:21 GMT, Jaikiran Pai <[email protected]> wrote:

>> Volker Simonis has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix tests to directly read 'GZIPInputStream.alwaysReadNextMember' instead 
>> of relying on the system property 'jdk.util.gzip.tryReadAheadAfterTrailer'
>
> Hello Volker, I had a look at this today. The goal of adding additional 
> coverage in these 2 tests for the always read-ahead vs 
> `InputStream.available()` call is a good idea. However, peeking into an 
> internal implementation field to determine how the `GZIPInputStream` should 
> behave wouldn't be appropriate here. Since the behaviour of `GZIPInputStream` 
> is specified to depend on the `jdk.util.gzip.tryReadAheadAfterTrailer` system 
> property, I think these 2 tests should test the behaviour against that 
> property value. 
> 
>> We do this by downporting 8385924: GZIPInputStream.read() behaves 
>> differently on some Java versions, but with 
>> GZIPInputStream.alwaysReadNextMember defaulting to true. 
> 
> That's a source change to `GZIPInputStream` itself. I think testing for that 
> behaviour should reside in the vendor specific repository, just like that 
> source change itself.

@jaikiran, @AlanBateman of course we can do this change in Corretto 25 (and we 
actually already did it there) or in OpenJDK 25u, but I thought we all agreed 
in the lengthy discussion for  [JDK-8385924: GZIPInputStream.read() behaves 
differently on some Java versions](https://bugs.openjdk.org/browse/JDK-8385924) 
to introduce the new `jdk.util.gzip.tryReadAheadAfterTrailer` property for 
controlling `GZIPInputStream`'s behavior when reading multi-member gzip streams 
because that would make it easy to downport that change to older JDK versions 
which have different defaults for `tryReadAheadAfterTrailer`.

I don't understand why we can't just write the corresponding tests in such a 
way that they work independently of what is the default setting for 
`tryReadAheadAfterTrailer` such that we don't have to change the test every 
time we change the default setting for that property? I hope we agree that both 
defaults for `tryReadAheadAfterTrailer` are legal and valid, so why shouldn't 
the corresponding tests work for both of them?

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

PR Comment: https://git.openjdk.org/jdk/pull/31790#issuecomment-5025900623

Reply via email to