From e868ce83282409c73048cf1f0c4208ab857d3f11 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Tue, 6 Sep 2016 06:23:55 -0400 Subject: [PATCH] cleaned up file deletion code slightly --- .../analyzer/AssemblyAnalyzer.java | 4 +- .../analyzer/CentralAnalyzer.java | 3 +- .../analyzer/NexusAnalyzer.java | 3 +- .../data/update/CpeUpdater.java | 6 ++- .../data/update/nvd/DownloadTask.java | 38 ++++++++----------- 5 files changed, 25 insertions(+), 29 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java index 3d4537e04..9501da8e6 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java @@ -209,8 +209,6 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { IOUtils.copy(is, fos); grokAssemblyExe = tempFile; - // Set the temp file to get deleted when we're done - grokAssemblyExe.deleteOnExit(); LOGGER.debug("Extracted GrokAssembly.exe to {}", grokAssemblyExe.getPath()); } catch (IOException ioe) { this.setEnabled(false); @@ -295,10 +293,12 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { super.close(); try { if (grokAssemblyExe != null && !grokAssemblyExe.delete()) { + LOGGER.debug("Unable to delete temporary GrokAssembly.exe; attempting delete on exit"); grokAssemblyExe.deleteOnExit(); } } catch (SecurityException se) { LOGGER.debug("Can't delete temporary GrokAssembly.exe"); + grokAssemblyExe.deleteOnExit(); } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CentralAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CentralAnalyzer.java index 3d96c3dc9..38f7c9c2b 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CentralAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/CentralAnalyzer.java @@ -229,7 +229,8 @@ public class CentralAnalyzer extends AbstractFileTypeAnalyzer { LOGGER.warn("Unable to download pom.xml for {} from Central; " + "this could result in undetected CPE/CVEs.", dependency.getFileName()); } finally { - if (pomFile != null && !FileUtils.deleteQuietly(pomFile)) { + if (pomFile != null && pomFile.exists() && !FileUtils.deleteQuietly(pomFile)) { + LOGGER.debug("Failed to delete temporary pom file {}", pomFile.toString()); pomFile.deleteOnExit(); } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NexusAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NexusAnalyzer.java index 1ff31ecaa..4a89f7278 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NexusAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/NexusAnalyzer.java @@ -245,7 +245,8 @@ public class NexusAnalyzer extends AbstractFileTypeAnalyzer { LOGGER.warn("Unable to download pom.xml for {} from Nexus repository; " + "this could result in undetected CPE/CVEs.", dependency.getFileName()); } finally { - if (pomFile != null && !FileUtils.deleteQuietly(pomFile)) { + if (pomFile != null && pomFile.exists() && !FileUtils.deleteQuietly(pomFile)) { + LOGGER.debug("Failed to delete temporary pom file {}", pomFile.toString()); pomFile.deleteOnExit(); } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/CpeUpdater.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/CpeUpdater.java index 0c158a92d..778ae124a 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/CpeUpdater.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/CpeUpdater.java @@ -158,6 +158,7 @@ public class CpeUpdater extends BaseUpdater implements CachedWebDataSource { final String originalPath = file.getPath(); final File gzip = new File(originalPath + ".gz"); if (gzip.isFile() && !gzip.delete()) { + LOGGER.debug("Failed to delete intial temporary file {}", gzip.toString()); gzip.deleteOnExit(); } if (!file.renameTo(gzip)) { @@ -192,8 +193,9 @@ public class CpeUpdater extends BaseUpdater implements CachedWebDataSource { LOGGER.trace("ignore", ex); } } - if (gzip.isFile()) { - FileUtils.deleteQuietly(gzip); + if (gzip.isFile() && !FileUtils.deleteQuietly(gzip)) { + LOGGER.debug("Failed to delete temporary file {}", gzip.toString()); + gzip.deleteOnExit(); } } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/nvd/DownloadTask.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/nvd/DownloadTask.java index 020c2263c..f6d29e811 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/nvd/DownloadTask.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/update/nvd/DownloadTask.java @@ -55,8 +55,9 @@ public class DownloadTask implements Callable> { * @param nvdCveInfo the NVD CVE info * @param processor the processor service to submit the downloaded files to * @param cveDB the CVE DB to use to store the vulnerability data - * @param settings a reference to the global settings object; this is necessary so that when the thread is started the - * dependencies have a correct reference to the global settings. + * @param settings a reference to the global settings object; this is + * necessary so that when the thread is started the dependencies have a + * correct reference to the global settings. * @throws UpdateException thrown if temporary files could not be created */ public DownloadTask(NvdCveInfo nvdCveInfo, ExecutorService processor, CveDB cveDB, Settings settings) throws UpdateException { @@ -205,25 +206,13 @@ public class DownloadTask implements Callable> { * Attempts to delete the files that were downloaded. */ public void cleanup() { - boolean deleted = false; - try { - if (first != null && first.exists()) { - deleted = first.delete(); - } - } finally { - if (first != null && (first.exists() || !deleted)) { - first.deleteOnExit(); - } + if (first != null && first.exists() && first.delete()) { + LOGGER.debug("Failed to delete first temporary file {}", second.toString()); + first.deleteOnExit(); } - try { - deleted = false; - if (second != null && second.exists()) { - deleted = second.delete(); - } - } finally { - if (second != null && (second.exists() || !deleted)) { - second.deleteOnExit(); - } + if (second != null && second.exists() && !second.delete()) { + LOGGER.debug("Failed to delete second temporary file {}", second.toString()); + second.deleteOnExit(); } } @@ -268,7 +257,8 @@ public class DownloadTask implements Callable> { } /** - * Extracts the file contained in a gzip archive. The extracted file is placed in the exact same path as the file specified. + * Extracts the file contained in a gzip archive. The extracted file is + * placed in the exact same path as the file specified. * * @param file the archive file * @throws FileNotFoundException thrown if the file does not exist @@ -278,6 +268,7 @@ public class DownloadTask implements Callable> { final String originalPath = file.getPath(); final File gzip = new File(originalPath + ".gz"); if (gzip.isFile() && !gzip.delete()) { + LOGGER.debug("Failed to delete initial temporary file when extracting 'gz' {}", gzip.toString()); gzip.deleteOnExit(); } if (!file.renameTo(gzip)) { @@ -312,8 +303,9 @@ public class DownloadTask implements Callable> { LOGGER.trace("ignore", ex); } } - if (gzip.isFile()) { - FileUtils.deleteQuietly(gzip); + if (gzip.isFile() && !FileUtils.deleteQuietly(gzip)) { + LOGGER.debug("Failed to delete temporary file when extracting 'gz' {}", gzip.toString()); + gzip.deleteOnExit(); } } }