Issue #730: Core tests for multiple suppression files

Added updates to Maven plugin documentation
Added upgrade notes to the README

README.md

@@ -136,6 +136,69 @@ docker run --rm \
 ```
 
 
+Upgrade Notes
+-------------
+
+### Upgrading from **1.x.x** to **2.x.x**
+
+Note that when upgrading from version 1.x.x that the following changes will need to be made to your configuration.
+
+#### Suppression file
+
+In order to support multiple suppression files, the mechanism for configuring suppression files has changed.
+As such, users that have defined a suppression file in their configuration will need to update.
+
+See the examples below:
+
+##### Ant
+
+Old:
+
+```xml
+<dependency-check
+  failBuildOnCVSS="3"
+  suppressionFile="suppression.xml">
+</dependency-check>
+```
+
+New:
+
+```xml
+<dependency-check
+  failBuildOnCVSS="3">
+  <suppressionFile>suppression.xml</suppressionFile>
+</dependency-check>
+```
+
+##### Maven
+
+Old:
+
+```xml
+<plugin>
+  <groupId>org.owasp</groupId>
+  <artifactId>dependency-check-maven</artifactId>
+  <configuration>
+    <suppressionFile>suppression.xml</suppressionFile>
+  </configuration>
+</plugin>
+```
+
+New:
+
+```xml
+<plugin>
+  <groupId>org.owasp</groupId>
+  <artifactId>dependency-check-maven</artifactId>
+  <configuration>
+    <suppressionFiles>
+      <suppressionFile>suppression.xml</suppressionFile>
+    </suppressionFiles>
+  </configuration>
+</plugin>
+```
+
+
 Mailing List
 ------------
 

dependency-check-ant/src/main/java/org/owasp/dependencycheck/taskdefs/Check.java

@@ -461,7 +461,7 @@ public class Check extends Update {
      */
     @Deprecated
     public void setSuppressionFile(String suppressionFile) {
-        throw new BuildException("Property form of suppressionFile has been replaced by a nested element, please update your configuration.");
+        throw new BuildException("Definition of a suppression file via a property has been deprecated. Suppression files are now defined as a nested element, please update your configuration.");
     }
 
     /**

dependency-check-ant/src/test/java/org/owasp/dependencycheck/taskdefs/DependencyCheckTaskTest.java

@@ -140,7 +140,7 @@ public class DependencyCheckTaskTest {
         // WHEN executing the ant task
         // THEN an exception with a warning is thrown
         expectedException.expect(BuildException.class);
-        expectedException.expectMessage("Property form of suppressionFile has been replaced by a nested element, please update your configuration.");
+        expectedException.expectMessage("Definition of a suppression file via a property has been deprecated. Suppression files are now defined as a nested element, please update your configuration.");
         buildFileRule.executeTarget(antTaskName);
     }
 }

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

@@ -17,30 +17,34 @@
  */
 package org.owasp.dependencycheck.analyzer;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
+
+import java.util.Set;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.owasp.dependencycheck.BaseTest;
 import org.owasp.dependencycheck.Engine;
 import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
 import org.owasp.dependencycheck.dependency.Dependency;
-import org.owasp.dependencycheck.xml.suppression.SuppressionRule;
-import org.owasp.dependencycheck.utils.Settings;
-import org.slf4j.LoggerFactory;
-
-import java.net.MalformedURLException;
-import java.net.URISyntaxException;
-import java.util.List;
-import java.util.Set;
-
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
 import org.owasp.dependencycheck.exception.InitializationException;
+import org.owasp.dependencycheck.utils.Settings;
+import org.owasp.dependencycheck.utils.Settings.KEYS;
 
 /**
  * @author Jeremy Long
  */
 public class AbstractSuppressionAnalyzerTest extends BaseTest {
 
+    /** A second suppression file to test with. */
+    private static final String OTHER_SUPPRESSIONS_FILE = "other-suppressions.xml";
+
+    /** Suppression file to test with. */
+    private static final String SUPPRESSIONS_FILE = "suppressions.xml";
+
     private AbstractSuppressionAnalyzer instance;
 
     @Before
@@ -64,24 +68,42 @@ public class AbstractSuppressionAnalyzerTest extends BaseTest {
      */
     @Test
     public void testGetRulesFromSuppressionFileFromURL() throws Exception {
-        setSupressionFileFromURL();
+        final String fileUrl = getClass().getClassLoader().getResource(SUPPRESSIONS_FILE).toURI().toURL().toString();
-        instance.initialize();
+        final int numberOfExtraLoadedRules = getNumberOfRulesLoadedFromPath(fileUrl) - getNumberOfRulesLoadedInCoreFile();
-        int expCount = 5;
+        assertEquals("Expected 5 extra rules in the given path", 5, numberOfExtraLoadedRules);
-        List<SuppressionRule> result = instance.getRules();
-        assertTrue(expCount <= result.size());
     }
 
     /**
      * Test of getRules method, of class AbstractSuppressionAnalyzer for
-     * suppression file declared as URL.
+     * suppression file on the classpath.
      */
     @Test
     public void testGetRulesFromSuppressionFileInClasspath() throws Exception {
-        Settings.setString(Settings.KEYS.SUPPRESSION_FILE, "suppressions.xml");
+        final int numberOfExtraLoadedRules = getNumberOfRulesLoadedFromPath(SUPPRESSIONS_FILE) - getNumberOfRulesLoadedInCoreFile();
+        assertEquals("Expected 5 extra rules in the given file", 5, numberOfExtraLoadedRules);
+    }
+
+    /**
+     * Assert that rules are loaded from multiple files if multiple files are denfined in the {@link Settings} singleton.
+     */
+    @Test
+    public void testGetRulesFromMultipleSuppressionFiles() throws Exception {
+        final int rulesInCoreFile = getNumberOfRulesLoadedInCoreFile();
+
+        // GIVEN suppression rules from one file
+        final int rulesInFirstFile = getNumberOfRulesLoadedFromPath(SUPPRESSIONS_FILE) - rulesInCoreFile;
+
+        // AND suppression rules from another file
+        final int rulesInSecondFile = getNumberOfRulesLoadedFromPath(OTHER_SUPPRESSIONS_FILE) - rulesInCoreFile;
+
+        // WHEN initializing with both suppression files
+        final String[] suppressionFiles = { SUPPRESSIONS_FILE, OTHER_SUPPRESSIONS_FILE };
+        Settings.setArrayIfNotEmpty(KEYS.SUPPRESSION_FILE, suppressionFiles);
         instance.initialize();
-        int expCount = 5;
+
-        int currentSize = instance.getRules().size();
+        // THEN rules from both files were loaded
-        assertTrue(expCount <= currentSize);
+        final int expectedSize = rulesInFirstFile + rulesInSecondFile + rulesInCoreFile;
+        assertThat("Expected suppressions from both files", instance.getRules().size(), is(expectedSize));
     }
 
     @Test(expected = InitializationException.class)
@@ -90,15 +112,33 @@ public class AbstractSuppressionAnalyzerTest extends BaseTest {
         instance.initialize();
     }
 
-    private void setSupressionFileFromURL() throws Exception {
+    /**
-        try {
+     * Return the number of rules that are loaded from the core suppression file.
-            final String uri = this.getClass().getClassLoader().getResource("suppressions.xml").toURI().toURL().toString();
+     *
-            Settings.setString(Settings.KEYS.SUPPRESSION_FILE, uri);
+     * @return the number of rules defined in the core suppresion file.
-        } catch (URISyntaxException ex) {
+     * @throws Exception if loading the rules fails.
-            LoggerFactory.getLogger(AbstractSuppressionAnalyzerTest.class).error("", ex);
+     */
-        } catch (MalformedURLException ex) {
+    private int getNumberOfRulesLoadedInCoreFile() throws Exception {
-            LoggerFactory.getLogger(AbstractSuppressionAnalyzerTest.class).error("", ex);
+        Settings.removeProperty(KEYS.SUPPRESSION_FILE);
-        }
+
+        final AbstractSuppressionAnalyzerImpl coreFileAnalyzer = new AbstractSuppressionAnalyzerImpl();
+        coreFileAnalyzer.initialize();
+        return coreFileAnalyzer.getRules().size();
+    }
+
+    /**
+     * Load a file into the {@link AbstractSuppressionAnalyzer} and return the number of rules loaded.
+     *
+     * @param path the path to load.
+     * @return the number of rules that were loaded (including the core rules).
+     * @throws Exception if loading the rules fails.
+     */
+    private int getNumberOfRulesLoadedFromPath(final String path) throws Exception {
+        Settings.setString(KEYS.SUPPRESSION_FILE, path);
+
+        final AbstractSuppressionAnalyzerImpl fileAnalyzer = new AbstractSuppressionAnalyzerImpl();
+        fileAnalyzer.initialize();
+        return fileAnalyzer.getRules().size();
     }
 
     public class AbstractSuppressionAnalyzerImpl extends AbstractSuppressionAnalyzer {

dependency-check-core/src/test/resources/other-suppressions.xml

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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) 2017 The OWASP Foundation. All Rights Reserved.
+-->
+<suppressions xmlns="https://jeremylong.github.io/DependencyCheck/dependency-suppression.1.1.xsd">
+    <suppress>
+        <notes><![CDATA[
+   file name: jackson-dataformat-xml-2.4.5.jar
+   ]]></notes>
+        <gav regex="true">^com\.fasterxml\.jackson.*:.*:.*$</gav>
+        <cpe>cpe:/a:fasterxml:jackson</cpe>
+    </suppress>
+</suppressions>

dependency-check-maven/src/it/730-multiple-suppression-files/pom.xml

@@ -44,8 +44,8 @@ Copyright (c) 2017 The OWASP Foundation. All Rights Reserved.
                 <artifactId>dependency-check-maven</artifactId>
                 <configuration>
                     <suppressionFiles>
-                        <param>${project.basedir}/test-suppression1.xml</param>
+                        <suppressionFile>${project.basedir}/test-suppression1.xml</suppressionFile>
-                        <param>${project.basedir}/test-suppression2.xml</param>
+                        <suppressionFile>${project.basedir}/test-suppression2.xml</suppressionFile>
                     </suppressionFiles>
                 </configuration>
             </plugin>

dependency-check-maven/src/site/markdown/configuration.md

@@ -28,7 +28,7 @@ skipRuntimeScope            | Skip analysis for artifacts with Runtime Scope.
 skipSystemScope             | Skip analysis for artifacts with System Scope.             | false
 skipTestScope               | Skip analysis for artifacts with Test Scope.               | true
 skipArtifactType            | A regular expression used to filter/skip artifact types.   |  
-suppressionFile             | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html). |  
+suppressionFiles            | The file paths to the XML suppression files \- used to suppress [false positives](../general/suppression.html). |  
 hintsFile                   | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html).       |  
 enableExperimental          | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used. | false
 

dependency-check-maven/src/site/markdown/index.md.vm

@@ -204,3 +204,39 @@ Update the local cache of the NVD data from NIST without analyzing the dependenc
     ...
 </project>
 ```
+
+$H$H$H Example 7:
+Suppress false positives using multiple suppression files (E.g. a company-wide suppression file and a local project file).
+
+```xml
+<project>
+    ...
+    <build>
+        ...
+        <plugins>
+            ...
+            <plugin>
+                <groupId>org.owasp</groupId>
+                <artifactId>dependency-check-maven</artifactId>
+                <version>${project.version}</version>
+                <configuration>
+                    <suppressionFiles>
+                        <suppressionFile>http://example.org/suppression.xml</suppressionFile>
+                        <suppressionFile>project-suppression.xml</suppressionFile>
+                    </suppressionFiles>
+                </configuration>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+            ...
+        </plugins>
+        ...
+    </build>
+    ...
+</project>
+```