SONAR-22039 Support new timeout properties

sonar-scanner-engine/src/main/java/org/sonar/scanner/bootstrap/ScannerWsClientProvider.java

@@ -21,6 +21,8 @@ package org.sonar.scanner.bootstrap;
 
 import java.net.InetSocketAddress;
 import java.net.Proxy;
+import java.time.Duration;
+import java.time.format.DateTimeParseException;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.notifications.AnalysisWarnings;
 import org.sonar.api.utils.System2;
@@ -37,11 +39,16 @@ import static org.sonar.core.config.ProxyProperties.HTTP_PROXY_PASSWORD;
 import static org.sonar.core.config.ProxyProperties.HTTP_PROXY_USER;
 
 public class ScannerWsClientProvider {
-  static final int CONNECT_TIMEOUT_MS = 5_000;
+  static final int DEFAULT_CONNECT_TIMEOUT = 5;
+  static final int DEFAULT_RESPONSE_TIMEOUT = 0;
   static final String READ_TIMEOUT_SEC_PROPERTY = "sonar.ws.timeout";
   public static final String TOKEN_PROPERTY = "sonar.token";
   private static final String TOKEN_ENV_VARIABLE = "SONAR_TOKEN";
   static final int DEFAULT_READ_TIMEOUT_SEC = 60;
+  public static final String SONAR_SCANNER_PROXY_PORT = "sonar.scanner.proxyPort";
+  public static final String SONAR_SCANNER_CONNECT_TIMEOUT = "sonar.scanner.connectTimeout";
+  public static final String SONAR_SCANNER_SOCKET_TIMEOUT = "sonar.scanner.socketTimeout";
+  public static final String SONAR_SCANNER_RESPONSE_TIMEOUT = "sonar.scanner.responseTimeout";
 
   @Bean("DefaultScannerWsClient")
   public DefaultScannerWsClient provide(ScannerProperties scannerProps, EnvironmentInformation env, GlobalAnalysisMode globalMode,
@@ -49,13 +56,17 @@ public class ScannerWsClientProvider {
     String url = defaultIfBlank(scannerProps.property("sonar.host.url"), "http://localhost:9000");
     HttpConnector.Builder connectorBuilder = HttpConnector.newBuilder().acceptGzip(true);
 
-    String timeoutSec = defaultIfBlank(scannerProps.property(READ_TIMEOUT_SEC_PROPERTY), valueOf(DEFAULT_READ_TIMEOUT_SEC));
+    String oldSocketTimeout = defaultIfBlank(scannerProps.property(READ_TIMEOUT_SEC_PROPERTY), valueOf(DEFAULT_READ_TIMEOUT_SEC));
+    String socketTimeout = defaultIfBlank(scannerProps.property(SONAR_SCANNER_SOCKET_TIMEOUT), oldSocketTimeout);
+    String connectTimeout = defaultIfBlank(scannerProps.property(SONAR_SCANNER_CONNECT_TIMEOUT), valueOf(DEFAULT_CONNECT_TIMEOUT));
+    String responseTimeout = defaultIfBlank(scannerProps.property(SONAR_SCANNER_RESPONSE_TIMEOUT), valueOf(DEFAULT_RESPONSE_TIMEOUT));
     String envVarToken = defaultIfBlank(system.envVariable(TOKEN_ENV_VARIABLE), null);
     String token = defaultIfBlank(scannerProps.property(TOKEN_PROPERTY), envVarToken);
     String login = defaultIfBlank(scannerProps.property(CoreProperties.LOGIN), token);
     connectorBuilder
-      .readTimeoutMilliseconds(parseInt(timeoutSec) * 1_000)
-      .connectTimeoutMilliseconds(CONNECT_TIMEOUT_MS)
+      .readTimeoutMilliseconds(parseDurationProperty(socketTimeout, SONAR_SCANNER_SOCKET_TIMEOUT))
+      .connectTimeoutMilliseconds(parseDurationProperty(connectTimeout, SONAR_SCANNER_CONNECT_TIMEOUT))
+      .responseTimeoutMilliseconds(parseDurationProperty(responseTimeout, SONAR_SCANNER_RESPONSE_TIMEOUT))
       .userAgent(env.toString())
       .url(url)
       .credentials(login, scannerProps.property(CoreProperties.PASSWORD));
@@ -63,13 +74,8 @@ public class ScannerWsClientProvider {
     // OkHttp detects 'http.proxyHost' java property already, so just focus on sonar properties
     String proxyHost = defaultIfBlank(scannerProps.property("sonar.scanner.proxyHost"), null);
     if (proxyHost != null) {
-      int proxyPort;
-      String proxyPortStr = defaultIfBlank(scannerProps.property("sonar.scanner.proxyPort"), url.startsWith("https") ? "443" : "80");
-      try {
-        proxyPort = parseInt(proxyPortStr);
-      } catch (NumberFormatException e) {
-        throw new IllegalArgumentException("Invalid proxy port: " + proxyPortStr, e);
-      }
+      String proxyPortStr = defaultIfBlank(scannerProps.property(SONAR_SCANNER_PROXY_PORT), url.startsWith("https") ? "443" : "80");
+      var proxyPort = parseIntProperty(proxyPortStr, SONAR_SCANNER_PROXY_PORT);
       connectorBuilder.proxy(new Proxy(Proxy.Type.HTTP, new InetSocketAddress(proxyHost, proxyPort)));
     }
 
@@ -83,4 +89,24 @@ public class ScannerWsClientProvider {
 
     return new DefaultScannerWsClient(WsClientFactories.getDefault().newClient(connectorBuilder.build()), login != null, globalMode, analysisWarnings);
   }
+
+  private static int parseIntProperty(String propValue, String propKey) {
+    try {
+      return parseInt(propValue);
+    } catch (NumberFormatException e) {
+      throw new IllegalArgumentException(propKey + " is not a valid integer: " + propValue, e);
+    }
+  }
+
+  /**
+   * For testing, we can accept timeouts that are smaller than a second, expressed using ISO-8601 format for durations.
+   * If we can't parse as ISO-8601, then fallback to the official format that is simply the number of seconds
+   */
+  private static int parseDurationProperty(String propValue, String propKey) {
+    try {
+      return (int) Duration.parse(propValue).toMillis();
+    } catch (DateTimeParseException e) {
+      return parseIntProperty(propValue, propKey) * 1_000;
+    }
+  }
 }

sonar-scanner-engine/src/test/java/org/sonar/scanner/bootstrap/ScannerWsClientProviderTest.java

@@ -125,6 +125,46 @@ class ScannerWsClientProviderTest {
     assertThat(httpConnector.okHttpClient().proxy()).isNull();
   }
 
+  @Test
+  void it_should_timeout_on_long_response() {
+    scannerProps.put("sonar.host.url", sonarqubeMock.baseUrl());
+    scannerProps.put("sonar.scanner.responseTimeout", "PT0.2S");
+
+    DefaultScannerWsClient client = underTest.provide(new ScannerProperties(scannerProps), env, GLOBAL_ANALYSIS_MODE, system2, ANALYSIS_WARNINGS);
+
+    sonarqubeMock.stubFor(get("/api/plugins/installed")
+      .willReturn(aResponse().withStatus(200)
+        .withFixedDelay(2000)
+        .withBody("Success")));
+
+    HttpConnector httpConnector = (HttpConnector) client.wsConnector();
+
+    var getRequest = new GetRequest("api/plugins/installed");
+    var thrown = assertThrows(IllegalStateException.class, () -> httpConnector.call(getRequest));
+
+    assertThat(thrown).hasStackTraceContaining("timeout");
+  }
+
+  @Test
+  void it_should_timeout_on_slow_response() {
+    scannerProps.put("sonar.host.url", sonarqubeMock.baseUrl());
+    scannerProps.put("sonar.scanner.socketTimeout", "PT0.2S");
+
+    DefaultScannerWsClient client = underTest.provide(new ScannerProperties(scannerProps), env, GLOBAL_ANALYSIS_MODE, system2, ANALYSIS_WARNINGS);
+
+    sonarqubeMock.stubFor(get("/api/plugins/installed")
+      .willReturn(aResponse().withStatus(200)
+        .withChunkedDribbleDelay(2, 2000)
+        .withBody("Success")));
+
+    HttpConnector httpConnector = (HttpConnector) client.wsConnector();
+
+    var getRequest = new GetRequest("api/plugins/installed");
+    var thrown = assertThrows(IllegalStateException.class, () -> httpConnector.call(getRequest));
+
+    assertThat(thrown).hasStackTraceContaining("timeout");
+  }
+
   @Test
   void it_should_honor_scanner_proxy_settings() {
     scannerProps.put("sonar.host.url", sonarqubeMock.baseUrl());

sonar-ws/src/main/java/org/sonarqube/ws/client/HttpConnector.java

@@ -57,6 +57,7 @@ public class HttpConnector implements WsConnector {
 
   public static final int DEFAULT_CONNECT_TIMEOUT_MILLISECONDS = 30_000;
   public static final int DEFAULT_READ_TIMEOUT_MILLISECONDS = 60_000;
+  public static final int DEFAULT_RESPONSE_TIMEOUT_MILLISECONDS = 0;
   private static final String JSON = "application/json; charset=utf-8";
 
   /**
@@ -85,6 +86,7 @@ public class HttpConnector implements WsConnector {
     okHttpClientBuilder.setProxyLogin(builder.proxyLogin);
     okHttpClientBuilder.setProxyPassword(builder.proxyPassword);
     okHttpClientBuilder.setConnectTimeoutMs(builder.connectTimeoutMs);
+    okHttpClientBuilder.setResponseTimeoutMs(builder.responseTimeoutMs);
     okHttpClientBuilder.setReadTimeoutMs(builder.readTimeoutMs);
     okHttpClientBuilder.setSSLSocketFactory(builder.sslSocketFactory);
     okHttpClientBuilder.setTrustManager(builder.sslTrustManager);
@@ -264,6 +266,7 @@ public class HttpConnector implements WsConnector {
     private String systemPassCode;
     private int connectTimeoutMs = DEFAULT_CONNECT_TIMEOUT_MILLISECONDS;
     private int readTimeoutMs = DEFAULT_READ_TIMEOUT_MILLISECONDS;
+    private int responseTimeoutMs = DEFAULT_RESPONSE_TIMEOUT_MILLISECONDS;
     private SSLSocketFactory sslSocketFactory = null;
     private X509TrustManager sslTrustManager = null;
     private boolean acceptGzip = false;
@@ -354,6 +357,15 @@ public class HttpConnector implements WsConnector {
       return this;
     }
 
+    /**
+     * Sets the response timeout to a specified timeout, in milliseconds.
+     * A timeout of zero is interpreted as an infinite timeout. Default value is {@link #DEFAULT_RESPONSE_TIMEOUT_MILLISECONDS}
+     */
+    public Builder responseTimeoutMilliseconds(int i) {
+      this.responseTimeoutMs = i;
+      return this;
+    }
+
     public Builder proxy(@Nullable Proxy proxy) {
       this.proxy = proxy;
       return this;

sonar-ws/src/main/java/org/sonarqube/ws/client/OkHttpClientBuilder.java

@@ -70,6 +70,7 @@ public class OkHttpClientBuilder {
   private Boolean followRedirects;
   private long connectTimeoutMs = -1;
   private long readTimeoutMs = -1;
+  private long responseTimeoutMs = -1;
   private SSLSocketFactory sslSocketFactory = null;
   private X509TrustManager sslTrustManager = null;
   private boolean acceptGzip = false;
@@ -169,6 +170,18 @@ public class OkHttpClientBuilder {
     return this;
   }
 
+  /**
+   * Sets the default response timeout for new connections. A value of 0 means no timeout.
+   * Default is to have no timeout.
+   */
+  public OkHttpClientBuilder setResponseTimeoutMs(long l) {
+    if (l < 0) {
+      throw new IllegalArgumentException("Response timeout must be positive. Got " + l);
+    }
+    this.responseTimeoutMs = l;
+    return this;
+  }
+
   /**
    * Set if redirects should be followed or not.
    * Default is defined by OkHttp (true, follow redirects).
@@ -187,6 +200,9 @@ public class OkHttpClientBuilder {
     if (readTimeoutMs >= 0) {
       builder.readTimeout(readTimeoutMs, TimeUnit.MILLISECONDS);
     }
+    if (responseTimeoutMs >= 0) {
+      builder.callTimeout(responseTimeoutMs, TimeUnit.MILLISECONDS);
+    }
     builder.addNetworkInterceptor(this::addHeaders);
     if(!acceptGzip) {
       builder.addNetworkInterceptor(new GzipRejectorInterceptor());

sonar-ws/src/test/java/org/sonarqube/ws/client/HttpConnectorTest.java

@@ -107,7 +107,7 @@ public class HttpConnectorTest {
     server.enqueue(new MockResponse().setResponseCode(200).addHeader("Content-Encoding", "gzip")
       .setBody(gzip("potentially a body with 100 GB of data normally encoded in gzip")));
 
-    //by default we dont accept gzip
+    // by default we dont accept gzip
     underTest = HttpConnector.newBuilder().url(serverUrl).build();
     GetRequest request = new GetRequest("rest/api/1.0/repos");
 
@@ -319,10 +319,12 @@ public class HttpConnectorTest {
       .url(serverUrl)
       .readTimeoutMilliseconds(42)
       .connectTimeoutMilliseconds(74)
+      .responseTimeoutMilliseconds(53)
       .build();
 
     assertThat(underTest.okHttpClient().readTimeoutMillis()).isEqualTo(42);
     assertThat(underTest.okHttpClient().connectTimeoutMillis()).isEqualTo(74);
+    assertThat(underTest.okHttpClient().callTimeoutMillis()).isEqualTo(53);
   }
 
   @Test

sonar-ws/src/test/java/org/sonarqube/ws/client/OkHttpClientBuilderTest.java

@@ -83,4 +83,11 @@ public class OkHttpClientBuilderTest {
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessage("Read timeout must be positive. Got -10");
   }
+
+  @Test
+  public void build_throws_IAE_if_response_timeout_is_negative() {
+    assertThatThrownBy(() -> underTest.setResponseTimeoutMs(-10))
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Response timeout must be positive. Got -10");
+  }
 }