From 903eaed250aded0fb8e4ea1c2161e46a1401bdca Mon Sep 17 00:00:00 2001 From: Hans Joachim Desserud Date: Sat, 12 Sep 2015 14:06:47 +0200 Subject: [PATCH 1/8] Remove unused imports --- .../src/main/java/org/owasp/dependencycheck/App.java | 1 - .../src/main/java/org/owasp/dependencycheck/CliParser.java | 1 - .../analyzer/DependencyBundlingAnalyzerIntegrationTest.java | 6 ------ .../org/owasp/dependencycheck/analyzer/JarAnalyzerTest.java | 2 -- .../dependencycheck/data/nuget/XPathNuspecParserTest.java | 1 - .../data/update/CpeUpdaterIntegrationTest.java | 5 ----- .../java/org/owasp/dependencycheck/xml/pom/ModelTest.java | 5 +---- .../org/owasp/dependencycheck/xml/pom/PomUtilsTest.java | 4 +--- .../java/org/owasp/dependencycheck/utils/ChecksumTest.java | 1 - 9 files changed, 2 insertions(+), 24 deletions(-) diff --git a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/App.java b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/App.java index 520c85009..78cb6fdc7 100644 --- a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/App.java +++ b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/App.java @@ -37,7 +37,6 @@ import org.owasp.dependencycheck.utils.Settings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import ch.qos.logback.core.FileAppender; -import java.util.logging.Level; import org.slf4j.impl.StaticLoggerBinder; /** diff --git a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java index 16dee9b18..aa82a32a4 100644 --- a/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java +++ b/dependency-check-cli/src/main/java/org/owasp/dependencycheck/CliParser.java @@ -19,7 +19,6 @@ package org.owasp.dependencycheck; import java.io.File; import java.io.FileNotFoundException; -import java.util.logging.Level; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerIntegrationTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerIntegrationTest.java index 56caecc24..8ee12448f 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerIntegrationTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyBundlingAnalyzerIntegrationTest.java @@ -17,14 +17,8 @@ */ package org.owasp.dependencycheck.analyzer; -import java.io.File; -import static org.junit.Assert.assertEquals; import org.junit.Test; -import org.owasp.dependencycheck.BaseTest; -import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.data.cpe.AbstractDatabaseTestCase; -import org.owasp.dependencycheck.dependency.Dependency; -import org.owasp.dependencycheck.utils.Settings; /** * diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/JarAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/JarAnalyzerTest.java index 90f345cd7..210542c5e 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/JarAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/JarAnalyzerTest.java @@ -23,8 +23,6 @@ import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.dependency.Evidence; import java.io.File; -import java.util.HashSet; -import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nuget/XPathNuspecParserTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nuget/XPathNuspecParserTest.java index e38b52c6b..e9590aa40 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nuget/XPathNuspecParserTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nuget/XPathNuspecParserTest.java @@ -18,7 +18,6 @@ package org.owasp.dependencycheck.data.nuget; import java.io.ByteArrayOutputStream; -import java.io.File; import java.io.InputStream; import java.io.PrintStream; import static org.junit.Assert.assertEquals; diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/CpeUpdaterIntegrationTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/CpeUpdaterIntegrationTest.java index 7d08204bb..66296edcf 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/CpeUpdaterIntegrationTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/update/CpeUpdaterIntegrationTest.java @@ -15,12 +15,7 @@ */ package org.owasp.dependencycheck.data.update; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; -import static org.junit.Assert.*; import org.owasp.dependencycheck.BaseTest; /** diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/ModelTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/ModelTest.java index d669aae1d..c8127634a 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/ModelTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/ModelTest.java @@ -18,10 +18,7 @@ package org.owasp.dependencycheck.xml.pom; import java.util.ArrayList; import java.util.List; import java.util.Properties; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; + import org.junit.Test; import static org.junit.Assert.*; diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/PomUtilsTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/PomUtilsTest.java index c4ceeff19..dcf187897 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/PomUtilsTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/PomUtilsTest.java @@ -17,7 +17,7 @@ package org.owasp.dependencycheck.xml.pom; import org.owasp.dependencycheck.xml.pom.PomUtils; import java.io.File; -import javax.xml.transform.sax.SAXSource; + import org.junit.After; import org.junit.AfterClass; import org.junit.Before; @@ -25,8 +25,6 @@ import org.junit.BeforeClass; import org.junit.Test; import static org.junit.Assert.*; import org.owasp.dependencycheck.BaseTest; -import org.owasp.dependencycheck.dependency.Dependency; -import org.owasp.dependencycheck.xml.pom.Model; /** * diff --git a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/ChecksumTest.java b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/ChecksumTest.java index a4654d134..0231fdd30 100644 --- a/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/ChecksumTest.java +++ b/dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/ChecksumTest.java @@ -26,7 +26,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.owasp.dependencycheck.utils.Checksum; -import org.owasp.dependencycheck.utils.Checksum; /** * From ca5607d79ee08a984be26212719c59d58153698d Mon Sep 17 00:00:00 2001 From: Hans Joachim Desserud Date: Sat, 12 Sep 2015 14:14:08 +0200 Subject: [PATCH 2/8] Removed empty methods from test --- .../dependencycheck/xml/pom/PomUtilsTest.java | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/PomUtilsTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/PomUtilsTest.java index dcf187897..85b22580a 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/PomUtilsTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/xml/pom/PomUtilsTest.java @@ -15,13 +15,8 @@ */ package org.owasp.dependencycheck.xml.pom; -import org.owasp.dependencycheck.xml.pom.PomUtils; import java.io.File; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; import static org.junit.Assert.*; import org.owasp.dependencycheck.BaseTest; @@ -32,25 +27,6 @@ import org.owasp.dependencycheck.BaseTest; */ public class PomUtilsTest { - public PomUtilsTest() { - } - - @BeforeClass - public static void setUpClass() { - } - - @AfterClass - public static void tearDownClass() { - } - - @Before - public void setUp() { - } - - @After - public void tearDown() { - } - /** * Test of readPom method, of class PomUtils. */ From 69bef59473da09a2620e7b3a7bcfd86b6540e0df Mon Sep 17 00:00:00 2001 From: Hans Joachim Desserud Date: Sat, 12 Sep 2015 14:50:35 +0200 Subject: [PATCH 3/8] Remove superflous semicolon --- .../org/owasp/dependencycheck/utils/DependencyVersionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java index 77e4fbda2..0c8fd18bb 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java @@ -204,7 +204,7 @@ public class DependencyVersionTest { DependencyVersion instance = new DependencyVersion(); List versionParts = Arrays.asList("1", "1", "1"); instance.setVersionParts(versionParts); - List expResult = Arrays.asList("1", "1", "1");; + List expResult = Arrays.asList("1", "1", "1"); List result = instance.getVersionParts(); assertEquals(expResult, result); } From f49cc6fb1f44b76d1c2f1b8809ec4cffef8c54c3 Mon Sep 17 00:00:00 2001 From: Hans Joachim Desserud Date: Sat, 12 Sep 2015 14:51:49 +0200 Subject: [PATCH 4/8] Unused methods in test --- .../utils/DependencyVersionTest.java | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java index 0c8fd18bb..e7fc32f8f 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java @@ -20,13 +20,9 @@ package org.owasp.dependencycheck.utils; import java.util.Arrays; import java.util.Iterator; import java.util.List; -import org.junit.After; -import org.junit.AfterClass; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; /** @@ -35,25 +31,6 @@ import org.junit.Test; */ public class DependencyVersionTest { - public DependencyVersionTest() { - } - - @BeforeClass - public static void setUpClass() { - } - - @AfterClass - public static void tearDownClass() { - } - - @Before - public void setUp() { - } - - @After - public void tearDown() { - } - /** * Test of parseVersion method, of class DependencyVersion. */ From e2fa7c666a2503491728f8a705f17e9e6b39cb74 Mon Sep 17 00:00:00 2001 From: Hans Joachim Desserud Date: Sat, 12 Sep 2015 14:53:01 +0200 Subject: [PATCH 5/8] Unused variable --- .../org/owasp/dependencycheck/utils/DependencyVersionTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java index e7fc32f8f..fc3d2431a 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java @@ -132,7 +132,6 @@ public class DependencyVersionTest { public void testCompareTo() { DependencyVersion instance = new DependencyVersion("1.2.3"); DependencyVersion version = new DependencyVersion("1.2.3"); - int expResult = 0; assertEquals(0, instance.compareTo(version)); version = new DependencyVersion("1.1"); assertEquals(1, instance.compareTo(version)); From c39c3cfdae3b16d44c245659c5be5be0f61e666f Mon Sep 17 00:00:00 2001 From: Hans Joachim Desserud Date: Sat, 12 Sep 2015 15:02:22 +0200 Subject: [PATCH 6/8] Comment for review --- .../org/owasp/dependencycheck/utils/DependencyVersionTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java index fc3d2431a..be39a647a 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java @@ -50,6 +50,7 @@ public class DependencyVersionTest { assertEquals(2, parts.size()); assertEquals("x6", parts.get(0)); assertEquals("0", parts.get(1)); + // TODO(code review): should this be here/do something? //assertEquals("0", parts.get(2)); } From fb85fb5b76c84f5720f4ae1d249184dc74678c66 Mon Sep 17 00:00:00 2001 From: Hans Joachim Desserud Date: Sat, 12 Sep 2015 15:03:41 +0200 Subject: [PATCH 7/8] Ensure that we assert something. If the iterator doesn't have any values we would never enter the loop, but the test would still be green --- .../org/owasp/dependencycheck/utils/DependencyVersionTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java index be39a647a..b303f552b 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/utils/DependencyVersionTest.java @@ -62,6 +62,7 @@ public class DependencyVersionTest { public void testIterator() { DependencyVersion instance = new DependencyVersion("1.2.3"); Iterator result = instance.iterator(); + assertTrue(result.hasNext()); int count = 1; while (result.hasNext()) { String v = (String) result.next(); From 48a6eb1f86d58581c09999a198da64e9b70a1974 Mon Sep 17 00:00:00 2001 From: Hans Joachim Desserud Date: Sat, 12 Sep 2015 15:35:56 +0200 Subject: [PATCH 8/8] Prefer interfaces over concerete classes. Did not change return type for public methods as this might potentially cause problems/need for changes for external users --- .../org/owasp/dependencycheck/Engine.java | 3 +- .../data/central/CentralSearch.java | 2 +- .../owasp/dependencycheck/data/cwe/CweDB.java | 7 +++-- .../data/nvdcve/CveDBIntegrationTest.java | 3 +- .../suppression/SuppressionRuleTest.java | 31 +++---------------- 5 files changed, 13 insertions(+), 33 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java index b6e170a7d..d6bf1d839 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/Engine.java @@ -42,6 +42,7 @@ import java.util.EnumMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -59,7 +60,7 @@ public class Engine implements FileFilter { /** * A Map of analyzers grouped by Analysis phase. */ - private EnumMap> analyzers = new EnumMap>(AnalysisPhase.class); + private Map> analyzers = new EnumMap>(AnalysisPhase.class); /** * A Map of analyzers grouped by Analysis phase. diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java index 53bdd37d5..a5f484c43 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/central/CentralSearch.java @@ -116,7 +116,7 @@ public class CentralSearch { if ("0".equals(numFound)) { missing = true; } else { - final ArrayList result = new ArrayList(); + final List result = new ArrayList(); final NodeList docs = (NodeList) xpath.evaluate("/response/result/doc", doc, XPathConstants.NODESET); for (int i = 0; i < docs.getLength(); i++) { final String g = xpath.evaluate("./str[@name='g']", docs.item(i)); diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/cwe/CweDB.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/cwe/CweDB.java index b17e3ca7d..30c50ab78 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/cwe/CweDB.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/cwe/CweDB.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.ObjectInputStream; import java.util.HashMap; +import java.util.Map; /** * @@ -45,21 +46,21 @@ public final class CweDB { /** * A HashMap of the CWE data. */ - private static final HashMap CWE = loadData(); + private static final Map CWE = loadData(); /** * Loads a HashMap containing the CWE data from a resource found in the jar. * * @return a HashMap of CWE data */ - private static HashMap loadData() { + private static Map loadData() { ObjectInputStream oin = null; try { final String filePath = "data/cwe.hashmap.serialized"; final InputStream input = CweDB.class.getClassLoader().getResourceAsStream(filePath); oin = new ObjectInputStream(input); @SuppressWarnings("unchecked") - final HashMap ret = (HashMap) oin.readObject(); + final Map ret = (HashMap) oin.readObject(); return ret; } catch (ClassNotFoundException ex) { LOGGER.warn("Unable to load CWE data. This should not be an issue."); diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nvdcve/CveDBIntegrationTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nvdcve/CveDBIntegrationTest.java index f0d2d5f93..72c3047c3 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nvdcve/CveDBIntegrationTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nvdcve/CveDBIntegrationTest.java @@ -19,6 +19,7 @@ package org.owasp.dependencycheck.data.nvdcve; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Map.Entry; import java.util.Set; import org.junit.Assert; @@ -121,7 +122,7 @@ public class CveDBIntegrationTest extends BaseDBTestCase { @Test public void testGetMatchingSoftware() throws Exception { CveDB instance = null; - HashMap versions = new HashMap(); + Map versions = new HashMap(); DependencyVersion identifiedVersion = new DependencyVersion("1.0.1o"); versions.put("cpe:/a:openssl:openssl:1.0.1e", Boolean.FALSE); try { diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/suppression/SuppressionRuleTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/suppression/SuppressionRuleTest.java index 4e293d7d6..f6e874304 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/suppression/SuppressionRuleTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/suppression/SuppressionRuleTest.java @@ -20,13 +20,9 @@ package org.owasp.dependencycheck.suppression; import java.io.File; import java.util.ArrayList; import java.util.List; -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 org.junit.Before; -import org.junit.BeforeClass; import org.junit.Test; import org.owasp.dependencycheck.BaseTest; import org.owasp.dependencycheck.dependency.Dependency; @@ -40,25 +36,6 @@ import org.owasp.dependencycheck.dependency.Vulnerability; */ public class SuppressionRuleTest { - public SuppressionRuleTest() { - } - - @BeforeClass - public static void setUpClass() { - } - - @AfterClass - public static void tearDownClass() { - } - - @Before - public void setUp() { - } - - @After - public void tearDown() { - } - // /** * Test of FilePath property, of class SuppressionRule. @@ -91,7 +68,7 @@ public class SuppressionRuleTest { @Test public void testCpe() { SuppressionRule instance = new SuppressionRule(); - ArrayList cpe = new ArrayList(); + List cpe = new ArrayList(); instance.setCpe(cpe); assertFalse(instance.hasCpe()); PropertyType pt = new PropertyType(); @@ -109,7 +86,7 @@ public class SuppressionRuleTest { @Test public void testGetCvssBelow() { SuppressionRule instance = new SuppressionRule(); - ArrayList cvss = new ArrayList(); + List cvss = new ArrayList(); instance.setCvssBelow(cvss); assertFalse(instance.hasCvssBelow()); instance.addCvssBelow(0.7f); @@ -124,7 +101,7 @@ public class SuppressionRuleTest { @Test public void testCwe() { SuppressionRule instance = new SuppressionRule(); - ArrayList cwe = new ArrayList(); + List cwe = new ArrayList(); instance.setCwe(cwe); assertFalse(instance.hasCwe()); instance.addCwe("2"); @@ -139,7 +116,7 @@ public class SuppressionRuleTest { @Test public void testCve() { SuppressionRule instance = new SuppressionRule(); - ArrayList cve = new ArrayList(); + List cve = new ArrayList(); instance.setCve(cve); assertFalse(instance.hasCve()); instance.addCve("CVE-2013-1337");