Add a configurable read timeout to downloads to resolve hang on stall…

…ed download issue #4413
jeremylong · Apr 27, 2022 · 041d9b0 · 041d9b0
1 parent baa22c3
commit 041d9b0
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 0 deletions.
diff --git a/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Update.java b/ant/src/main/java/org/owasp/dependencycheck/taskdefs/Update.java
@@ -64,6 +64,10 @@ public class Update extends Purge {
      * The Connection Timeout.
      */
     private String connectionTimeout;
+    /**
+     * The Read Timeout.
+     */
+    private String readTimeout;
     /**
      * The database driver name; such as org.h2.Driver.
      */
@@ -223,6 +227,24 @@ public void setConnectionTimeout(String connectionTimeout) {
         this.connectionTimeout = connectionTimeout;
     }
 
+    /**
+     * Get the value of readTimeout.
+     *
+     * @return the value of readTimeout
+     */
+    public String getReadTimeout() {
+        return readTimeout;
+    }
+
+    /**
+     * Set the value of readTimeout.
+     *
+     * @param readTimeout new value of readTimeout
+     */
+    public void setReadTimeout(String readTimeout) {
+        this.readTimeout = readTimeout;
+    }
+
     /**
      * Get the value of databaseDriverName.
      *
@@ -456,6 +478,7 @@ protected void populateSettings() throws BuildException {
         getSettings().setStringIfNotEmpty(Settings.KEYS.PROXY_PASSWORD, proxyPassword);
         getSettings().setStringIfNotEmpty(Settings.KEYS.PROXY_NON_PROXY_HOSTS, nonProxyHosts);
         getSettings().setStringIfNotEmpty(Settings.KEYS.CONNECTION_TIMEOUT, connectionTimeout);
+        getSettings().setStringIfNotEmpty(Settings.KEYS.CONNECTION_READ_TIMEOUT, readTimeout);
         getSettings().setStringIfNotEmpty(Settings.KEYS.DB_DRIVER_NAME, databaseDriverName);
         getSettings().setStringIfNotEmpty(Settings.KEYS.DB_DRIVER_PATH, databaseDriverPath);
         getSettings().setStringIfNotEmpty(Settings.KEYS.DB_CONNECTION_STRING, connectionString);

diff --git a/cli/src/main/java/org/owasp/dependencycheck/App.java b/cli/src/main/java/org/owasp/dependencycheck/App.java
@@ -439,6 +439,8 @@ protected void populateSettings(CliParser cli) throws InvalidSettingException {
                 cli.getStringArgument(CliParser.ARGUMENT.NON_PROXY_HOSTS));
         settings.setStringIfNotEmpty(Settings.KEYS.CONNECTION_TIMEOUT,
                 cli.getStringArgument(CliParser.ARGUMENT.CONNECTION_TIMEOUT));
+        settings.setStringIfNotEmpty(Settings.KEYS.CONNECTION_READ_TIMEOUT,
+                cli.getStringArgument(CliParser.ARGUMENT.CONNECTION_READ_TIMEOUT));
         settings.setStringIfNotEmpty(Settings.KEYS.HINTS_FILE,
                 cli.getStringArgument(CliParser.ARGUMENT.HINTS_FILE));
         settings.setIntIfNotNull(Settings.KEYS.CVE_CHECK_VALID_FOR_HOURS,

diff --git a/cli/src/main/java/org/owasp/dependencycheck/CliParser.java b/cli/src/main/java/org/owasp/dependencycheck/CliParser.java
@@ -338,6 +338,8 @@ private void addAdvancedOptions(final Options options) {
                         + "Use pipe, comma or colon as list separator."))
                 .addOption(newOptionWithArg(ARGUMENT.CONNECTION_TIMEOUT_SHORT, ARGUMENT.CONNECTION_TIMEOUT, "timeout",
                         "The connection timeout (in milliseconds) to use when downloading resources."))
+                .addOption(newOptionWithArg(ARGUMENT.CONNECTION_READ_TIMEOUT, "timeout",
+                        "The read timeout (in milliseconds) to use when downloading resources."))
                 .addOption(newOptionWithArg(ARGUMENT.CONNECTION_STRING, "connStr",
                         "The connection string to the database."))
                 .addOption(newOptionWithArg(ARGUMENT.DB_NAME, "user",
@@ -1058,6 +1060,10 @@ public static class ARGUMENT {
          * The CLI argument name indicating the connection timeout.
          */
         public static final String CONNECTION_TIMEOUT = "connectiontimeout";
+        /**
+         * The CLI argument name indicating the connection read timeout.
+         */
+        public static final String CONNECTION_READ_TIMEOUT = "readtimeout";
         /**
          * The short CLI argument name for setting the location of an additional
          * properties file.

diff --git a/core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java b/core/src/main/java/org/owasp/dependencycheck/agent/DependencyCheckScanAgent.java
@@ -141,6 +141,10 @@ public class DependencyCheckScanAgent {
      * The Connection Timeout.
      */
     private String connectionTimeout;
+    /**
+     * The Connection Read Timeout.
+     */
+    private String readTimeout;
     /**
      * The file path used for verbose logging.
      */
@@ -505,6 +509,24 @@ public void setConnectionTimeout(String connectionTimeout) {
         this.connectionTimeout = connectionTimeout;
     }
 
+    /**
+     * Get the value of readTimeout.
+     *
+     * @return the value of readTimeout
+     */
+    public String getReadTimeout() {
+        return readTimeout;
+    }
+
+    /**
+     * Set the value of readTimeout.
+     *
+     * @param readTimeout new value of readTimeout
+     */
+    public void setReadTimeout(String readTimeout) {
+        this.readTimeout = readTimeout;
+    }
+
     /**
      * Get the value of logFile.
      *
@@ -933,6 +955,7 @@ private void populateSettings() {
         settings.setStringIfNotEmpty(Settings.KEYS.PROXY_USERNAME, proxyUsername);
         settings.setStringIfNotEmpty(Settings.KEYS.PROXY_PASSWORD, proxyPassword);
         settings.setStringIfNotEmpty(Settings.KEYS.CONNECTION_TIMEOUT, connectionTimeout);
+        settings.setStringIfNotEmpty(Settings.KEYS.CONNECTION_READ_TIMEOUT, readTimeout);
         settings.setStringIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressionFile);
         settings.setStringIfNotEmpty(Settings.KEYS.CVE_CPE_STARTS_WITH_FILTER, cpeStartsWithFilter);
         settings.setBoolean(Settings.KEYS.ANALYZER_CENTRAL_ENABLED, centralAnalyzerEnabled);

diff --git a/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java b/maven/src/main/java/org/owasp/dependencycheck/maven/BaseDependencyCheckMojo.java
@@ -327,6 +327,12 @@ public abstract class BaseDependencyCheckMojo extends AbstractMojo implements Ma
     @SuppressWarnings("CanBeFinal")
     @Parameter(property = "connectionTimeout")
     private String connectionTimeout;
+    /**
+     * The Read Timeout.
+     */
+    @SuppressWarnings("CanBeFinal")
+    @Parameter(property = "readTimeout")
+    private String readTimeout;
     /**
      * Sets whether dependency-check should check if there is a new version
      * available.
@@ -2049,6 +2055,7 @@ protected void populateSettings() {
         settings.setArrayIfNotEmpty(Settings.KEYS.SUPPRESSION_FILE, suppressions);
         settings.setBooleanIfNotNull(Settings.KEYS.UPDATE_VERSION_CHECK_ENABLED, versionCheckEnabled);
         settings.setStringIfNotEmpty(Settings.KEYS.CONNECTION_TIMEOUT, connectionTimeout);
+        settings.setStringIfNotEmpty(Settings.KEYS.CONNECTION_READ_TIMEOUT, readTimeout);
         settings.setStringIfNotEmpty(Settings.KEYS.HINTS_FILE, hintsFile);
         settings.setFloat(Settings.KEYS.JUNIT_FAIL_ON_CVSS, junitFailOnCVSS);
         settings.setBooleanIfNotNull(Settings.KEYS.ANALYZER_JAR_ENABLED, jarAnalyzerEnabled);

diff --git a/utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java b/utils/src/main/java/org/owasp/dependencycheck/utils/Settings.java
@@ -254,6 +254,10 @@ public static final class KEYS {
          * The properties key for the connection timeout.
          */
         public static final String CONNECTION_TIMEOUT = "connection.timeout";
+        /**
+         * The properties key for the connection read timeout.
+         */
+        public static final String CONNECTION_READ_TIMEOUT = "connection.read.timeout";
         /**
          * The location of the temporary directory.
          */

diff --git a/utils/src/main/java/org/owasp/dependencycheck/utils/URLConnectionFactory.java b/utils/src/main/java/org/owasp/dependencycheck/utils/URLConnectionFactory.java
@@ -112,7 +112,11 @@ public PasswordAuthentication getPasswordAuthentication() {
                 conn = (HttpURLConnection) url.openConnection();
             }
             final int connectionTimeout = settings.getInt(Settings.KEYS.CONNECTION_TIMEOUT, 10000);
+            // set a conservative long default timeout to compensate for MITM-proxies that return the (final) bytes only
+            // after all security checks passed
+            final int readTimeout = settings.getInt(Settings.KEYS.CONNECTION_READ_TIMEOUT, 60_000);
             conn.setConnectTimeout(connectionTimeout);
+            conn.setReadTimeout(readTimeout);
             conn.setInstanceFollowRedirects(true);
         } catch (IOException ex) {
             if (conn != null) {