From 80ca3e114eb35e4e663cc9de048b9d90ff6bbf18 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Fri, 28 Feb 2014 06:52:51 -0500 Subject: [PATCH] updated deletion and logging of temporary files to resolve issue #73 Former-commit-id: 7acc91ef84a01b021c5d619602b8a0a7f656947a --- .../analyzer/ArchiveAnalyzer.java | 10 ++++++--- .../dependencycheck/analyzer/JarAnalyzer.java | 8 +++++-- .../dependencycheck/utils/FileUtils.java | 22 +++++++++---------- .../dependencycheck/utils/FileUtilsTest.java | 4 +++- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java index c9e906f23..14c8a9055 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzer.java +++ b/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"); + } } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/JarAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/JarAnalyzer.java index bdfc22cb9..f9ea07435 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/JarAnalyzer.java +++ b/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"); + } } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java index 5ea80f944..bcbb59dd6 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java +++ b/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 { - if (file.isDirectory()) { - for (File c : file.listFiles()) { - delete(c); + public static boolean delete(File file) { + boolean success = true; + if (file.isDirectory()) { //some of this may duplicative of deleteQuietly.... + 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 { - //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(); - }*/ + return success; } /** diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/FileUtilsTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/FileUtilsTest.java index bf23a3af3..65ac3fb8f 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/FileUtilsTest.java +++ b/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()); } }