Move plain socket create and connect operations to HttpClientConnectionOperator

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

@@ -42,9 +42,9 @@
 import org.apache.hc.client5.http.UnsupportedSchemeException;
 import org.apache.hc.client5.http.impl.ConnPoolSupport;
 import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
+import org.apache.hc.client5.http.io.DetachedSocketFactory;
 import org.apache.hc.client5.http.io.HttpClientConnectionOperator;
 import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
-import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
 import org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory;
 import org.apache.hc.core5.annotation.Contract;
@@ -52,9 +52,11 @@
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.ConnectionClosedException;
 import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.URIScheme;
 import org.apache.hc.core5.http.config.Lookup;
 import org.apache.hc.core5.http.io.SocketConfig;
 import org.apache.hc.core5.http.protocol.HttpContext;
+import org.apache.hc.core5.io.Closer;
 import org.apache.hc.core5.util.Args;
 import org.apache.hc.core5.util.TimeValue;
 import org.apache.hc.core5.util.Timeout;
@@ -72,35 +74,41 @@
 @Contract(threading = ThreadingBehavior.STATELESS)
 public class DefaultHttpClientConnectionOperator implements HttpClientConnectionOperator {
 
-    static final String SOCKET_FACTORY_REGISTRY = "http.socket-factory-registry";
-
     private static final Logger LOG = LoggerFactory.getLogger(DefaultHttpClientConnectionOperator.class);
 
+    static final DetachedSocketFactory PLAIN_SOCKET_FACTORY = new DetachedSocketFactory() {
+
+        @Override
+        public Socket create(final Proxy socksProxy) throws IOException {
+            return socksProxy == null ? new Socket() : new Socket(socksProxy);
+        }
+
+    };
+
+    private final DetachedSocketFactory detachedSocketFactory;
     private final Lookup<ConnectionSocketFactory> socketFactoryRegistry;
     private final SchemePortResolver schemePortResolver;
     private final DnsResolver dnsResolver;
 
     public DefaultHttpClientConnectionOperator(
+            final DetachedSocketFactory detachedSocketFactory,
             final Lookup<ConnectionSocketFactory> socketFactoryRegistry,
             final SchemePortResolver schemePortResolver,
             final DnsResolver dnsResolver) {
         super();
-        Args.notNull(socketFactoryRegistry, "Socket factory registry");
+        this.detachedSocketFactory = Args.notNull(detachedSocketFactory, "Plain socket factory");
-        this.socketFactoryRegistry = socketFactoryRegistry;
+        this.socketFactoryRegistry = Args.notNull(socketFactoryRegistry, "Socket factory registry");
         this.schemePortResolver = schemePortResolver != null ? schemePortResolver :
             DefaultSchemePortResolver.INSTANCE;
         this.dnsResolver = dnsResolver != null ? dnsResolver :
             SystemDefaultDnsResolver.INSTANCE;
     }
 
-    @SuppressWarnings("unchecked")
+    public DefaultHttpClientConnectionOperator(
-    private Lookup<ConnectionSocketFactory> getSocketFactoryRegistry(final HttpContext context) {
+            final Lookup<ConnectionSocketFactory> socketFactoryRegistry,
-        Lookup<ConnectionSocketFactory> reg = (Lookup<ConnectionSocketFactory>) context.getAttribute(
+            final SchemePortResolver schemePortResolver,
-                SOCKET_FACTORY_REGISTRY);
+            final DnsResolver dnsResolver) {
-        if (reg == null) {
+        this(PLAIN_SOCKET_FACTORY, socketFactoryRegistry, schemePortResolver, dnsResolver);
-            reg = this.socketFactoryRegistry;
-        }
-        return reg;
     }
 
     @Override
@@ -128,11 +136,6 @@ public void connect(
         Args.notNull(host, "Host");
         Args.notNull(socketConfig, "Socket config");
         Args.notNull(context, "Context");
-        final Lookup<ConnectionSocketFactory> registry = getSocketFactoryRegistry(context);
-        final ConnectionSocketFactory sf = registry.lookup(host.getSchemeName());
-        if (sf == null) {
-            throw new UnsupportedSchemeException(host.getSchemeName() + " protocol is not supported");
-        }
         final InetAddress[] remoteAddresses;
         if (host.getAddress() != null) {
             remoteAddresses = new InetAddress[] { host.getAddress() };
@@ -154,47 +157,59 @@ public void connect(
 
         final Timeout soTimeout = socketConfig.getSoTimeout();
         final SocketAddress socksProxyAddress = socketConfig.getSocksProxyAddress();
-        final Proxy proxy = socksProxyAddress != null ? new Proxy(Proxy.Type.SOCKS, socksProxyAddress) : null;
+        final Proxy socksProxy = socksProxyAddress != null ? new Proxy(Proxy.Type.SOCKS, socksProxyAddress) : null;
         final int port = this.schemePortResolver.resolve(host);
         for (int i = 0; i < remoteAddresses.length; i++) {
             final InetAddress address = remoteAddresses[i];
             final boolean last = i == remoteAddresses.length - 1;
-
-            Socket sock = sf.createSocket(proxy, context);
-            if (soTimeout != null) {
-                sock.setSoTimeout(soTimeout.toMillisecondsIntBound());
-            }
-            sock.setReuseAddress(socketConfig.isSoReuseAddress());
-            sock.setTcpNoDelay(socketConfig.isTcpNoDelay());
-            sock.setKeepAlive(socketConfig.isSoKeepAlive());
-            if (socketConfig.getRcvBufSize() > 0) {
-                sock.setReceiveBufferSize(socketConfig.getRcvBufSize());
-            }
-            if (socketConfig.getSndBufSize() > 0) {
-                sock.setSendBufferSize(socketConfig.getSndBufSize());
-            }
-
-            final int linger = socketConfig.getSoLinger().toMillisecondsIntBound();
-            if (linger >= 0) {
-                sock.setSoLinger(true, linger);
-            }
-            conn.bind(sock);
-
             final InetSocketAddress remoteAddress = new InetSocketAddress(address, port);
             if (LOG.isDebugEnabled()) {
                 LOG.debug("{}:{} connecting {}->{} ({})",
                         host.getHostName(), host.getPort(), localAddress, remoteAddress, connectTimeout);
             }
+            final Socket socket = detachedSocketFactory.create(socksProxy);
             try {
-                sock = sf.connectSocket(sock, host, remoteAddress, localAddress, connectTimeout, attachment, context);
+                conn.bind(socket);
-                conn.bind(sock);
+                if (soTimeout != null) {
+                    socket.setSoTimeout(soTimeout.toMillisecondsIntBound());
+                }
+                socket.setReuseAddress(socketConfig.isSoReuseAddress());
+                socket.setTcpNoDelay(socketConfig.isTcpNoDelay());
+                socket.setKeepAlive(socketConfig.isSoKeepAlive());
+                if (socketConfig.getRcvBufSize() > 0) {
+                    socket.setReceiveBufferSize(socketConfig.getRcvBufSize());
+                }
+                if (socketConfig.getSndBufSize() > 0) {
+                    socket.setSendBufferSize(socketConfig.getSndBufSize());
+                }
+
+                final int linger = socketConfig.getSoLinger().toMillisecondsIntBound();
+                if (linger >= 0) {
+                    socket.setSoLinger(true, linger);
+                }
+
+                if (localAddress != null) {
+                    socket.bind(localAddress);
+                }
+                socket.connect(remoteAddress, TimeValue.isPositive(connectTimeout) ? connectTimeout.toMillisecondsIntBound() : 0);
+                conn.bind(socket);
                 conn.setSocketTimeout(soTimeout);
                 if (LOG.isDebugEnabled()) {
                     LOG.debug("{}:{} connected {}->{} as {}",
                             host.getHostName(), host.getPort(), localAddress, remoteAddress, ConnPoolSupport.getId(conn));
                 }
+                final ConnectionSocketFactory connectionSocketFactory = socketFactoryRegistry != null ? socketFactoryRegistry.lookup(host.getSchemeName()) : null;
+                if (connectionSocketFactory instanceof LayeredConnectionSocketFactory && URIScheme.HTTPS.same(host.getSchemeName())) {
+                    final LayeredConnectionSocketFactory lsf = (LayeredConnectionSocketFactory) connectionSocketFactory;
+                    final Socket upgradedSocket = lsf.createLayeredSocket(socket, host.getHostName(), port, attachment, context);
+                    conn.bind(upgradedSocket);
+                }
                 return;
+            } catch (final RuntimeException ex) {
+                Closer.closeQuietly(socket);
+                throw ex;
             } catch (final IOException ex) {
+                Closer.closeQuietly(socket);
                 if (last) {
                     if (LOG.isDebugEnabled()) {
                         LOG.debug("{}:{} connection to {} failed ({}); terminating operation",
@@ -225,9 +240,7 @@ public void upgrade(
             final HttpHost host,
             final Object attachment,
             final HttpContext context) throws IOException {
-        final HttpClientContext clientContext = HttpClientContext.adapt(context);
+        final ConnectionSocketFactory sf = socketFactoryRegistry.lookup(host.getSchemeName());
-        final Lookup<ConnectionSocketFactory> registry = getSocketFactoryRegistry(clientContext);
-        final ConnectionSocketFactory sf = registry.lookup(host.getSchemeName());
         if (sf == null) {
             throw new UnsupportedSchemeException(host.getSchemeName() +
                     " protocol is not supported");

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

@@ -95,7 +95,6 @@ public Future<ManagedAsyncClientConnection> connect(
         final ComplexFuture<ManagedAsyncClientConnection> future = new ComplexFuture<>(callback);
         final HttpHost remoteEndpoint = RoutingSupport.normalize(host, schemePortResolver);
         final InetAddress remoteAddress = host.getAddress();
-        final TlsStrategy tlsStrategy = tlsStrategyLookup != null ? tlsStrategyLookup.lookup(host.getSchemeName()) : null;
         final TlsConfig tlsConfig = attachment instanceof TlsConfig ? (TlsConfig) attachment : TlsConfig.DEFAULT;
         final Future<IOSession> sessionFuture = sessionRequester.connect(
                 connectionInitiator,
@@ -109,6 +108,7 @@ public Future<ManagedAsyncClientConnection> connect(
                     @Override
                     public void completed(final IOSession session) {
                         final DefaultManagedAsyncClientConnection connection = new DefaultManagedAsyncClientConnection(session);
+                        final TlsStrategy tlsStrategy = tlsStrategyLookup != null ? tlsStrategyLookup.lookup(host.getSchemeName()) : null;
                         if (tlsStrategy != null && URIScheme.HTTPS.same(host.getSchemeName())) {
                             try {
                                 final Timeout socketTimeout = connection.getSocketTimeout();

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

@@ -0,0 +1,47 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.hc.client5.http.io;
+
+import java.io.IOException;
+import java.net.Proxy;
+import java.net.Socket;
+
+import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.annotation.ThreadingBehavior;
+
+/**
+ * @since 5.4
+ */
+@Internal
+@Contract(threading = ThreadingBehavior.STATELESS)
+public interface DetachedSocketFactory {
+
+    Socket create(Proxy proxy) throws IOException;
+
+}

httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java

@@ -38,6 +38,7 @@
 import org.apache.hc.client5.http.config.ConnectionConfig;
 import org.apache.hc.client5.http.config.TlsConfig;
 import org.apache.hc.client5.http.io.ConnectionEndpoint;
+import org.apache.hc.client5.http.io.DetachedSocketFactory;
 import org.apache.hc.client5.http.io.LeaseRequest;
 import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
@@ -57,7 +58,6 @@
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 
-@SuppressWarnings({"boxing","static-access"}) // test code
 public class TestBasicHttpClientConnectionManager {
 
     @Mock
@@ -67,7 +67,7 @@ public class TestBasicHttpClientConnectionManager {
     @Mock
     private Lookup<ConnectionSocketFactory> socketFactoryRegistry;
     @Mock
-    private ConnectionSocketFactory plainSocketFactory;
+    private DetachedSocketFactory detachedSocketFactory;
     @Mock
     private LayeredConnectionSocketFactory sslSocketFactory;
     @Mock
@@ -82,8 +82,9 @@ public class TestBasicHttpClientConnectionManager {
     @BeforeEach
     public void setup() throws Exception {
         MockitoAnnotations.openMocks(this);
-        mgr = new BasicHttpClientConnectionManager(
+        mgr = new BasicHttpClientConnectionManager(new DefaultHttpClientConnectionOperator(
-                socketFactoryRegistry, connFactory, schemePortResolver, dnsResolver);
+                detachedSocketFactory, socketFactoryRegistry, schemePortResolver, dnsResolver),
+                connFactory);
     }
 
     @Test
@@ -382,14 +383,13 @@ public void testTargetConnect() throws Exception {
 
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] {remote});
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
-        Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(plainSocketFactory);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
+
-        Mockito.when(plainSocketFactory.connectSocket(
+        Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory);
-                Mockito.eq(socket),
+        Mockito.when(sslSocketFactory.createLayeredSocket(
-                Mockito.any(),
+                Mockito.same(socket),
-                Mockito.any(),
+                Mockito.eq("somehost"),
-                Mockito.any(),
+                Mockito.eq(8443),
-                Mockito.any(),
                 Mockito.any(),
                 Mockito.any())).thenReturn(socket);
 
@@ -397,29 +397,17 @@ public void testTargetConnect() throws Exception {
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(null, context);
+        Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+        Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 234);
-                socket,
+        Mockito.verify(sslSocketFactory).createLayeredSocket(socket, "somehost", 8443, tlsConfig, context);
-                target,
-                new InetSocketAddress(remote, 8443),
-                new InetSocketAddress(local, 0),
-                Timeout.ofMilliseconds(234),
-                tlsConfig,
-                context);
 
         mgr.connect(endpoint1, TimeValue.ofMilliseconds(123), context);
 
         Mockito.verify(dnsResolver, Mockito.times(2)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(2)).resolve(target);
-        Mockito.verify(plainSocketFactory, Mockito.times(2)).createSocket(null, context);
+        Mockito.verify(detachedSocketFactory, Mockito.times(2)).create(null);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+        Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 123);
-                socket,
+        Mockito.verify(sslSocketFactory, Mockito.times(2)).createLayeredSocket(socket, "somehost", 8443, tlsConfig, context);
-                target,
-                new InetSocketAddress(remote, 8443),
-                new InetSocketAddress(local, 0),
-                Timeout.ofMilliseconds(123),
-                tlsConfig,
-                context);
     }
 
     @Test
@@ -453,31 +441,15 @@ public void testProxyConnectAndUpgrade() throws Exception {
         Mockito.when(dnsResolver.resolve("someproxy")).thenReturn(new InetAddress[] {remote});
         Mockito.when(schemePortResolver.resolve(proxy)).thenReturn(8080);
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
-        Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
-        Mockito.when(plainSocketFactory.connectSocket(
-                Mockito.eq(socket),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any())).thenReturn(socket);
 
         mgr.connect(endpoint1, null, context);
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("someproxy");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(proxy);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(null, context);
+        Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+        Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8080), 234);
-                socket,
-                proxy,
-                new InetSocketAddress(remote, 8080),
-                new InetSocketAddress(local, 0),
-                Timeout.ofMilliseconds(234),
-                tlsConfig,
-                context);
 
         Mockito.when(conn.getSocket()).thenReturn(socket);
 

httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java

@@ -40,6 +40,7 @@
 import org.apache.hc.client5.http.SchemePortResolver;
 import org.apache.hc.client5.http.UnsupportedSchemeException;
 import org.apache.hc.client5.http.config.TlsConfig;
+import org.apache.hc.client5.http.io.DetachedSocketFactory;
 import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
 import org.apache.hc.client5.http.socket.ConnectionSocketFactory;
 import org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory;
@@ -48,6 +49,7 @@
 import org.apache.hc.core5.http.io.SocketConfig;
 import org.apache.hc.core5.http.protocol.BasicHttpContext;
 import org.apache.hc.core5.http.protocol.HttpContext;
+import org.apache.hc.core5.http2.HttpVersionPolicy;
 import org.apache.hc.core5.util.TimeValue;
 import org.apache.hc.core5.util.Timeout;
 import org.junit.jupiter.api.Assertions;
@@ -55,12 +57,11 @@
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
-@SuppressWarnings({"boxing","static-access"}) // test code
 public class TestHttpClientConnectionOperator {
 
     private ManagedHttpClientConnection conn;
     private Socket socket;
-    private ConnectionSocketFactory plainSocketFactory;
+    private DetachedSocketFactory detachedSocketFactory;
     private LayeredConnectionSocketFactory sslSocketFactory;
     private Lookup<ConnectionSocketFactory> socketFactoryRegistry;
     private SchemePortResolver schemePortResolver;
@@ -71,13 +72,13 @@ public class TestHttpClientConnectionOperator {
     public void setup() throws Exception {
         conn = Mockito.mock(ManagedHttpClientConnection.class);
         socket = Mockito.mock(Socket.class);
-        plainSocketFactory = Mockito.mock(ConnectionSocketFactory.class);
+        detachedSocketFactory = Mockito.mock(DetachedSocketFactory.class);
         sslSocketFactory = Mockito.mock(LayeredConnectionSocketFactory.class);
         socketFactoryRegistry = Mockito.mock(Lookup.class);
         schemePortResolver = Mockito.mock(SchemePortResolver.class);
         dnsResolver = Mockito.mock(DnsResolver.class);
         connectionOperator = new DefaultHttpClientConnectionOperator(
-                socketFactoryRegistry, schemePortResolver, dnsResolver);
+                detachedSocketFactory, socketFactoryRegistry, schemePortResolver, dnsResolver);
     }
 
     @Test
@@ -89,17 +90,8 @@ public void testConnect() throws Exception {
         final InetAddress ip2 = InetAddress.getByAddress(new byte[] {127, 0, 0, 2});
 
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 });
-        Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
-        Mockito.when(plainSocketFactory.connectSocket(
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any())).thenReturn(socket);
 
         final SocketConfig socketConfig = SocketConfig.custom()
             .setSoKeepAlive(true)
@@ -108,29 +100,56 @@ public void testConnect() throws Exception {
             .setTcpNoDelay(true)
             .setSoLinger(50, TimeUnit.MILLISECONDS)
             .build();
-        final TlsConfig tlsConfig = TlsConfig.custom()
-                .build();
         final InetSocketAddress localAddress = new InetSocketAddress(local, 0);
-        connectionOperator.connect(conn, host, localAddress,
+        connectionOperator.connect(conn, host, localAddress, Timeout.ofMilliseconds(123), socketConfig, null, context);
-                Timeout.ofMilliseconds(123), socketConfig, tlsConfig, context);
 
         Mockito.verify(socket).setKeepAlive(true);
         Mockito.verify(socket).setReuseAddress(true);
         Mockito.verify(socket).setSoTimeout(5000);
         Mockito.verify(socket).setSoLinger(true, 50);
         Mockito.verify(socket).setTcpNoDelay(true);
+        Mockito.verify(socket).bind(localAddress);
 
-        Mockito.verify(plainSocketFactory).connectSocket(
+        Mockito.verify(socket).connect(new InetSocketAddress(ip1, 80), 123);
-                socket,
-                host,
-                new InetSocketAddress(ip1, 80),
-                localAddress,
-                Timeout.ofMilliseconds(123),
-                tlsConfig,
-                context);
         Mockito.verify(conn, Mockito.times(2)).bind(socket);
     }
 
+    @Test
+    public void testConnectWithTLSUpgrade() throws Exception {
+        final HttpContext context = new BasicHttpContext();
+        final HttpHost host = new HttpHost("https", "somehost");
+        final InetAddress local = InetAddress.getByAddress(new byte[] {127, 0, 0, 0});
+        final InetAddress ip1 = InetAddress.getByAddress(new byte[] {127, 0, 0, 1});
+        final InetAddress ip2 = InetAddress.getByAddress(new byte[] {127, 0, 0, 2});
+
+        final TlsConfig tlsConfig = TlsConfig.custom()
+                .setHandshakeTimeout(Timeout.ofMilliseconds(345))
+                .setVersionPolicy(HttpVersionPolicy.FORCE_HTTP_1)
+                .build();
+
+        Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 });
+        Mockito.when(schemePortResolver.resolve(host)).thenReturn(443);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
+
+        Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory);
+        final Socket upgradedSocket = Mockito.mock(Socket.class);
+        Mockito.when(sslSocketFactory.createLayeredSocket(
+                Mockito.same(socket),
+                Mockito.eq("somehost"),
+                Mockito.eq(443),
+                Mockito.any(),
+                Mockito.any())).thenReturn(upgradedSocket);
+
+        final InetSocketAddress localAddress = new InetSocketAddress(local, 0);
+        connectionOperator.connect(conn, host, localAddress,
+                Timeout.ofMilliseconds(123), SocketConfig.DEFAULT, tlsConfig, context);
+
+        Mockito.verify(socket).connect(new InetSocketAddress(ip1, 443), 123);
+        Mockito.verify(conn, Mockito.times(2)).bind(socket);
+        Mockito.verify(sslSocketFactory).createLayeredSocket(socket, "somehost", 443, tlsConfig, context);
+        Mockito.verify(conn, Mockito.times(1)).bind(upgradedSocket);
+    }
+
     @Test
     public void testConnectTimeout() throws Exception {
         final HttpContext context = new BasicHttpContext();
@@ -139,17 +158,9 @@ public void testConnectTimeout() throws Exception {
         final InetAddress ip2 = InetAddress.getByAddress(new byte[] {10, 0, 0, 2});
 
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 });
-        Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
-        Mockito.when(plainSocketFactory.connectSocket(
+        Mockito.doThrow(new SocketTimeoutException()).when(socket).connect(Mockito.any(), Mockito.anyInt());
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any())).thenThrow(new SocketTimeoutException());
 
         Assertions.assertThrows(ConnectTimeoutException.class, () ->
                 connectionOperator.connect(
@@ -164,17 +175,9 @@ public void testConnectFailure() throws Exception {
         final InetAddress ip2 = InetAddress.getByAddress(new byte[] {10, 0, 0, 2});
 
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 });
-        Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
-        Mockito.when(plainSocketFactory.connectSocket(
+        Mockito.doThrow(new ConnectException()).when(socket).connect(Mockito.any(), Mockito.anyInt());
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any())).thenThrow(new ConnectException());
 
         Assertions.assertThrows(HttpHostConnectException.class, () ->
                 connectionOperator.connect(
@@ -190,25 +193,11 @@ public void testConnectFailover() throws Exception {
         final InetAddress ip2 = InetAddress.getByAddress(new byte[] {10, 0, 0, 2});
 
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 });
-        Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
-        Mockito.when(plainSocketFactory.connectSocket(
+        Mockito.doThrow(new ConnectException()).when(socket).connect(
-                Mockito.any(),
-                Mockito.any(),
                 Mockito.eq(new InetSocketAddress(ip1, 80)),
-                Mockito.any(),
+                Mockito.anyInt());
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any())).thenThrow(new ConnectException());
-        Mockito.when(plainSocketFactory.connectSocket(
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.eq(new InetSocketAddress(ip2, 80)),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any())).thenReturn(socket);
 
         final InetSocketAddress localAddress = new InetSocketAddress(local, 0);
         final TlsConfig tlsConfig = TlsConfig.custom()
@@ -216,15 +205,10 @@ public void testConnectFailover() throws Exception {
         connectionOperator.connect(conn, host, localAddress,
                 Timeout.ofMilliseconds(123), SocketConfig.DEFAULT, tlsConfig, context);
 
-        Mockito.verify(plainSocketFactory).connectSocket(
+        Mockito.verify(socket, Mockito.times(2)).bind(localAddress);
-                socket,
+        Mockito.verify(socket).connect(new InetSocketAddress(ip2, 80), 123);
-                host,
-                new InetSocketAddress(ip2, 80),
-                localAddress,
-                Timeout.ofMilliseconds(123),
-                tlsConfig,
-                context);
         Mockito.verify(conn, Mockito.times(3)).bind(socket);
+
     }
 
     @Test
@@ -234,17 +218,8 @@ public void testConnectExplicitAddress() throws Exception {
         final InetAddress ip = InetAddress.getByAddress(new byte[] {127, 0, 0, 23});
         final HttpHost host = new HttpHost(ip);
 
-        Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
         Mockito.when(schemePortResolver.resolve(host)).thenReturn(80);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
-        Mockito.when(plainSocketFactory.connectSocket(
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any())).thenReturn(socket);
 
         final InetSocketAddress localAddress = new InetSocketAddress(local, 0);
         final TlsConfig tlsConfig = TlsConfig.custom()
@@ -252,14 +227,8 @@ public void testConnectExplicitAddress() throws Exception {
         connectionOperator.connect(conn, host, localAddress,
                 Timeout.ofMilliseconds(123), SocketConfig.DEFAULT, tlsConfig, context);
 
-        Mockito.verify(plainSocketFactory).connectSocket(
+        Mockito.verify(socket).bind(localAddress);
-                socket,
+        Mockito.verify(socket).connect(new InetSocketAddress(ip, 80), 123);
-                host,
-                new InetSocketAddress(ip, 80),
-                localAddress,
-                Timeout.ofMilliseconds(123),
-                tlsConfig,
-                context);
         Mockito.verify(dnsResolver, Mockito.never()).resolve(Mockito.anyString());
         Mockito.verify(conn, Mockito.times(2)).bind(socket);
     }
@@ -290,7 +259,6 @@ public void testUpgrade() throws Exception {
     public void testUpgradeUpsupportedScheme() throws Exception {
         final HttpContext context = new BasicHttpContext();
         final HttpHost host = new HttpHost("httpsssss", "somehost", -1);
-        Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
 
         Assertions.assertThrows(UnsupportedSchemeException.class, () ->
                 connectionOperator.upgrade(conn, host, context));
@@ -300,7 +268,6 @@ public void testUpgradeUpsupportedScheme() throws Exception {
     public void testUpgradeNonLayeringScheme() throws Exception {
         final HttpContext context = new BasicHttpContext();
         final HttpHost host = new HttpHost("http", "somehost", -1);
-        Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory);
 
         Assertions.assertThrows(UnsupportedSchemeException.class, () ->
                 connectionOperator.upgrade(conn, host, context));

httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java

@@ -40,6 +40,7 @@
 import org.apache.hc.client5.http.config.ConnectionConfig;
 import org.apache.hc.client5.http.config.TlsConfig;
 import org.apache.hc.client5.http.io.ConnectionEndpoint;
+import org.apache.hc.client5.http.io.DetachedSocketFactory;
 import org.apache.hc.client5.http.io.LeaseRequest;
 import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
@@ -62,7 +63,6 @@
 /**
  * {@link PoolingHttpClientConnectionManager} tests.
  */
-@SuppressWarnings({"boxing","static-access","resource"}) // test code
 public class TestPoolingHttpClientConnectionManager {
 
     @Mock
@@ -70,9 +70,9 @@ public class TestPoolingHttpClientConnectionManager {
     @Mock
     private Lookup<ConnectionSocketFactory> socketFactoryRegistry;
     @Mock
-    private ConnectionSocketFactory plainSocketFactory;
+    private DetachedSocketFactory detachedSocketFactory;
     @Mock
-    private ConnectionSocketFactory sslSocketFactory;
+    private LayeredConnectionSocketFactory sslSocketFactory;
     @Mock
     private Socket socket;
     @Mock
@@ -83,13 +83,15 @@ public class TestPoolingHttpClientConnectionManager {
     private Future<PoolEntry<HttpRoute, ManagedHttpClientConnection>> future;
     @Mock
     private StrictConnPool<HttpRoute, ManagedHttpClientConnection> pool;
+
     private PoolingHttpClientConnectionManager mgr;
 
     @BeforeEach
     public void setup() throws Exception {
         MockitoAnnotations.openMocks(this);
-        mgr = new PoolingHttpClientConnectionManager(
+        mgr = new PoolingHttpClientConnectionManager(new DefaultHttpClientConnectionOperator(
-                new DefaultHttpClientConnectionOperator(socketFactoryRegistry, schemePortResolver, dnsResolver), pool, null);
+                detachedSocketFactory, socketFactoryRegistry, schemePortResolver, dnsResolver), pool,
+                null);
     }
 
     @Test
@@ -259,14 +261,13 @@ public void testTargetConnect() throws Exception {
 
         Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[]{remote});
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
-        Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(plainSocketFactory);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
-        Mockito.when(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket);
+
-        Mockito.when(plainSocketFactory.connectSocket(
+        Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory);
-                Mockito.eq(socket),
+        Mockito.when(sslSocketFactory.createLayeredSocket(
-                Mockito.any(),
+                Mockito.same(socket),
-                Mockito.any(),
+                Mockito.eq("somehost"),
-                Mockito.any(),
+                Mockito.eq(8443),
-                Mockito.any(),
                 Mockito.any(),
                 Mockito.any())).thenReturn(socket);
 
@@ -274,29 +275,17 @@ public void testTargetConnect() throws Exception {
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(null, context);
+        Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+        Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 234);
-                socket,
+        Mockito.verify(sslSocketFactory).createLayeredSocket(socket, "somehost", 8443, tlsConfig, context);
-                target,
-                new InetSocketAddress(remote, 8443),
-                new InetSocketAddress(local, 0),
-                Timeout.ofMilliseconds(234),
-                tlsConfig,
-                context);
 
         mgr.connect(endpoint1, TimeValue.ofMilliseconds(123), context);
 
         Mockito.verify(dnsResolver, Mockito.times(2)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(2)).resolve(target);
-        Mockito.verify(plainSocketFactory, Mockito.times(2)).createSocket(null, context);
+        Mockito.verify(detachedSocketFactory, Mockito.times(2)).create(null);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+        Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 123);
-                socket,
+        Mockito.verify(sslSocketFactory, Mockito.times(2)).createLayeredSocket(socket, "somehost", 8443, tlsConfig, context);
-                target,
-                new InetSocketAddress(remote, 8443),
-                new InetSocketAddress(local, 0),
-                Timeout.ofMilliseconds(123),
-                tlsConfig,
-                context);
     }
 
     @Test
@@ -323,9 +312,7 @@ public void testProxyConnectAndUpgrade() throws Exception {
         final ConnectionEndpoint endpoint1 = connRequest1.get(Timeout.ofSeconds(1));
         Assertions.assertNotNull(endpoint1);
 
-        final ConnectionSocketFactory plainsf = Mockito.mock(ConnectionSocketFactory.class);
         final LayeredConnectionSocketFactory sslsf = Mockito.mock(LayeredConnectionSocketFactory.class);
-        final Socket mockSock = Mockito.mock(Socket.class);
         final HttpClientContext context = HttpClientContext.create();
         final SocketConfig sconfig = SocketConfig.custom().build();
 
@@ -343,40 +330,24 @@ public void testProxyConnectAndUpgrade() throws Exception {
         Mockito.when(dnsResolver.resolve("someproxy")).thenReturn(new InetAddress[] {remote});
         Mockito.when(schemePortResolver.resolve(proxy)).thenReturn(8080);
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
-        Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainsf);
         Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslsf);
-        Mockito.when(plainsf.createSocket(Mockito.any(), Mockito.any())).thenReturn(mockSock);
+        Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket);
-        Mockito.when(plainsf.connectSocket(
-                Mockito.eq(mockSock),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any(),
-                Mockito.any())).thenReturn(mockSock);
 
         mgr.connect(endpoint1, null, context);
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("someproxy");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(proxy);
-        Mockito.verify(plainsf, Mockito.times(1)).createSocket(null, context);
+        Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null);
-        Mockito.verify(plainsf, Mockito.times(1)).connectSocket(
+        Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8080), 234);
-                mockSock,
-                proxy,
-                new InetSocketAddress(remote, 8080),
-                new InetSocketAddress(local, 0),
-                Timeout.ofMilliseconds(234),
-                tlsConfig,
-                context);
 
         Mockito.when(conn.isOpen()).thenReturn(true);
-        Mockito.when(conn.getSocket()).thenReturn(mockSock);
+        Mockito.when(conn.getSocket()).thenReturn(socket);
 
         mgr.upgrade(endpoint1, context);
 
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target);
         Mockito.verify(sslsf, Mockito.times(1)).createLayeredSocket(
-                mockSock, "somehost", 8443, tlsConfig, context);
+                socket, "somehost", 8443, tlsConfig, context);
     }
 
 }