8210471: GZIPInputStream constructor could leak an un-end()ed Inflater

Reviewed-by: lancea

Author: jaikiran

src/java.base/share/classes/java/util/zip/GZIPInputStream.java

@@ -31,6 +31,7 @@
 import java.io.InputStream;
 import java.io.IOException;
 import java.io.EOFException;
+import java.util.Objects;
 
 /**
  * This class implements a stream filter for reading compressed data in
@@ -75,9 +76,29 @@ private void ensureOpen() throws IOException {
      * @throws    IllegalArgumentException if {@code size <= 0}
      */
     public GZIPInputStream(InputStream in, int size) throws IOException {
-        super(in, in != null ? new Inflater(true) : null, size);
+        super(in, createInflater(in, size), size);
         usesDefaultInflater = true;
-        readHeader(in);
+        try {
+            readHeader(in);
+        } catch (IOException ioe) {
+            this.inf.end();
+            throw ioe;
+        }
+    }
+
+    /*
+     * Creates and returns an Inflater only if the input stream is not null and the
+     * buffer size is > 0.
+     * If the input stream is null, then this method throws a
+     * NullPointerException. If the size is <= 0, then this method throws
+     * an IllegalArgumentException
+     */
+    private static Inflater createInflater(InputStream in, int size) {
+        Objects.requireNonNull(in);
+        if (size <= 0) {
+            throw new IllegalArgumentException("buffer size <= 0");
+        }
+        return new Inflater(true);
     }
 
     /**

test/jdk/java/util/zip/GZIP/BasicGZIPInputStreamTest.java

@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.util.stream.Stream;
+import java.util.zip.GZIPInputStream;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.function.Executable;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/*
+ * @test
+ * @summary basic API verification tests for GZIPInputStream
+ * @run junit BasicGZIPInputStreamTest
+ */
+public class BasicGZIPInputStreamTest {
+
+
+    private static Stream<Arguments> npeFromConstructors() {
+        return Stream.of(Arguments.of((Executable) () -> new GZIPInputStream(null)),
+                Arguments.of((Executable) () -> new GZIPInputStream(null, 1)));
+    }
+
+    /*
+     * Verifies that the GZIPInputStream constructors throw the expected NullPointerException
+     */
+    @ParameterizedTest
+    @MethodSource("npeFromConstructors")
+    public void testNPEFromConstructors(final Executable constructor) {
+        Assertions.assertThrows(NullPointerException.class, constructor,
+                "GZIPInputStream constructor did not throw NullPointerException");
+    }
+
+    private static Stream<Arguments> iaeFromConstructors() {
+        return Stream.of(
+                Arguments.of((Executable) () -> new GZIPInputStream(
+                        new ByteArrayInputStream(new byte[0]), 0)),
+                Arguments.of((Executable) () -> new GZIPInputStream(
+                        new ByteArrayInputStream(new byte[0]), -1)),
+                Arguments.of((Executable) () -> new GZIPInputStream(
+                        new ByteArrayInputStream(new byte[0]), -42)));
+    }
+
+    /*
+     * Verifies that the GZIPInputStream constructors throw the expected IllegalArgumentException
+     */
+    @ParameterizedTest
+    @MethodSource("iaeFromConstructors")
+    public void testIAEFromConstructors(final Executable constructor) {
+        Assertions.assertThrows(IllegalArgumentException.class, constructor,
+                "GZIPInputStream constructor did not throw IllegalArgumentException");
+    }
+
+    private static Stream<Arguments> ioeFromConstructors() {
+        final ByteArrayInputStream notGZIPContent = new ByteArrayInputStream(new byte[0]);
+        return Stream.of(
+                Arguments.of((Executable) () -> new GZIPInputStream(notGZIPContent)),
+                Arguments.of((Executable) () -> new GZIPInputStream(
+                        notGZIPContent, 1024 /* buffer size */)));
+    }
+
+    /*
+     * Verifies that the GZIPInputStream constructors throw the expected IOException
+     */
+    @ParameterizedTest
+    @MethodSource("ioeFromConstructors")
+    public void testIOEFromConstructors(final Executable constructor) {
+        Assertions.assertThrows(IOException.class, constructor,
+                "GZIPInputStream constructor did not throw IOException");
+    }
+}