fix: ensure open handles will not leak on errors

jeremylong · Dec 18, 2023 · c940c49 · c940c49
1 parent 42e7f73
commit c940c49
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 38 deletions.
diff --git a/...rc/main/java/org/owasp/dependencycheck/data/update/nvd/api/CveApiJson20CveItemSource.java b/...rc/main/java/org/owasp/dependencycheck/data/update/nvd/api/CveApiJson20CveItemSource.java
@@ -22,30 +22,23 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
 import io.github.jeremylong.openvulnerability.client.nvd.DefCveItem;
+import org.apache.commons.io.IOUtils;
 
-import java.io.BufferedInputStream;
-import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.nio.file.Files;
-import java.util.zip.GZIPInputStream;
 
 public class CveApiJson20CveItemSource implements CveItemSource<DefCveItem> {
 
-    private final File jsonFile;
     private final ObjectMapper mapper;
     private final InputStream inputStream;
     private final JsonParser jsonParser;
     private DefCveItem currentItem;
     private DefCveItem nextItem;
 
-    public CveApiJson20CveItemSource(File jsonFile) throws IOException {
-        this.jsonFile = jsonFile;
+    public CveApiJson20CveItemSource(InputStream inputStream) throws IOException {
         mapper = new ObjectMapper();
         mapper.registerModule(new JavaTimeModule());
-        inputStream = jsonFile.getName().endsWith(".gz") ?
-                new BufferedInputStream(new GZIPInputStream(Files.newInputStream(jsonFile.toPath()))) :
-                new BufferedInputStream(Files.newInputStream(jsonFile.toPath()));
+        this.inputStream = inputStream;
         jsonParser = mapper.getFactory().createParser(inputStream);
 
         JsonToken token = null;
@@ -62,9 +55,7 @@ public CveApiJson20CveItemSource(File jsonFile) throws IOException {
 
     @Override
     public void close() throws Exception {
-        jsonParser.close();
-        inputStream.close();
-        Files.delete(jsonFile.toPath());
+        IOUtils.closeQuietly(jsonParser, inputStream);
     }
 
     @Override

diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/JsonArrayCveItemSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/JsonArrayCveItemSource.java
@@ -22,30 +22,23 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
 import io.github.jeremylong.openvulnerability.client.nvd.DefCveItem;
+import org.apache.commons.io.IOUtils;
 
-import java.io.BufferedInputStream;
-import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.nio.file.Files;
-import java.util.zip.GZIPInputStream;
 
 public class JsonArrayCveItemSource implements CveItemSource<DefCveItem> {
 
-    private final File jsonFile;
     private final ObjectMapper mapper;
     private final InputStream inputStream;
     private final JsonParser jsonParser;
     private DefCveItem currentItem;
     private DefCveItem nextItem;
 
-    public JsonArrayCveItemSource(File jsonFile) throws IOException {
-        this.jsonFile = jsonFile;
+    public JsonArrayCveItemSource(InputStream inputStream) throws IOException {
         mapper = new ObjectMapper();
         mapper.registerModule(new JavaTimeModule());
-        inputStream = jsonFile.getName().endsWith(".gz") ?
-                new BufferedInputStream(new GZIPInputStream(Files.newInputStream(jsonFile.toPath()))) :
-                new BufferedInputStream(Files.newInputStream(jsonFile.toPath()));
+        this.inputStream = inputStream;
         jsonParser = mapper.getFactory().createParser(inputStream);
 
         if (jsonParser.nextToken() == JsonToken.START_ARRAY) {
@@ -55,9 +48,7 @@ public JsonArrayCveItemSource(File jsonFile) throws IOException {
 
     @Override
     public void close() throws Exception {
-        jsonParser.close();
-        inputStream.close();
-        Files.delete(jsonFile.toPath());
+        IOUtils.closeQuietly(jsonParser, inputStream);
     }
 
     @Override

diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessor.java b/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessor.java
@@ -18,8 +18,15 @@
 package org.owasp.dependencycheck.data.update.nvd.api;
 
 import io.github.jeremylong.openvulnerability.client.nvd.DefCveItem;
+
+import java.io.BufferedInputStream;
 import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
 import java.util.concurrent.Callable;
+import java.util.zip.GZIPInputStream;
+
 import org.owasp.dependencycheck.data.nvd.ecosystem.CveEcosystemMapper;
 import org.owasp.dependencycheck.data.nvdcve.CveDB;
 import org.slf4j.Logger;
@@ -82,16 +89,7 @@ public NvdApiProcessor(final CveDB cveDB, File jsonFile) {
 
     @Override
     public NvdApiProcessor call() throws Exception {
-        CveItemSource<DefCveItem> itemSource = null;
-
-        if (jsonFile.getName().endsWith(".jsonarray.gz")) {
-            itemSource = new JsonArrayCveItemSource(jsonFile);
-        } else if (jsonFile.getName().endsWith(".gz")) {
-            itemSource = new CveApiJson20CveItemSource(jsonFile);
-        } else {
-            itemSource = new JsonArrayCveItemSource(jsonFile);
-        }
-        try {
+        try (CveItemSource<DefCveItem> itemSource = buildItemSource(jsonFile)) {
             while (itemSource.hasNext()) {
                 DefCveItem entry = itemSource.next();
                 try {
@@ -100,13 +98,32 @@ public NvdApiProcessor call() throws Exception {
                     LOGGER.error("Failed to process " + entry.getCve().getId(), ex);
                 }
             }
-        } finally {
-            itemSource.close();
         }
         endTime = System.currentTimeMillis();
         return this;
     }
 
+    static CveItemSource<DefCveItem> buildItemSource(File file) throws IOException {
+        if (file.getName().endsWith(".jsonarray.gz")) {
+            try (InputStream fis = Files.newInputStream(file.toPath());
+                 InputStream zis = new GZIPInputStream(fis);
+                 InputStream is = new BufferedInputStream(zis)) {
+                return new JsonArrayCveItemSource(is);
+            }
+        } else if (file.getName().endsWith(".gz")) {
+            try (InputStream fis = Files.newInputStream(file.toPath());
+                 InputStream zis = new GZIPInputStream(fis);
+                 InputStream is = new BufferedInputStream(zis)) {
+                return new CveApiJson20CveItemSource(is);
+            }
+        } else {
+            try (InputStream fis = Files.newInputStream(file.toPath());
+                 InputStream is = new BufferedInputStream(fis)) {
+                return new JsonArrayCveItemSource(is);
+            }
+        }
+    }
+
     /**
      * Calculates how long the update process took.
      *