HTTPCLIENT-2305: SSLConnectionSocketFactory allows socket.connect to be decorated (#499)

httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestSSLSocketFactory.java

@@ -35,6 +35,7 @@ import java.net.Socket;
 import java.security.KeyManagementException;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLContext;
@@ -57,6 +58,7 @@ import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.ssl.SSLContexts;
 import org.apache.hc.core5.ssl.TrustStrategy;
 import org.apache.hc.core5.util.TimeValue;
+import org.apache.hc.core5.util.Timeout;
 import org.hamcrest.CoreMatchers;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
@@ -123,6 +125,44 @@ public class TestSSLSocketFactory {
         }
     }
 
+    @Test
+    public void testBasicSslConnectOverride() throws Exception {
+        this.server = ServerBootstrap.bootstrap()
+                .setSslContext(SSLTestContexts.createServerSSLContext())
+                .create();
+        this.server.start();
+
+        final HttpContext context = new BasicHttpContext();
+        final AtomicBoolean connectCalled = new AtomicBoolean();
+        final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(
+                SSLTestContexts.createClientSSLContext()) {
+            @Override
+            protected void connectSocket(
+                    final Socket sock,
+                    final InetSocketAddress remoteAddress,
+                    final Timeout connectTimeout,
+                    final HttpContext context) throws IOException {
+                connectCalled.set(true);
+                super.connectSocket(sock, remoteAddress, connectTimeout, context);
+            }
+        };
+        try (final Socket socket = socketFactory.createSocket(context)) {
+            final InetSocketAddress remoteAddress = new InetSocketAddress("localhost", this.server.getLocalPort());
+            final HttpHost target = new HttpHost("https", "localhost", this.server.getLocalPort());
+            try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket(
+                    TimeValue.ZERO_MILLISECONDS,
+                    socket,
+                    target,
+                    remoteAddress,
+                    null,
+                    context)) {
+                final SSLSession sslsession = sslSocket.getSession();
+                Assertions.assertNotNull(sslsession);
+                Assertions.assertTrue(connectCalled.get());
+            }
+        }
+    }
+
     @Test
     public void testBasicDefaultHostnameVerifier() throws Exception {
         // @formatter:off

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

@@ -32,6 +32,7 @@ import java.io.InputStream;
 import java.net.InetSocketAddress;
 import java.net.Proxy;
 import java.net.Socket;
+import java.net.SocketAddress;
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
@@ -236,6 +237,40 @@ public class SSLConnectionSocketFactory implements LayeredConnectionSocketFactor
             sock.bind(localAddress);
         }
         try {
+            connectSocket(sock, remoteAddress, connectTimeout, context);
+        } catch (final IOException ex) {
+            Closer.closeQuietly(sock);
+            throw ex;
+        }
+        // Setup SSL layering if necessary
+        if (sock instanceof SSLSocket) {
+            final SSLSocket sslsock = (SSLSocket) sock;
+            executeHandshake(sslsock, host.getHostName(), attachment, context);
+            return sock;
+        }
+        return createLayeredSocket(sock, host.getHostName(), remoteAddress.getPort(), attachment, context);
+    }
+
+    /**
+     * Connects the socket to the target host with the given resolved remote address using
+     * {@link Socket#connect(SocketAddress, int)}. This method may be overridden to customize
+     * how precisely {@link Socket#connect(SocketAddress, int)} is handled without impacting
+     * other connection establishment code within {@link #executeHandshake(SSLSocket, String, Object, HttpContext)},
+     * for example.
+     *
+     * @param sock the socket to connect.
+     * @param remoteAddress the resolved remote address to connect to.
+     * @param connectTimeout connect timeout.
+     * @param context the actual HTTP context.
+     * @throws IOException if an I/O error occurs
+     */
+    protected void connectSocket(
+            final Socket sock,
+            final InetSocketAddress remoteAddress,
+            final Timeout connectTimeout,
+            final HttpContext context) throws IOException {
+        Args.notNull(sock, "Socket");
+        Args.notNull(remoteAddress, "Remote address");
         if (LOG.isDebugEnabled()) {
             LOG.debug("Connecting socket to {} with timeout {}", remoteAddress, connectTimeout);
         }
@@ -252,17 +287,6 @@ public class SSLConnectionSocketFactory implements LayeredConnectionSocketFactor
             // only checked exceptions are wrapped - error and RTExceptions are rethrown by doPrivileged
             throw (IOException) e.getCause();
         }
-        } catch (final IOException ex) {
-            Closer.closeQuietly(sock);
-            throw ex;
-        }
-        // Setup SSL layering if necessary
-        if (sock instanceof SSLSocket) {
-            final SSLSocket sslsock = (SSLSocket) sock;
-            executeHandshake(sslsock, host.getHostName(), attachment, context);
-            return sock;
-        }
-        return createLayeredSocket(sock, host.getHostName(), remoteAddress.getPort(), attachment, context);
     }
 
     @Override