Close the stream parse(Path) and parse(URL) open on construction failure

(#630).

Author: garydgregory

src/changes/changes.xml

@@ -72,6 +72,7 @@
       <action type="fix" dev="ggregory" due-to="Gary Gregory, saleem malik">Fill the lookahead buffer on short reads from a chunked source (#625).</action>
       <action type="fix" dev="ggregory" due-to="Gary Gregory, Naveed Khan">Handle null name in CSVRecord accessors under ignoreHeaderCase (#628).</action>
       <action type="fix" dev="ggregory" due-to="Gary Gregory, Naveed Khan">Quote null value that starts a record in minimal quote mode (#629).</action>
+      <action type="fix" dev="ggregory" due-to="Gary Gregory, Naveed Khan">Close the stream parse(Path) and parse(URL) open on construction failure (#630).</action>
       <!-- ADD -->
       <action type="add" dev="ggregory" due-to="Gary Gregory, Indy, Sylvia van Os" issue="CSV-307">Add an "Android Compatibility" section to the web site.</action>
       <action type="add" dev="ggregory" due-to="Ruiqi Dong, Gary Gregory" issue="CSV-325">Add CSVParser.Builder.setByteOffset(long) (#604).</action>

src/main/java/org/apache/commons/csv/CSVParser.java

@@ -50,7 +50,9 @@
 import java.util.stream.StreamSupport;
 
 import org.apache.commons.io.Charsets;
+import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.build.AbstractStreamBuilder;
+import org.apache.commons.io.function.IOSupplier;
 import org.apache.commons.io.function.Uncheck;
 
 /**
@@ -371,6 +373,18 @@ public static CSVParser parse(final InputStream inputStream, final Charset chars
         return parse(new InputStreamReader(inputStream, Charsets.toCharset(charset)), format);
     }
 
+    private static CSVParser parse(final IOSupplier<InputStream> supplier, final Charset charset, final CSVFormat format) throws IOException {
+        Objects.requireNonNull(supplier, "supplier");
+        final InputStream inputStream = supplier.get();
+        try {
+            return parse(inputStream, charset, format);
+        } catch (final IOException | RuntimeException e) {
+            // This method allocated the stream and the caller never gets a parser to close, so close it here.
+            IOUtils.closeQuietlySuppress(inputStream, e);
+            throw e;
+        }
+    }
+
     /**
      * Creates and returns a parser for the given {@link Path}, which the caller MUST close.
      *
@@ -392,7 +406,7 @@ public static CSVParser parse(final InputStream inputStream, final Charset chars
     @SuppressWarnings("resource")
     public static CSVParser parse(final Path path, final Charset charset, final CSVFormat format) throws IOException {
         Objects.requireNonNull(path, "path");
-        return parse(Files.newInputStream(path), charset, format);
+        return parse(() -> Files.newInputStream(path), charset, format);
     }
 
     /**
@@ -461,10 +475,9 @@ public static CSVParser parse(final String string, final CSVFormat format) throw
      * @throws CSVException Thrown on invalid CSV input data.
      * @throws NullPointerException if {@code url} is {@code null}.
      */
-    @SuppressWarnings("resource")
     public static CSVParser parse(final URL url, final Charset charset, final CSVFormat format) throws IOException {
         Objects.requireNonNull(url, "url");
-        return parse(url.openStream(), charset, format);
+        return parse(url::openStream, charset, format);
     }
 
     private String headerComment;

src/test/java/org/apache/commons/csv/CSVParserTest.java

@@ -30,6 +30,7 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
@@ -45,6 +46,8 @@
 import java.io.StringWriter;
 import java.io.UncheckedIOException;
 import java.net.URL;
+import java.net.URLConnection;
+import java.net.URLStreamHandler;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
@@ -308,6 +311,57 @@ void testClose() throws Exception {
         assertThrows(NoSuchElementException.class, records::next);
     }
 
+    @Test
+    void testClosesInputStreamOnParsePathException() throws IOException {
+        final Path path = Files.createTempFile(getClass().getName(), ".csv");
+        try {
+            Files.write(path, "A,,C\n1,2,3\n".getBytes(UTF_8));
+            final CSVFormat format = CSVFormat.DEFAULT.builder().setHeader().get();
+            assertThrows(IllegalArgumentException.class, () -> CSVParser.parse(path, UTF_8, format));
+        } finally {
+            Files.delete(path);
+        }
+    }
+
+    @Test
+    void testClosesInputStreamOnParseUrlException() throws IOException {
+        final AtomicBoolean closed = new AtomicBoolean();
+        final URLStreamHandler handler = new URLStreamHandler() {
+
+            @Override
+            protected URLConnection openConnection(final URL u) {
+                return new URLConnection(u) {
+
+                    @Override
+                    public void connect() {
+                        // noop
+                    }
+
+                    @Override
+                    public InputStream getInputStream() {
+                        return new FilterInputStream(new ByteArrayInputStream("A,,C\n1,2,3\n".getBytes(UTF_8))) {
+
+                            @Override
+                            public void close() throws IOException {
+                                closed.set(true);
+                                super.close();
+                            }
+                        };
+                    }
+                };
+            }
+        };
+        final URL url = new URL("csv", null, -1, "test.csv", handler);
+        final CSVFormat format = CSVFormat.DEFAULT.builder().setHeader().get();
+        assertThrows(IllegalArgumentException.class, () -> {
+            try (CSVParser parser = CSVParser.parse(url, UTF_8, format)) {
+                // we never get here
+                fail("The parser should not be constructed when the header is invalid");
+            }
+        });
+        assertTrue(closed.get(), "The stream opened from the URL must be closed when the parser cannot be constructed");
+    }
+
     @Test
     void testCSV141CSVFormat_DEFAULT() throws Exception {
         testCSV141Failure(CSVFormat.DEFAULT, 3);
@@ -2065,6 +2119,7 @@ void testTrim() throws Exception {
         }
     }
 
+
     @Test
     void testTryWithResourcesParseInputStreamWhenHeaderIsInvalid() throws IOException {
         final AtomicBoolean closed = new AtomicBoolean();