Merge pull request #983 from jeremylong/compare-cleanup

CompareTo() cleanup

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 {

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

@@ -309,8 +309,8 @@ public class Engine implements FileFilter, AutoCloseable {
      */
     public synchronized void sortDependencies() {
         //TODO - is this actually necassary????
-        Collections.sort(dependencies);
+//        Collections.sort(dependencies);
-        dependenciesExternalView = null;
+//        dependenciesExternalView = null;
     }
 
     /**

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 {

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

@@ -297,8 +297,7 @@ public class JarAnalyzer extends AbstractFileTypeAnalyzer {
      */
     private boolean hasDependencyWithFilename(final Dependency[] dependencies, final String fileName) {
         for (final Dependency dependency : dependencies) {
-            if (Paths.get(dependency.getActualFilePath()).getFileName().toString().toLowerCase()
+            if (Paths.get(dependency.getActualFilePath()).getFileName().toString().equalsIgnoreCase(fileName)) {
-                    .equals(fileName.toLowerCase())) {
                 return true;
             }
         }

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;
 
@@ -46,7 +45,7 @@ import org.slf4j.LoggerFactory;
  * @author Jeremy Long
  */
 @ThreadSafe
-public class Dependency extends EvidenceCollection implements Serializable, Comparable<Dependency> {
+public class Dependency extends EvidenceCollection implements Serializable {
 
     /**
      * The serial version UID for serialization.
@@ -95,7 +94,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp
     /**
      * A set of vulnerabilities that have been suppressed.
      */
-    private final SortedSet<Vulnerability> suppressedVulnerabilities = new TreeSet<>(new VulnerabilityComparator());
+    private final Set<Vulnerability> 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<Vulnerability> vulnerabilities = new TreeSet<>(new VulnerabilityComparator());
+    private final Set<Vulnerability> vulnerabilities = new HashSet<>();
     /**
      * A collection of related dependencies.
      */
-    private final Set<Dependency> relatedDependencies = new TreeSet<>();
+    private final Set<Dependency> 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<Vulnerability> 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<Vulnerability> getVulnerabilities(boolean sorted) {
+        Set<Vulnerability> 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<Vulnerability> getSuppressedVulnerabilities() {
+    public synchronized Set<Vulnerability> getSuppressedVulnerabilities() {
-        return Collections.unmodifiableSortedSet(new TreeSet<>(suppressedVulnerabilities));
+        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<Vulnerability> getSuppressedVulnerabilities(boolean sorted) {
+        Set<Vulnerability> 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.
+     * Determines the SHA1 and MD5 sum for the given file.
-     *
-     * @return the unmodifiable sorted set of vulnerabilities
-     */
-    public synchronized SortedSet<Vulnerability> getVulnerabilities() {
-        return Collections.unmodifiableSortedSet(new TreeSet<>(vulnerabilities));
-    }
-
-    /**
-     * 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
+     * Adds a related dependency.
-     * {@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.
      *
      * @param dependency a reference to the related dependency
      */
@@ -682,18 +709,6 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp
         return isVirtual;
     }
 
-    /**
-     * Implementation of the Comparable&lt;Dependency&gt; interface. The
-     * comparison is solely based on the file path.
-     *
-     * @param o a dependency to compare
-     * @return an integer representing the natural ordering
-     */
-    @Override
-    public int compareTo(Dependency o) {
-        return this.getFilePath().compareToIgnoreCase(o.getFilePath());
-    }
-
     /**
      * Implementation of the equals method.
      *
@@ -720,6 +735,8 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp
                 .append(this.vulnerabilities, other.vulnerabilities)
                 .append(this.projectReferences, other.projectReferences)
                 .append(this.availableVersions, other.availableVersions)
+                .append(this.version, other.version)
+                .append(this.ecosystem, other.ecosystem)
                 .isEquals();
     }
 
@@ -735,6 +752,7 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp
                 .append(actualFilePath)
                 .append(filePath)
                 .append(fileName)
+                .append(packagePath)
                 .append(md5sum)
                 .append(sha1sum)
                 .append(identifiers)
@@ -743,6 +761,8 @@ public class Dependency extends EvidenceCollection implements Serializable, Comp
                 .append(vulnerabilities)
                 .append(projectReferences)
                 .append(availableVersions)
+                .append(version)
+                .append(ecosystem)
                 .toHashCode();
     }
 

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

@@ -209,7 +209,7 @@ public class Evidence implements Serializable, Comparable<Evidence> {
     @Override
     public int compareTo(Evidence o) {
         if (o == null) {
-            return 1;
+            throw new IllegalArgumentException("Unable to compare null evidence");
         }
         if (StringUtils.equalsIgnoreCase(source, o.source)) {
             if (StringUtils.equalsIgnoreCase(name, o.name)) {

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

@@ -19,6 +19,9 @@ package org.owasp.dependencycheck.dependency;
 
 import java.io.Serializable;
 import javax.annotation.concurrent.ThreadSafe;
+import org.apache.commons.lang3.builder.CompareToBuilder;
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.apache.commons.lang3.builder.HashCodeBuilder;
 
 /**
  * In identifier such as a CPE or dependency coordinates (i.e. GAV).
@@ -42,7 +45,7 @@ public class Identifier implements Serializable, Comparable<Identifier> {
      */
     private String value;
     /**
-     * The url for the identifier.
+     * The URL for the identifier.
      */
     private String url;
     /**
@@ -186,7 +189,7 @@ public class Identifier implements Serializable, Comparable<Identifier> {
      *
      * @param type the identifier type.
      * @param value the identifier value.
-     * @param url the identifier url.
+     * @param url the identifier URL.
      */
     public Identifier(String type, String value, String url) {
         this.type = type;
@@ -199,7 +202,7 @@ public class Identifier implements Serializable, Comparable<Identifier> {
      *
      * @param type the identifier type.
      * @param value the identifier value.
-     * @param url the identifier url.
+     * @param url the identifier URL.
      * @param description the description of the identifier.
      */
     public Identifier(String type, String value, String url, String description) {
@@ -207,27 +210,36 @@ public class Identifier implements Serializable, Comparable<Identifier> {
         this.description = description;
     }
 
+    /**
+     * Basic implementation of equals. This only compares the type and value of
+     * the identifier.
+     * @param obj the identifier to compare
+     * @return true if the objects are equal
+     */
     @Override
     public boolean equals(Object obj) {
-        if (obj == null) {
+        if (obj == null || getClass() != obj.getClass()) {
-            return false;
-        }
-        if (getClass() != obj.getClass()) {
             return false;
         }
         final Identifier other = (Identifier) obj;
-        if ((this.value == null) ? (other.value != null) : !this.value.equals(other.value)) {
+
-            return false;
+        return new EqualsBuilder()
-        }
+                .append(this.type, other.type)
-        return !((this.type == null) ? (other.type != null) : !this.type.equals(other.type));
+                .append(this.value, other.value)
+                .isEquals();
     }
 
+    /**
+     * Basic implementation of hasCode. Note, this only takes into consideration
+     * the type and value of the identifier.
+     * @return the hash code
+     */
     @Override
     public int hashCode() {
-        int hash = 5;
+        return new HashCodeBuilder(5, 49)
-        hash = 53 * hash + (this.value != null ? this.value.hashCode() : 0);
+                .append(type)
-        hash = 53 * hash + (this.type != null ? this.type.hashCode() : 0);
+                .append(value)
-        return hash;
+                .toHashCode();
     }
 
     /**
@@ -241,7 +253,7 @@ public class Identifier implements Serializable, Comparable<Identifier> {
     }
 
     /**
-     * Implementation of the comparator interface. This compares the value of
+     * Implementation of the comparator interface. This compares the type and value of
      * the identifier only.
      *
      * @param o the object being compared
@@ -250,8 +262,11 @@ public class Identifier implements Serializable, Comparable<Identifier> {
     @Override
     public int compareTo(Identifier o) {
         if (o == null) {
-            return -1;
+            throw new IllegalArgumentException("Unable to compare a null identifier");
         }
-        return this.value.compareTo(o.value);
+        return new CompareToBuilder()
+                .append(this.type, o.type)
+                .append(this.value, o.value)
+                .toComparison();
     }
 }

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

@@ -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<Vulnerability>, 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());
-    }
-}

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

dependency-check-core/src/main/resources/templates/htmlReport.vsl

@@ -843,7 +843,7 @@ Getting Help: <a href="https://groups.google.com/forum/#!forum/dependency-check"
                     #set($cnt=$cnt+1)
                     <h4 id="header$cnt" class="subsectionheader expandable collaspablesubsection white">Published Vulnerabilities</h4>
                     <div id="content$cnt" class="subsectioncontent standardsubsection">
-                        #foreach($vuln in $dependency.getVulnerabilities())
+                        #foreach($vuln in $dependency.getVulnerabilities(true))
                             #set($vsctr=$vsctr+1)
                             #if($vuln.getSource().name().equals("NVD"))
                                 <p><b><a target="_blank" href="http://web.nvd.nist.gov/view/vuln/detail?vulnId=$enc.url($vuln.name)">$enc.html($vuln.name)</a></b>&nbsp;&nbsp;<button class="copybutton" title="Generate Suppression XML for this CCE for this file" onclick="copyText('$enc.javascript($dependency.DisplayFileName)', '$enc.javascript($dependency.Sha1sum)', '$enc.javascript($suppressGav)', 'cve', '$enc.javascript($vuln.name)')">suppress</button></p>
@@ -1015,7 +1015,7 @@ Getting Help: <a href="https://groups.google.com/forum/#!forum/dependency-check"
                             #set($cnt=$cnt+1)
                             <h4 id="header$cnt" class="subsectionheader expandable collaspablesubsection white">Suppressed Vulnerabilities</h4>
                             <div id="content$cnt" class="subsectioncontent standardsubsection">
-                                #foreach($vuln in $dependency.getSuppressedVulnerabilities())
+                                #foreach($vuln in $dependency.getSuppressedVulnerabilities(true))
                                     #set($vsctr=$vsctr+1)
                                     #if($vuln.getSource().name().equals("NVD"))
                                         <p><b><a target="_blank" href="http://web.nvd.nist.gov/view/vuln/detail?vulnId=$enc.url($vuln.name)">$enc.html($vuln.name)</a></b>&nbsp;&nbsp;<span class="suppressedLabel" >suppressed</span></p>

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",

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
                     <tbody>
                 #foreach($dependency in $dependencies)
                     #if($dependency.getVulnerabilities().size()>0)
-                        #foreach($vuln in $dependency.getVulnerabilities())
+                        #foreach($vuln in $dependency.getVulnerabilities(true))
                             <tr>
                                 <td>
                                     #if($vuln.getSource().name().equals("NVD"))

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)
             <vulnerabilities>
-#foreach($vuln in $dependency.getVulnerabilities())
+#foreach($vuln in $dependency.getVulnerabilities(true))
                 <vulnerability source="$enc.xml($vuln.getSource().name())">
                     <name>$enc.xml($vuln.name)</name>
                     <cvssScore>$vuln.cvssScore</cvssScore>
@@ -180,7 +180,7 @@ Copyright (c) 2012 Jeremy Long. All Rights Reserved.
                     </vulnerableSoftware>
                 </vulnerability>
 #end
-#foreach($vuln in $dependency.getSuppressedVulnerabilities())
+#foreach($vuln in $dependency.getSuppressedVulnerabilities(true))
                 <suppressedVulnerability source="$enc.xml($vuln.getSource().name())">
                     <name>$enc.xml($vuln.name)</name>
                     <cvssScore>$vuln.cvssScore</cvssScore>

dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerIT.java

@@ -157,7 +157,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 = dependency.getVulnerabilities(true).iterator().next();
             assertEquals(vulnerability.getCvssScore(), 5.0f, 0.0);
 
         } catch (InitializationException | DatabaseException | AnalysisException | UpdateException e) {

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

@@ -194,7 +194,7 @@ public final class Checksum {
             return MessageDigest.getInstance(algorithm);
         } catch (NoSuchAlgorithmException e) {
             LOGGER.error(e.getMessage());
-            final String msg = String.format("Failed to obtain the {} message digest.", algorithm);
+            final String msg = String.format("Failed to obtain the %s message digest.", algorithm);
             throw new IllegalStateException(msg, e);
         }
     }

pom.xml

@@ -132,6 +132,7 @@ Copyright (c) 2012 - Jeremy Long
         <maven.api.version>3.0</maven.api.version>
         <reporting.checkstyle-plugin.version>2.17</reporting.checkstyle-plugin.version>
         <reporting.pmd-plugin.version>3.6</reporting.pmd-plugin.version>
+        <surefireArgLine/>
     </properties>
     <distributionManagement>
         <snapshotRepository>