Issue #730: Allow multiple suppression files in Ant

The core has not been extended but the Ant task is able to parse and pass to the Settings singleton
NOTE: This change is breaking for users of the Ant Task

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

@@ -18,7 +18,9 @@
 package org.owasp.dependencycheck.taskdefs;
 
 import java.io.File;
+import java.util.ArrayList;
 import java.util.List;
+
 import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.Project;
 import org.apache.tools.ant.types.EnumeratedAttribute;
@@ -148,9 +150,10 @@ public class Check extends Update {
      */
     private String reportFormat = "HTML";
     /**
-     * The path to the suppression file.
+     * Suppression file paths.
      */
-    private String suppressionFile;
+    private List<String> suppressionFiles = new ArrayList<>();
+
     /**
      * The path to the suppression file.
      */
@@ -225,6 +228,17 @@ public class Check extends Update {
         getPath().add(rc);
     }
 
+    /**
+     * Add a suppression file.
+     *
+     * This is called by Ant with the configured {@link SuppressionFile}.
+     *
+     * @param suppressionFile the suppression file to add.
+     */
+    public void addConfiguredSuppressionFile(final SuppressionFile suppressionFile) {
+        suppressionFiles.add(suppressionFile.getPath());
+    }
+
     /**
      * Returns the path. If the path has not been initialized yet, this class is
      * synchronized, and will instantiate the path object.
@@ -431,21 +445,23 @@ public class Check extends Update {
     }
 
     /**
-     * Get the value of suppressionFile.
+     * Gets suppression file paths.
      *
-     * @return the value of suppressionFile
+     * @return the suppression files.
      */
-    public String getSuppressionFile() {
-        return suppressionFile;
+    public List<String> getSuppressionFiles() {
+        return suppressionFiles;
     }
 
     /**
      * Set the value of suppressionFile.
      *
      * @param suppressionFile new value of suppressionFile
+     * @deprecated property form of suppressionFile has been replaced by a child element
      */
+    @Deprecated
     public void setSuppressionFile(String suppressionFile) {
-        this.suppressionFile = suppressionFile;
+        throw new BuildException("Property form of suppressionFile has been replaced by a nested element, please update your configuration.");
     }
 
     /**
@@ -992,7 +1008,7 @@ public class Check extends Update {
     protected void populateSettings() throws BuildException {
         super.populateSettings();
         Settings.setBooleanIfNotNull(Settings.KEYS.AUTO_UPDATE, autoUpdate);
-        Settings.setStringIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFile);
+        Settings.setArrayIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFiles.toArray(new String[suppressionFiles.size()]));
         Settings.setStringIfNotEmpty(Settings.KEYS.HINTS_FILE, hintsFile);
         Settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_EXPERIMENTAL_ENABLED, enableExperimental);
         Settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_JAR_ENABLED, jarAnalyzerEnabled);

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

@@ -0,0 +1,49 @@
+/*
+ * This file is part of dependency-check-ant.
+ *
+ * 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.
+ */
+package org.owasp.dependencycheck.taskdefs;
+
+/**
+ * Class          : {@link SuppressionFile}
+ * Responsibility : Models a suppression file nested XML element where the simple content is its location.
+ */
+public class SuppressionFile {
+
+    /**
+     * The path to the suppression file.
+     */
+    private String path;
+
+    /**
+     * Called by ant with the simple content of the suppressionFile xml element.
+     *
+     * @param text the simple content.
+     */
+    public final void addText(String text) {
+        this.path = text;
+    }
+
+    /**
+     * Gets the path to the suppression file.
+     *
+     * @return the path.
+     */
+    public String getPath() {
+        return path;
+    }
+
+}

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

@@ -18,6 +18,7 @@ the project's dependencies.
                       reportoutputdirectory="${basedir}"
                       reportformat="ALL">
 
+        <suppressionfile>path/to/suppression.xml</suppressionfile>
         <fileset dir="lib">
             <include name="**/*.jar"/>
         </fileset>
@@ -38,14 +39,19 @@ failOnError           | Whether the build should fail if there is an error execu
 projectName           | The name of the project being scanned.                                                                                                                                                             | Dependency-Check
 reportFormat          | The report format to be generated (HTML, XML, CSV, JSON, VULN, ALL). This configuration option has no affect if using this within the Site plugin unless the externalReport is set to true.        | HTML
 reportOutputDirectory | The location to write the report(s). Note, this is not used if generating the report as part of a `mvn site` build                                                                                 | 'target'
-suppressionFile       | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html)                                                                                       | &nbsp;
 hintsFile             | The file path to the XML hints file \- used to resolve [false negatives](../general/hints.html)                                                                                                    | &nbsp;
 proxyServer           | The Proxy Server; see the [proxy configuration](../data/proxy.html) page for more information.                                                                                                     | &nbsp;
 proxyPort             | The Proxy Port.                                                                                                                                                                                    | &nbsp;
 proxyUsername         | Defines the proxy user name.                                                                                                                                                                       | &nbsp;
 proxyPassword         | Defines the proxy password.                                                                                                                                                                        | &nbsp;
 connectionTimeout     | The URL Connection Timeout.                                                                                                                                                                        | &nbsp;
-enableExperimental    | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used.                                                                             | false
+enableExperimental    | Enable the [experimental analyzers](../analyzers/index.html). If not enabled the experimental analyzers (see below) will not be loaded or used.                                                    | false
+
+The following nested elements can be set on the dependency-check task.
+
+Property              | Description                                                                                                                                                                                        | Default Value
+----------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------
+suppressionFile       | The file path to the XML suppression file \- used to suppress [false positives](../general/suppression.html). Element can be specified multiple times.                                             | &nbsp;
 
 Analyzer Configuration
 ====================

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

@@ -128,4 +128,19 @@ public class DependencyCheckTaskTest {
 
         // THEN the ant task executed without error
     }
+
+    /**
+     * Test the DependencyCheckTask deprecated suppression property throws an exception with a warning.
+     */
+    @Test
+    public void testDeprecatedSuppressingCVE() {
+        // GIVEN an ant task with a vulnerability using the legacy property
+        final String antTaskName = "deprecated-suppression";
+
+        // 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.");
+        buildFileRule.executeTarget(antTaskName);
+    }
 }

dependency-check-ant/src/test/resources/build.xml

@@ -72,13 +72,23 @@
         </dependency-check>
     </target>
 
-    <target name="suppression">
+    <target name="deprecated-suppression">
         <dependency-check
                 applicationName="test suppression"
                 reportOutputDirectory="${project.build.directory}"
                 autoupdate="false"
                 failBuildOnCVSS="3"
-                suppressionFile="${project.build.directory}/test-classes/test-suppression.xml">
+                suppressionFile="${project.build.directory}/test-classes/test-suppression1.xml"/>
+    </target>
+
+    <target name="suppression">
+        <dependency-check
+                applicationName="test suppression"
+                reportOutputDirectory="${project.build.directory}"
+                autoupdate="false"
+                failBuildOnCVSS="3">
+            <suppressionfile>${project.build.directory}/test-classes/test-suppression1.xml</suppressionfile>
+            <suppressionfile>${project.build.directory}/test-classes/test-suppression2.xml</suppressionfile>
             <fileset dir="${project.build.directory}/test-classes/jars">
                 <include name="axis-1.4.jar"/>
             </fileset>

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

@@ -71,7 +71,7 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer {
         try {
             loadSuppressionData();
         } catch (SuppressionParseException ex) {
-            throw new InitializationException("Error initializing the suppression analyzer", ex);
+            throw new InitializationException("Error initializing the suppression analyzer: " + ex.getLocalizedMessage(), ex);
         }
     }
 
@@ -112,10 +112,14 @@ public abstract class AbstractSuppressionAnalyzer extends AbstractAnalyzer {
         } catch (SAXException ex) {
             throw new SuppressionParseException("Unable to parse the base suppression data file", ex);
         }
-        final String suppressionFilePath = Settings.getString(Settings.KEYS.SUPPRESSION_FILE);
-        if (suppressionFilePath == null) {
+        final String[] suppressionFilePaths = Settings.getArray(Settings.KEYS.SUPPRESSION_FILE);
+        if (suppressionFilePaths == null || suppressionFilePaths.length == 0) {
             return;
         }
+
+        // TODO support more than one file
+        final String suppressionFilePath = suppressionFilePaths[0];
+
         boolean deleteTempFile = false;
         try {
             final Pattern uriRx = Pattern.compile("^(https?|file)\\:.*", Pattern.CASE_INSENSITIVE);

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

@@ -821,6 +821,22 @@ public final class Settings {
         return System.getProperty(key, LOCAL_SETTINGS.get().props.getProperty(key));
     }
 
+    /**
+     * Returns a list with the given key.
+     *
+     * If the propery is not set then {@code null} will be returned.
+     *
+     * @param key the key to get from this {@link Settings} singleton.
+     * @return the list or {@code null} if the key wasn't present.
+     */
+    public static String[] getArray(final String key) {
+        final String string = getString(key);
+        if (string != null) {
+            return string.split(ARRAY_SEP);
+        }
+        return null;
+    }
+
     /**
      * Removes a property from the local properties collection. This is mainly
      * used in test cases.

dependency-check-utils/src/test/java/org/owasp/dependencycheck/utils/SettingsTest.java

@@ -18,6 +18,7 @@
 package org.owasp.dependencycheck.utils;
 
 import static org.hamcrest.core.Is.is;
+import static org.hamcrest.core.IsNull.notNullValue;
 import static org.hamcrest.core.IsNull.nullValue;
 import static org.junit.Assert.assertThat;
 
@@ -244,6 +245,37 @@ public class SettingsTest extends BaseTest {
         Assert.assertTrue(tmp.exists());
     }
 
+    /**
+     * Assert {@link Settings#getArray(String)} from a delimited string returns multiple values in an array.
+     */
+    @Test
+    public void testGetArrayFromADelimitedString() {
+        // GIVEN a delimited string
+        final String delimitedString = "value1,value2";
+        Settings.setString("key", delimitedString);
+
+        // WHEN getting the array
+        final String[] array = Settings.getArray("key");
+
+        // THEN the split array is returned
+        assertThat("Expected the array to be non-null", array, notNullValue());
+        assertThat("Expected the array to have two values", array.length, is(2));
+        assertThat("Expected the first array value to be value1", array[0], is("value1"));
+        assertThat("Expected the second array value to be value2", array[1], is("value2"));
+    }
+
+    /**
+     * Assert {@link Settings#getArray(String)} returns {@code null} if the property is not set.
+     */
+    @Test
+    public void testGetArrayWhereThePropertyIsNotSet() {
+        // WHEN getting the array
+        final String[] array = Settings.getArray("key");
+
+        // THEN null is returned
+        assertThat("Expected the array to be null", array, nullValue());
+    }
+
     /**
      * Assert {@link Settings#setArrayIfNotEmpty(String, String[])} with an empty array is ignored.
      */