Improve HTTP client timeouts (#275)

* 1.6.2 release (#269)

* 1.6.2 release

* Make DefaultHttpClient timeouts settable

* Refactor timeout names

Co-authored-by: SomkaPe <[email protected]>

Author: Avery-Dunn

src/main/java/com/microsoft/aad/msal4j/AbstractClientApplicationBase.java

@@ -66,6 +66,14 @@ abstract class AbstractClientApplicationBase implements IClientApplicationBase {
     @Getter
     private SSLSocketFactory sslSocketFactory;
 
+    @Accessors(fluent = true)
+    @Getter
+    private Integer connectTimeoutForDefaultHttpClient;
+
+    @Accessors(fluent = true)
+    @Getter
+    private Integer readTimeoutForDefaultHttpClient;
+
     @Accessors(fluent = true)
     @Getter
     protected TokenCache tokenCache;
@@ -284,6 +292,8 @@ abstract static class Builder<T extends Builder<T>> {
         private ITokenCacheAccessAspect tokenCacheAccessAspect;
         private AadInstanceDiscoveryResponse aadInstanceDiscoveryResponse;
         private String clientCapabilities;
+        private Integer connectTimeoutForDefaultHttpClient;
+        private Integer readTimeoutForDefaultHttpClient;
 
         /**
          * Constructor to create instance of Builder of client application
@@ -446,6 +456,34 @@ public T sslSocketFactory(SSLSocketFactory val) {
             return self();
         }
 
+        /**
+         * Sets the connect timeout value used in HttpsURLConnection connections made by {@link DefaultHttpClient},
+         * and is not needed if using a custom HTTP client
+         *
+         * @param val timeout value in milliseconds
+         * @return instance of the Builder on which method was called
+         */
+        public T connectTimeoutForDefaultHttpClient(Integer val) {
+            validateNotNull("connectTimeoutForDefaultHttpClient", val);
+
+            connectTimeoutForDefaultHttpClient = val;
+            return self();
+        }
+
+        /**
+         * Sets the read timeout value used in HttpsURLConnection connections made by {@link DefaultHttpClient},
+         * and is not needed if using a custom HTTP client
+         *
+         * @param val timeout value in milliseconds
+         * @return instance of the Builder on which method was called
+         */
+        public T readTimeoutForDefaultHttpClient(Integer val) {
+            validateNotNull("readTimeoutForDefaultHttpClient", val);
+
+            readTimeoutForDefaultHttpClient = val;
+            return self();
+        }
+
         T telemetryConsumer(Consumer<List<HashMap<String, String>>> val) {
             validateNotNull("telemetryConsumer", val);
 
@@ -549,10 +587,12 @@ public T clientCapabilities(Set<String> capabilities) {
         telemetryConsumer = builder.telemetryConsumer;
         proxy = builder.proxy;
         sslSocketFactory = builder.sslSocketFactory;
+        connectTimeoutForDefaultHttpClient = builder.connectTimeoutForDefaultHttpClient;
+        readTimeoutForDefaultHttpClient = builder.readTimeoutForDefaultHttpClient;
         serviceBundle = new ServiceBundle(
                 builder.executorService,
                 builder.httpClient == null ?
-                        new DefaultHttpClient(builder.proxy, builder.sslSocketFactory) :
+                        new DefaultHttpClient(builder.proxy, builder.sslSocketFactory, builder.connectTimeoutForDefaultHttpClient, builder.readTimeoutForDefaultHttpClient) :
                         builder.httpClient,
                 new TelemetryManager(telemetryConsumer, builder.onlySendFailureTelemetry));
         authenticationAuthority = builder.authenticationAuthority;

src/main/java/com/microsoft/aad/msal4j/DefaultHttpClient.java

@@ -15,12 +15,14 @@ class DefaultHttpClient implements IHttpClient {
 
     private final Proxy proxy;
     private final SSLSocketFactory sslSocketFactory;
-    public int DEFAULTCONNECTIONTIMEOUT = 3000;
-    public int DEFAULTREADTIMEOUT = 5000;
+    private int connectTimeout = 10000;
+    private int readTimeout = 15000;
 
-    DefaultHttpClient(Proxy proxy, SSLSocketFactory sslSocketFactory){
+    DefaultHttpClient(Proxy proxy, SSLSocketFactory sslSocketFactory, Integer connectTimeout, Integer readTimeout){
         this.proxy = proxy;
         this.sslSocketFactory = sslSocketFactory;
+        if (connectTimeout != null) this.connectTimeout = connectTimeout;
+        if (readTimeout != null) this.readTimeout = readTimeout;
     }
 
     public IHttpResponse send(HttpRequest httpRequest) throws Exception{
@@ -78,8 +80,8 @@ private HttpsURLConnection openConnection(final URL finalURL)
             connection.setSSLSocketFactory(sslSocketFactory);
         }
 
-        connection.setConnectTimeout(DEFAULTCONNECTIONTIMEOUT);
-        connection.setReadTimeout(DEFAULTREADTIMEOUT);
+        connection.setConnectTimeout(connectTimeout);
+        connection.setReadTimeout(readTimeout);
 
         return connection;
     }

src/test/java/com/microsoft/aad/msal4j/TokenRequestExecutorTest.java

@@ -119,7 +119,7 @@ private TokenRequestExecutor createMockedTokenRequest() throws URISyntaxExceptio
 
         ServiceBundle serviceBundle = new ServiceBundle(
                 null,
-                new DefaultHttpClient(null, null),
+                new DefaultHttpClient(null, null, null, null),
                 new TelemetryManager(null, false));
 
         return PowerMock.createPartialMock(