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

[Lance Andersen]

On Fri, 31 May 2024 10:09:20 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:
> 
>   enhance the new test to even verify IOException from the constructor

Looks fine overall Jai.

Personally, I would have had each assertThrows  as its own test,  as if one fails(which should not in this case) then the rest of the assertions are never executed until the first failure is addressed.

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

Marked as reviewed by lancea (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19475#pullrequestreview-2090494111