From 8428e96702489f597cf54570cb277cdc58a892c9 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sun, 12 Nov 2017 07:03:35 -0500 Subject: [PATCH] removed TreeSet to improve performance --- .../owasp/dependencycheck/taskdefs/Check.java | 2 +- .../agent/DependencyCheckScanAgent.java | 2 +- .../dependency/Dependency.java | 75 +++++++++++++------ .../dependency/VulnerabilityComparator.java | 48 ------------ .../main/resources/templates/csvReport.vsl | 2 +- .../main/resources/templates/htmlReport.vsl | 4 +- .../main/resources/templates/jsonReport.vsl | 4 +- .../main/resources/templates/vulnReport.vsl | 2 +- .../main/resources/templates/xmlReport.vsl | 4 +- .../analyzer/RubyBundleAuditAnalyzerIT.java | 3 +- 10 files changed, 63 insertions(+), 83 deletions(-) delete mode 100644 dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/VulnerabilityComparator.java diff --git a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java index 7c6bfc5a6..ec88800b6 100644 --- a/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java +++ b/dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java @@ -1128,7 +1128,7 @@ public class Check extends Update { for (Dependency d : dependencies) { boolean firstEntry = true; final StringBuilder ids = new StringBuilder(); - for (Vulnerability v : d.getVulnerabilities()) { + for (Vulnerability v : d.getVulnerabilities(true)) { if (firstEntry) { firstEntry = false; } else { diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java index fcbe5e3ce..a0e9e6fe3 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java @@ -1053,7 +1053,7 @@ public class DependencyCheckScanAgent { for (Dependency d : dependencies) { boolean firstEntry = true; final StringBuilder ids = new StringBuilder(); - for (Vulnerability v : d.getVulnerabilities()) { + for (Vulnerability v : d.getVulnerabilities(true)) { if (firstEntry) { firstEntry = false; } else { diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java index ee5bb1037..d21734def 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/Dependency.java @@ -26,7 +26,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.SortedSet; import java.util.TreeSet; import javax.annotation.concurrent.ThreadSafe; @@ -83,7 +82,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp /** * A list of Identifiers. */ - private final Set identifiers = new TreeSet<>(); + private final Set identifiers = new HashSet<>(); /** * The file name to display in reports. */ @@ -91,11 +90,11 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp /** * A set of identifiers that have been suppressed. */ - private final Set suppressedIdentifiers = new TreeSet<>(); + private final Set suppressedIdentifiers = new HashSet<>(); /** * A set of vulnerabilities that have been suppressed. */ - private final SortedSet suppressedVulnerabilities = new TreeSet<>(new VulnerabilityComparator()); + private final Set suppressedVulnerabilities = new HashSet<>(); /** * The description of the JAR file. */ @@ -107,11 +106,11 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp /** * A list of vulnerabilities for this dependency. */ - private final SortedSet vulnerabilities = new TreeSet<>(new VulnerabilityComparator()); + private final Set vulnerabilities = new HashSet<>(); /** * A collection of related dependencies. */ - private final Set relatedDependencies = new TreeSet<>(); + private final Set relatedDependencies = new HashSet<>(); /** * A list of projects that reference this dependency. */ @@ -457,12 +456,53 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } /** - * Get an unmodifiable sorted set of suppressedVulnerabilities. + * Get the unmodifiable sorted set of vulnerabilities. + * + * @return the unmodifiable sorted set of vulnerabilities + */ + public synchronized Set getVulnerabilities() { + return getVulnerabilities(false); + } + + /** + * Get the unmodifiable list of vulnerabilities; optionally sorted. + * + * @param sorted if true the list will be sorted + * @return the unmodifiable list set of vulnerabilities + */ + public synchronized Set getVulnerabilities(boolean sorted) { + Set r; + if (sorted) { + r = new TreeSet<>(vulnerabilities); + } else { + r = vulnerabilities; + } + return Collections.unmodifiableSet(r); + } + + /** + * Get an unmodifiable set of suppressedVulnerabilities. * * @return the unmodifiable sorted set of suppressedVulnerabilities */ - public synchronized SortedSet getSuppressedVulnerabilities() { - return Collections.unmodifiableSortedSet(new TreeSet<>(suppressedVulnerabilities)); + public synchronized Set getSuppressedVulnerabilities() { + return getSuppressedVulnerabilities(false); + } + + /** + * Get an unmodifiable, optionally sorted. set of suppressedVulnerabilities. + * + * @param sorted whether or not the set is sorted + * @return the unmodifiable sorted set of suppressedVulnerabilities + */ + public synchronized Set getSuppressedVulnerabilities(boolean sorted) { + Set r; + if (sorted) { + r = new TreeSet<>(suppressedVulnerabilities); + } else { + r = suppressedVulnerabilities; + } + return Collections.unmodifiableSet(r); } /** @@ -525,16 +565,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } /** - * Get the unmodifiable sorted set of vulnerabilities. - * - * @return the unmodifiable sorted set of vulnerabilities - */ - public synchronized SortedSet getVulnerabilities() { - return Collections.unmodifiableSortedSet(new TreeSet<>(vulnerabilities)); - } - - /** - * Determines the sha1 and md5 sum for the given file. + * Determines the SHA1 and MD5 sum for the given file. * * @param file the file to create checksums for */ @@ -624,11 +655,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp } /** - * Adds a related dependency. The internal collection is normally a - * {@link java.util.TreeSet}, which relies on - * {@link #compareTo(Dependency)}. A consequence of this is that if you - * attempt to add a dependency with the same file path (modulo character - * case) as one that is already in the collection, it won't get added. + * Adds a related dependency. * * @param dependency a reference to the related dependency */ diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/VulnerabilityComparator.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/VulnerabilityComparator.java deleted file mode 100644 index 5432559a0..000000000 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/dependency/VulnerabilityComparator.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * This file is part of dependency-check-core. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * Copyright (c) 2012 Jeremy Long. All Rights Reserved. - */ -package org.owasp.dependencycheck.dependency; - -import java.io.Serializable; -import java.util.Comparator; -import javax.annotation.concurrent.ThreadSafe; - -/** - * Comparator for Vulnerability objects. - * - * @author Jeremy Long - */ -@ThreadSafe -public class VulnerabilityComparator implements Comparator, Serializable { - - /** - * The serial version UID. - */ - private static final long serialVersionUID = 1L; - - /** - * Implements the comparison of vulnerabilities. - * - * @param o1 a vulnerability - * @param o2 a second vulnerability - * @return the comparison - */ - @Override - public int compare(Vulnerability o1, Vulnerability o2) { - return o2.getName().compareTo(o1.getName()); - } -} diff --git a/dependency-check-core/src/main/resources/templates/csvReport.vsl b/dependency-check-core/src/main/resources/templates/csvReport.vsl index 536b4b370..816384cee 100644 --- a/dependency-check-core/src/main/resources/templates/csvReport.vsl +++ b/dependency-check-core/src/main/resources/templates/csvReport.vsl @@ -20,7 +20,7 @@ Copyright (c) 2017 Jeremy Long. All Rights Reserved. "Project","ScanDate","DependencyName","DependencyPath","Description","License","Md5","Sha1","Identifiers","CPE","CVE","CWE","Vulnerability","Source","Severity","CVSSv2","GAV","CPE Confidence","Evidence Count" #macro(writeSev $score)#if($score<4.0)"Low"#elseif($score>=7.0)"High"#else"Medium"#end#end #foreach($dependency in $dependencies)#if($dependency.getVulnerabilities().size()>0) -#foreach($vuln in $dependency.getVulnerabilities()) +#foreach($vuln in $dependency.getVulnerabilities(true)) $enc.csv($applicationName),$enc.csv($scanDate),$enc.csv($dependency.DisplayFileName),#if($dependency.FilePath)$enc.csv($dependency.FilePath)#end,#if($dependency.description)$enc.csv($dependency.description)#end,#if($dependency.license)$enc.csv($dependency.license)#end,#if($dependency.Md5sum)$enc.csv($dependency.Md5sum)#end,#if($dependency.Sha1sum)$enc.csv($dependency.Sha1sum)#end,#if($dependency.identifiers)$enc.csvIdentifiers($dependency.identifiers)#end,#if($dependency.identifiers)$enc.csvCpe($dependency.identifiers)#end,#if($vuln.name)$enc.csv($vuln.name)#end,#if($dependency.cwe)$enc.csv($vuln.cwe)#end,#if($vuln.description)$enc.csv($vuln.description)#end,#if($vuln.getSource().name())$enc.csv($vuln.getSource().name())#end,#writeSev($vuln.cvssScore),$vuln.cvssScore,#if($dependency.identifiers)$enc.csvGav($dependency.identifiers)#end,#if($dependency.identifiers)$enc.csvCpeConfidence($dependency.identifiers)#end,$dependency.size() #end #end diff --git a/dependency-check-core/src/main/resources/templates/htmlReport.vsl b/dependency-check-core/src/main/resources/templates/htmlReport.vsl index ceb7922d9..08170c044 100644 --- a/dependency-check-core/src/main/resources/templates/htmlReport.vsl +++ b/dependency-check-core/src/main/resources/templates/htmlReport.vsl @@ -843,7 +843,7 @@ Getting Help:

$enc.html($vuln.name)  

@@ -1015,7 +1015,7 @@ Getting Help:

$enc.html($vuln.name)  suppressed

diff --git a/dependency-check-core/src/main/resources/templates/jsonReport.vsl b/dependency-check-core/src/main/resources/templates/jsonReport.vsl index e0e72d76d..decfeaa83 100644 --- a/dependency-check-core/src/main/resources/templates/jsonReport.vsl +++ b/dependency-check-core/src/main/resources/templates/jsonReport.vsl @@ -132,7 +132,7 @@ #if($dependency.getVulnerabilities().size()>0) ,"vulnerabilities": [ - #foreach($vuln in $dependency.getVulnerabilities())#if($foreach.count > 1),#end { + #foreach($vuln in $dependency.getVulnerabilities(true))#if($foreach.count > 1),#end { "source": "$enc.json($vuln.getSource().name())", "name": "$enc.json($vuln.name)", "cvssScore": "$vuln.cvssScore", @@ -170,7 +170,7 @@ #if($dependency.getSuppressedVulnerabilities().size()>0 || $dependency.getSuppressedVulnerabilities().size()>0) ,"suppressedVulnerabilities": [ - #foreach($vuln in $dependency.getSuppressedVulnerabilities())#if($foreach.count > 1),#end { + #foreach($vuln in $dependency.getSuppressedVulnerabilities(true))#if($foreach.count > 1),#end { "source": "$enc.json($vuln.getSource().name())", "name": "$enc.json($vuln.name)", "cvssScore": "$vuln.cvssScore", diff --git a/dependency-check-core/src/main/resources/templates/vulnReport.vsl b/dependency-check-core/src/main/resources/templates/vulnReport.vsl index f3b23e41c..de84dec1b 100644 --- a/dependency-check-core/src/main/resources/templates/vulnReport.vsl +++ b/dependency-check-core/src/main/resources/templates/vulnReport.vsl @@ -208,7 +208,7 @@ have been reported. Additionally, the HTML report provides many features not fou #foreach($dependency in $dependencies) #if($dependency.getVulnerabilities().size()>0) - #foreach($vuln in $dependency.getVulnerabilities()) + #foreach($vuln in $dependency.getVulnerabilities(true)) #if($vuln.getSource().name().equals("NVD")) diff --git a/dependency-check-core/src/main/resources/templates/xmlReport.vsl b/dependency-check-core/src/main/resources/templates/xmlReport.vsl index e7068b110..86581ff37 100644 --- a/dependency-check-core/src/main/resources/templates/xmlReport.vsl +++ b/dependency-check-core/src/main/resources/templates/xmlReport.vsl @@ -140,7 +140,7 @@ Copyright (c) 2012 Jeremy Long. All Rights Reserved. #end #if($dependency.getVulnerabilities().size()>0 || $dependency.getSuppressedVulnerabilities().size()>0) -#foreach($vuln in $dependency.getVulnerabilities()) +#foreach($vuln in $dependency.getVulnerabilities(true)) $enc.xml($vuln.name) $vuln.cvssScore @@ -180,7 +180,7 @@ Copyright (c) 2012 Jeremy Long. All Rights Reserved. #end -#foreach($vuln in $dependency.getSuppressedVulnerabilities()) +#foreach($vuln in $dependency.getSuppressedVulnerabilities(true)) $enc.xml($vuln.name) $vuln.cvssScore diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerIT.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerIT.java index 1f44f88f7..73d79c84c 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerIT.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerIT.java @@ -22,6 +22,7 @@ import edu.emory.mathcs.backport.java.util.Arrays; import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.SortedSet; import org.junit.After; import org.junit.Assume; @@ -157,7 +158,7 @@ public class RubyBundleAuditAnalyzerIT extends BaseDBTestCase { "ruby/vulnerable/gems/sinatra/Gemfile.lock")); analyzer.analyze(result, engine); Dependency dependency = engine.getDependencies()[0]; - Vulnerability vulnerability = dependency.getVulnerabilities().first(); + Vulnerability vulnerability = ((SortedSet)dependency.getVulnerabilities(true)).first(); assertEquals(vulnerability.getCvssScore(), 5.0f, 0.0); } catch (InitializationException | DatabaseException | AnalysisException | UpdateException e) {