RFR: 8210471: GZIPInputStream constructor could leak an un-end()ed Inflater [v4]

[Jaikiran Pai]

On Fri, 31 May 2024 11:03:18 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this change which proposes to address the issue noted in https://bugs.openjdk.org/browse/JDK-8210471?
>> 
>> `java.util.zip.Inflater` when instantiated will hold on the native resources which are freed only when `Inflater.end()` is called. The constructor of `java.util.zip.GZIPInputStream` instantiates an `Inflater` for its internal use. After instantiating the `Inflater`, the `GZIPInputStream` constructor before returning from the constructor, can run into either a `IllegalArgumentException` (because the `size` is `<=0`) or an `IOException` when trying to read a GZIP header from the underlying `InputStream`. In either of these cases, the `Inflater` that the `GZIPInputStream` created internally will end up leaking and the caller has no way to `end()` that `Inflater` or even knowledge of that `Inflater`.
>> 
>> The commit in this PR catches the `IOException` when reading the GZIP header and `end()`s the `Inflater`. Furthermore, to prevent instantiation of an `Inflater`, the `GZIPInputStream` constructor before calling `super(...)` will now check the `InputStream` to be non-null and `size` to be `>0`, both of which were being done by the `super` constructor.
>> 
>> Given the nature of the change, no new test has been added. Existing tests in `test/jdk/java/util/zip` continue to pass and tier1, tier2 and tier3 testing is in progress.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   rework the test to make it parameterized

Thank you Lance for the review. tier testing went fine with the latest state in this PR. I'll go ahead and integrate this now.

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

PR Comment: https://git.openjdk.org/jdk/pull/19475#issuecomment-2142193675