On Fri, 31 May 2024 11:03:18 GMT, Jaikiran Pai <j...@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 Jai for the update. I think this makes the test much cleaner ------------- Marked as reviewed by lancea (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19475#pullrequestreview-2090569778
