HTTPCLIENT-2386: Classic transport to use connect timeout as a default if TLS timeout has not been explicitly set

Author: ok2c

httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java

@@ -41,6 +41,7 @@
 import org.apache.hc.client5.http.SchemePortResolver;
 import org.apache.hc.client5.http.SystemDefaultDnsResolver;
 import org.apache.hc.client5.http.UnsupportedSchemeException;
+import org.apache.hc.client5.http.config.TlsConfig;
 import org.apache.hc.client5.http.impl.ConnPoolSupport;
 import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
 import org.apache.hc.client5.http.io.DetachedSocketFactory;
@@ -160,7 +161,6 @@ public void connect(
         Args.notNull(socketConfig, "Socket config");
         Args.notNull(context, "Context");
 
-        final Timeout soTimeout = socketConfig.getSoTimeout();
         final SocketAddress socksProxyAddress = socketConfig.getSocksProxyAddress();
         final Proxy socksProxy = socksProxyAddress != null ? new Proxy(Proxy.Type.SOCKS, socksProxyAddress) : null;
 
@@ -186,8 +186,9 @@ public void connect(
                     socket.bind(localAddress);
                 }
                 conn.bind(socket);
-                if (soTimeout != null) {
-                    socket.setSoTimeout(soTimeout.toMillisecondsIntBound());
+                final Timeout socketTimeout = socketConfig.getSoTimeout();
+                if (socketTimeout != null) {
+                    socket.setSoTimeout(socketTimeout.toMillisecondsIntBound());
                 }
                 socket.setReuseAddress(socketConfig.isSoReuseAddress());
                 socket.setTcpNoDelay(socketConfig.isTcpNoDelay());
@@ -217,16 +218,22 @@ public void connect(
                 if (LOG.isDebugEnabled()) {
                     LOG.debug("{} {} connected {}->{}", ConnPoolSupport.getId(conn), endpointHost, conn.getLocalAddress(), conn.getRemoteAddress());
                 }
-                conn.setSocketTimeout(soTimeout);
                 final TlsSocketStrategy tlsSocketStrategy = tlsSocketStrategyLookup != null ? tlsSocketStrategyLookup.lookup(endpointHost.getSchemeName()) : null;
                 if (tlsSocketStrategy != null) {
                     final NamedEndpoint tlsName = endpointName != null ? endpointName : endpointHost;
                     onBeforeTlsHandshake(context, endpointHost);
                     if (LOG.isDebugEnabled()) {
                         LOG.debug("{} {} upgrading to TLS", ConnPoolSupport.getId(conn), tlsName);
                     }
+                    final TlsConfig tlsConfig = attachment instanceof TlsConfig ? (TlsConfig) attachment : TlsConfig.DEFAULT;
+                    final int soTimeout = socket.getSoTimeout();
+                    final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout() != null ? tlsConfig.getHandshakeTimeout() : connectTimeout;
+                    if (handshakeTimeout != null) {
+                        socket.setSoTimeout(handshakeTimeout.toMillisecondsIntBound());
+                    }
                     final SSLSocket sslSocket = tlsSocketStrategy.upgrade(socket, tlsName.getHostName(), tlsName.getPort(), attachment, context);
                     conn.bind(sslSocket, socket);
+                    socket.setSoTimeout(soTimeout);
                     onAfterTlsHandshake(context, endpointHost);
                     if (LOG.isDebugEnabled()) {
                         LOG.debug("{} {} upgraded to TLS", ConnPoolSupport.getId(conn), tlsName);

httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java

@@ -141,7 +141,7 @@ public void completed(final IOSession session) {
                         if (tlsStrategy != null) {
                             try {
                                 final Timeout socketTimeout = connection.getSocketTimeout();
-                                final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout();
+                                final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout() != null ? tlsConfig.getHandshakeTimeout() : connectTimeout;
                                 final NamedEndpoint tlsName = endpointName != null ? endpointName : endpointHost;
                                 onBeforeTlsHandshake(context, endpointHost);
                                 if (LOG.isDebugEnabled()) {
@@ -151,7 +151,7 @@ public void completed(final IOSession session) {
                                         connection,
                                         tlsName,
                                         attachment,
-                                        handshakeTimeout != null ? handshakeTimeout : connectTimeout,
+                                        handshakeTimeout,
                                         new FutureContribution<TransportSecurityLayer>(future) {
 
                                             @Override

httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java

@@ -220,8 +220,6 @@ private void executeHandshake(
             final SSLSocket upgradedSocket,
             final String target,
             final Object attachment) throws IOException {
-        final TlsConfig tlsConfig = attachment instanceof TlsConfig ? (TlsConfig) attachment : TlsConfig.DEFAULT;
-
         final SSLParameters sslParameters = upgradedSocket.getSSLParameters();
         if (supportedProtocols != null) {
             sslParameters.setProtocols(supportedProtocols);
@@ -238,17 +236,11 @@ private void executeHandshake(
         }
         upgradedSocket.setSSLParameters(sslParameters);
 
-        final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout();
-        if (handshakeTimeout != null) {
-            upgradedSocket.setSoTimeout(handshakeTimeout.toMillisecondsIntBound());
-        }
-
         initializeSocket(upgradedSocket);
 
         if (LOG.isDebugEnabled()) {
             LOG.debug("Enabled protocols: {}", (Object) upgradedSocket.getEnabledProtocols());
             LOG.debug("Enabled cipher suites: {}", (Object) upgradedSocket.getEnabledCipherSuites());
-            LOG.debug("Starting handshake ({})", handshakeTimeout);
         }
         upgradedSocket.startHandshake();
         verifySession(target, upgradedSocket.getSession());