updated deletion and logging of temporary files to resolve issue #73

Former-commit-id: 7acc91ef84a01b021c5d619602b8a0a7f656947a

dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java

@@ -39,11 +39,11 @@ import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream;
 import org.apache.commons.compress.compressors.CompressorInputStream;
 import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream;
 import org.apache.commons.compress.compressors.gzip.GzipUtils;
-import org.h2.store.fs.FileUtils;
 import org.owasp.dependencycheck.Engine;
 import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
 import org.owasp.dependencycheck.analyzer.exception.ArchiveExtractionException;
 import org.owasp.dependencycheck.dependency.Dependency;
+import org.owasp.dependencycheck.utils.FileUtils;
 import org.owasp.dependencycheck.utils.Settings;
 
 /**
@@ -167,14 +167,18 @@ public class ArchiveAnalyzer extends AbstractAnalyzer implements Analyzer {
     }
 
     /**
-     * The close method does nothing for this Analyzer.
+     * The close method deletes any temporary files and directories created during analysis.
      *
      * @throws Exception thrown if there is an exception deleting temporary files
      */
     @Override
     public void close() throws Exception {
         if (tempFileLocation != null && tempFileLocation.exists()) {
-            FileUtils.deleteRecursive(tempFileLocation.getAbsolutePath(), true);
+            Logger.getLogger(ArchiveAnalyzer.class.getName()).log(Level.FINE, "Attempting to delete temporary files");
+            boolean success = FileUtils.delete(tempFileLocation);
+            if (!success) {
+                Logger.getLogger(ArchiveAnalyzer.class.getName()).log(Level.WARNING, "Failed to delete some temporary files, see the log for more details");
+            }
         }
     }
 

dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/JarAnalyzer.java

@@ -53,7 +53,6 @@ import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
 import javax.xml.transform.sax.SAXSource;
-import org.h2.store.fs.FileUtils;
 import org.jsoup.Jsoup;
 import org.owasp.dependencycheck.Engine;
 import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
@@ -65,6 +64,7 @@ import org.owasp.dependencycheck.jaxb.pom.generated.License;
 import org.owasp.dependencycheck.jaxb.pom.generated.Model;
 import org.owasp.dependencycheck.jaxb.pom.generated.Organization;
 import org.owasp.dependencycheck.jaxb.pom.generated.Parent;
+import org.owasp.dependencycheck.utils.FileUtils;
 import org.owasp.dependencycheck.utils.NonClosingStream;
 import org.owasp.dependencycheck.utils.Settings;
 import org.xml.sax.InputSource;
@@ -937,7 +937,11 @@ public class JarAnalyzer extends AbstractAnalyzer implements Analyzer {
     @Override
     public void close() {
         if (tempFileLocation != null && tempFileLocation.exists()) {
-            FileUtils.deleteRecursive(tempFileLocation.getAbsolutePath(), true);
+            Logger.getLogger(JarAnalyzer.class.getName()).log(Level.FINE, "Attempting to delete temporary files");
+            boolean success = FileUtils.delete(tempFileLocation);
+            if (!success) {
+                Logger.getLogger(JarAnalyzer.class.getName()).log(Level.WARNING, "Failed to delete some temporary files, see the log for more details");
+            }
         }
     }
 

dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java

@@ -69,23 +69,21 @@ public final class FileUtils {
      * Deletes a file. If the File is a directory it will recursively delete the contents.
      *
      * @param file the File to delete
-     * @throws IOException is thrown if the file could not be deleted
+     * @return true if the file was deleted successfully, otherwise false
      */
-    public static void delete(File file) throws IOException {
+    public static boolean delete(File file) {
-        if (file.isDirectory()) {
+        boolean success = true;
-            for (File c : file.listFiles()) {
+        if (file.isDirectory()) { //some of this may duplicative of deleteQuietly....
-                delete(c);
+            for (File f : file.listFiles()) {
+                success &= delete(f);
             }
         }
         if (!org.apache.commons.io.FileUtils.deleteQuietly(file)) {
-            throw new FileNotFoundException("Failed to delete file: " + file);
+            success = false;
+            final String msg = String.format("Failed to delete file: %s", file.getPath());
+            Logger.getLogger(FileUtils.class.getName()).log(Level.FINE, msg);
         }
-        /* else {
+        return success;
-         //delete on exit was a bad idea. if for some reason the file can't be deleted
-         // this will cause a newly constructed file to be deleted and a subsequent run may fail.
-         // still not sure why a file fails to be deleted, but can be overwritten... odd.
-         file.deleteOnExit();
-         }*/
     }
 
     /**

dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/FileUtilsTest.java

@@ -22,6 +22,7 @@ import org.junit.After;
 import org.junit.AfterClass;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -76,7 +77,8 @@ public class FileUtilsTest {
         if (!file.exists()) {
             fail("Unable to create a temporary file.");
         }
-        FileUtils.delete(file);
+        boolean status = FileUtils.delete(file);
+        assertTrue("delete returned a failed status", status);
         assertFalse("Temporary file exists after attempting deletion", file.exists());
     }
 }