From 877aed32ff57639fb5894f920ea7540335f48fb2 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 21 Jan 2024 16:59:38 +0100 Subject: [PATCH 1/4] Move plain socket create and connect operations to HttpClientConnectionOperator --- .../DefaultHttpClientConnectionOperator.java | 103 ++++++------ .../DefaultAsyncClientConnectionOperator.java | 2 +- .../http/io/DetachedSocketFactory.java | 47 ++++++ .../TestBasicHttpClientConnectionManager.java | 70 +++----- .../io/TestHttpClientConnectionOperator.java | 149 +++++++----------- ...estPoolingHttpClientConnectionManager.java | 79 +++------- 6 files changed, 210 insertions(+), 240 deletions(-) create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/io/DetachedSocketFactory.java diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java index bbbf826e15..95b549e7d1 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java +++ b/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 socketFactoryRegistry; private final SchemePortResolver schemePortResolver; private final DnsResolver dnsResolver; public DefaultHttpClientConnectionOperator( + final DetachedSocketFactory detachedSocketFactory, final Lookup socketFactoryRegistry, final SchemePortResolver schemePortResolver, final DnsResolver dnsResolver) { super(); - Args.notNull(socketFactoryRegistry, "Socket factory registry"); - this.socketFactoryRegistry = socketFactoryRegistry; + this.detachedSocketFactory = Args.notNull(detachedSocketFactory, "Plain socket factory"); + this.socketFactoryRegistry = Args.notNull(socketFactoryRegistry, "Socket factory registry"); this.schemePortResolver = schemePortResolver != null ? schemePortResolver : DefaultSchemePortResolver.INSTANCE; this.dnsResolver = dnsResolver != null ? dnsResolver : SystemDefaultDnsResolver.INSTANCE; } - @SuppressWarnings("unchecked") - private Lookup getSocketFactoryRegistry(final HttpContext context) { - Lookup reg = (Lookup) context.getAttribute( - SOCKET_FACTORY_REGISTRY); - if (reg == null) { - reg = this.socketFactoryRegistry; - } - return reg; + public DefaultHttpClientConnectionOperator( + final Lookup socketFactoryRegistry, + final SchemePortResolver schemePortResolver, + final DnsResolver dnsResolver) { + this(PLAIN_SOCKET_FACTORY, socketFactoryRegistry, schemePortResolver, dnsResolver); } @Override @@ -128,11 +136,6 @@ public void connect( Args.notNull(host, "Host"); Args.notNull(socketConfig, "Socket config"); Args.notNull(context, "Context"); - final Lookup 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(sock); + conn.bind(socket); + 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 Lookup registry = getSocketFactoryRegistry(clientContext); - final ConnectionSocketFactory sf = registry.lookup(host.getSchemeName()); + final ConnectionSocketFactory sf = socketFactoryRegistry.lookup(host.getSchemeName()); if (sf == null) { throw new UnsupportedSchemeException(host.getSchemeName() + " protocol is not supported"); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java index abc2fbda30..9571af4e28 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java @@ -95,7 +95,6 @@ public Future connect( final ComplexFuture 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 sessionFuture = sessionRequester.connect( connectionInitiator, @@ -109,6 +108,7 @@ public Future 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(); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/io/DetachedSocketFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/io/DetachedSocketFactory.java new file mode 100644 index 0000000000..553410f9bf --- /dev/null +++ b/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 + * . + * + */ + +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; + +} diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java index 9c1f3dd6bb..c567e09bc1 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java +++ b/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 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( - socketFactoryRegistry, connFactory, schemePortResolver, dnsResolver); + mgr = new BasicHttpClientConnectionManager(new DefaultHttpClientConnectionOperator( + 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(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket); - Mockito.when(plainSocketFactory.connectSocket( - Mockito.eq(socket), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), + Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); + + Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory); + Mockito.when(sslSocketFactory.createLayeredSocket( + Mockito.same(socket), + Mockito.eq("somehost"), + Mockito.eq(8443), 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(plainSocketFactory, Mockito.times(1)).connectSocket( - socket, - target, - new InetSocketAddress(remote, 8443), - new InetSocketAddress(local, 0), - Timeout.ofMilliseconds(234), - tlsConfig, - context); + Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null); + Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 234); + Mockito.verify(sslSocketFactory).createLayeredSocket(socket, "somehost", 8443, 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(plainSocketFactory, Mockito.times(1)).connectSocket( - socket, - target, - new InetSocketAddress(remote, 8443), - new InetSocketAddress(local, 0), - Timeout.ofMilliseconds(123), - tlsConfig, - context); + Mockito.verify(detachedSocketFactory, Mockito.times(2)).create(null); + Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 123); + Mockito.verify(sslSocketFactory, Mockito.times(2)).createLayeredSocket(socket, "somehost", 8443, 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(plainSocketFactory.connectSocket( - Mockito.eq(socket), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any())).thenReturn(socket); + Mockito.when(detachedSocketFactory.create(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(plainSocketFactory, Mockito.times(1)).connectSocket( - socket, - proxy, - new InetSocketAddress(remote, 8080), - new InetSocketAddress(local, 0), - Timeout.ofMilliseconds(234), - tlsConfig, - context); + Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null); + Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8080), 234); Mockito.when(conn.getSocket()).thenReturn(socket); diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java index ca982c7d74..5d2050100b 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java +++ b/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 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(plainSocketFactory.connectSocket( - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any())).thenReturn(socket); + Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); final SocketConfig socketConfig = SocketConfig.custom() .setSoKeepAlive(true) @@ -108,27 +100,54 @@ 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, - Timeout.ofMilliseconds(123), socketConfig, tlsConfig, context); + connectionOperator.connect(conn, host, localAddress, Timeout.ofMilliseconds(123), socketConfig, null, 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(socket).connect(new InetSocketAddress(ip1, 80), 123); + 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(plainSocketFactory).connectSocket( - socket, - host, - new InetSocketAddress(ip1, 80), - localAddress, - Timeout.ofMilliseconds(123), - 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 @@ -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(plainSocketFactory.connectSocket( - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any())).thenThrow(new SocketTimeoutException()); + Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); + Mockito.doThrow(new SocketTimeoutException()).when(socket).connect(Mockito.any(), Mockito.anyInt()); 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(plainSocketFactory.connectSocket( - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any())).thenThrow(new ConnectException()); + Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); + Mockito.doThrow(new ConnectException()).when(socket).connect(Mockito.any(), Mockito.anyInt()); 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(plainSocketFactory.connectSocket( - Mockito.any(), - Mockito.any(), + Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); + Mockito.doThrow(new ConnectException()).when(socket).connect( Mockito.eq(new InetSocketAddress(ip1, 80)), - Mockito.any(), - 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); + Mockito.anyInt()); 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( - socket, - host, - new InetSocketAddress(ip2, 80), - localAddress, - Timeout.ofMilliseconds(123), - tlsConfig, - context); + Mockito.verify(socket, Mockito.times(2)).bind(localAddress); + Mockito.verify(socket).connect(new InetSocketAddress(ip2, 80), 123); 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(plainSocketFactory.connectSocket( - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any())).thenReturn(socket); + Mockito.when(detachedSocketFactory.create(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( - socket, - host, - new InetSocketAddress(ip, 80), - localAddress, - Timeout.ofMilliseconds(123), - tlsConfig, - context); + Mockito.verify(socket).bind(localAddress); + Mockito.verify(socket).connect(new InetSocketAddress(ip, 80), 123); 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)); diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java index 4bcf225606..4f43ce0c1f 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java +++ b/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 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> future; @Mock private StrictConnPool pool; + private PoolingHttpClientConnectionManager mgr; @BeforeEach public void setup() throws Exception { MockitoAnnotations.openMocks(this); - mgr = new PoolingHttpClientConnectionManager( - new DefaultHttpClientConnectionOperator(socketFactoryRegistry, schemePortResolver, dnsResolver), pool, null); + mgr = new PoolingHttpClientConnectionManager(new DefaultHttpClientConnectionOperator( + 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(plainSocketFactory.createSocket(Mockito.any(), Mockito.any())).thenReturn(socket); - Mockito.when(plainSocketFactory.connectSocket( - Mockito.eq(socket), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), + Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); + + Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory); + Mockito.when(sslSocketFactory.createLayeredSocket( + Mockito.same(socket), + Mockito.eq("somehost"), + Mockito.eq(8443), 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(plainSocketFactory, Mockito.times(1)).connectSocket( - socket, - target, - new InetSocketAddress(remote, 8443), - new InetSocketAddress(local, 0), - Timeout.ofMilliseconds(234), - tlsConfig, - context); + Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null); + Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 234); + Mockito.verify(sslSocketFactory).createLayeredSocket(socket, "somehost", 8443, 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(plainSocketFactory, Mockito.times(1)).connectSocket( - socket, - target, - new InetSocketAddress(remote, 8443), - new InetSocketAddress(local, 0), - Timeout.ofMilliseconds(123), - tlsConfig, - context); + Mockito.verify(detachedSocketFactory, Mockito.times(2)).create(null); + Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 123); + Mockito.verify(sslSocketFactory, Mockito.times(2)).createLayeredSocket(socket, "somehost", 8443, 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(plainsf.connectSocket( - Mockito.eq(mockSock), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any(), - Mockito.any())).thenReturn(mockSock); + Mockito.when(detachedSocketFactory.create(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(plainsf, Mockito.times(1)).createSocket(null, context); - Mockito.verify(plainsf, Mockito.times(1)).connectSocket( - mockSock, - proxy, - new InetSocketAddress(remote, 8080), - new InetSocketAddress(local, 0), - Timeout.ofMilliseconds(234), - tlsConfig, - context); + Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null); + Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8080), 234); 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); } } From c8cc5aaba3a6fac4ba9d7463584b4d2203a2a52d Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 21 Jan 2024 18:40:52 +0100 Subject: [PATCH 2/4] Standard client TLS strategy implementations to support upgrade of blocking sockets --- .../http/ssl/AbstractClientTlsStrategy.java | 136 +++++++++++++++++- .../http/ssl/SSLConnectionSocketFactory.java | 80 ++++++++++- .../client5/http/ssl/TlsSessionValidator.java | 122 ---------------- .../client5/http/ssl/TlsSocketStrategy.java | 64 +++++++++ 4 files changed, 273 insertions(+), 129 deletions(-) delete mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/ssl/TlsSessionValidator.java create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/ssl/TlsSocketStrategy.java diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java index 8818afb919..b52e636490 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/AbstractClientTlsStrategy.java @@ -27,8 +27,16 @@ package org.apache.hc.client5.http.ssl; +import java.io.IOException; +import java.net.Socket; import java.net.SocketAddress; +import java.security.cert.Certificate; +import java.security.cert.X509Certificate; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Objects; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; @@ -36,7 +44,10 @@ import javax.net.ssl.SSLException; import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; +import javax.security.auth.x500.X500Principal; import org.apache.hc.client5.http.config.TlsConfig; import org.apache.hc.core5.annotation.Contract; @@ -44,6 +55,7 @@ import org.apache.hc.core5.concurrent.FutureCallback; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.nio.ssl.TlsStrategy; +import org.apache.hc.core5.http.protocol.HttpContext; import org.apache.hc.core5.http.ssl.TLS; import org.apache.hc.core5.http.ssl.TlsCiphers; import org.apache.hc.core5.http2.HttpVersionPolicy; @@ -59,7 +71,7 @@ import org.slf4j.LoggerFactory; @Contract(threading = ThreadingBehavior.STATELESS) -abstract class AbstractClientTlsStrategy implements TlsStrategy { +abstract class AbstractClientTlsStrategy implements TlsStrategy, TlsSocketStrategy { private static final Logger LOG = LoggerFactory.getLogger(AbstractClientTlsStrategy.class); @@ -68,7 +80,6 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy { private final String[] supportedCipherSuites; private final SSLBufferMode sslBufferManagement; private final HostnameVerifier hostnameVerifier; - private final TlsSessionValidator tlsSessionValidator; AbstractClientTlsStrategy( final SSLContext sslContext, @@ -82,7 +93,6 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy { this.supportedCipherSuites = supportedCipherSuites; this.sslBufferManagement = sslBufferManagement != null ? sslBufferManagement : SSLBufferMode.STATIC; this.hostnameVerifier = hostnameVerifier != null ? hostnameVerifier : HttpsSupport.getDefaultHostnameVerifier(); - this.tlsSessionValidator = new TlsSessionValidator(LOG); } /** @@ -165,10 +175,128 @@ public void upgrade( protected void initializeEngine(final SSLEngine sslEngine) { } + protected void initializeSocket(final SSLSocket socket) { + } + protected void verifySession( final String hostname, final SSLSession sslsession) throws SSLException { - tlsSessionValidator.verifySession(hostname, sslsession, hostnameVerifier); + verifySession(hostname, sslsession, hostnameVerifier); + } + + @Override + public SSLSocket upgrade(final Socket socket, + final String target, + final int port, + final Object attachment, + final HttpContext context) throws IOException { + final SSLSocket upgradedSocket = (SSLSocket) sslContext.getSocketFactory().createSocket( + socket, + target, + port, + true); + executeHandshake(upgradedSocket, target, attachment); + return upgradedSocket; + } + + private void executeHandshake( + final SSLSocket upgradedSocket, + final String target, + final Object attachment) throws IOException { + final TlsConfig tlsConfig = attachment instanceof TlsConfig ? (TlsConfig) attachment : TlsConfig.DEFAULT; + if (supportedProtocols != null) { + upgradedSocket.setEnabledProtocols(supportedProtocols); + } else { + upgradedSocket.setEnabledProtocols((TLS.excludeWeak(upgradedSocket.getEnabledProtocols()))); + } + if (supportedCipherSuites != null) { + upgradedSocket.setEnabledCipherSuites(supportedCipherSuites); + } else { + upgradedSocket.setEnabledCipherSuites(TlsCiphers.excludeWeak(upgradedSocket.getEnabledCipherSuites())); + } + 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()); + } + + void verifySession( + final String hostname, + final SSLSession sslsession, + final HostnameVerifier hostnameVerifier) throws SSLException { + + if (LOG.isDebugEnabled()) { + LOG.debug("Secure session established"); + LOG.debug(" negotiated protocol: {}", sslsession.getProtocol()); + LOG.debug(" negotiated cipher suite: {}", sslsession.getCipherSuite()); + + try { + + final Certificate[] certs = sslsession.getPeerCertificates(); + final Certificate cert = certs[0]; + if (cert instanceof X509Certificate) { + final X509Certificate x509 = (X509Certificate) cert; + final X500Principal peer = x509.getSubjectX500Principal(); + + LOG.debug(" peer principal: {}", peer); + final Collection> altNames1 = x509.getSubjectAlternativeNames(); + if (altNames1 != null) { + final List altNames = new ArrayList<>(); + for (final List aC : altNames1) { + if (!aC.isEmpty()) { + altNames.add(Objects.toString(aC.get(1), null)); + } + } + LOG.debug(" peer alternative names: {}", altNames); + } + + final X500Principal issuer = x509.getIssuerX500Principal(); + LOG.debug(" issuer principal: {}", issuer); + final Collection> altNames2 = x509.getIssuerAlternativeNames(); + if (altNames2 != null) { + final List altNames = new ArrayList<>(); + for (final List aC : altNames2) { + if (!aC.isEmpty()) { + altNames.add(Objects.toString(aC.get(1), null)); + } + } + LOG.debug(" issuer alternative names: {}", altNames); + } + } + } catch (final Exception ignore) { + } + } + + if (hostnameVerifier != null) { + final Certificate[] certs = sslsession.getPeerCertificates(); + if (certs.length < 1) { + throw new SSLPeerUnverifiedException("Peer certificate chain is empty"); + } + final Certificate peerCertificate = certs[0]; + final X509Certificate x509Certificate; + if (peerCertificate instanceof X509Certificate) { + x509Certificate = (X509Certificate) peerCertificate; + } else { + throw new SSLPeerUnverifiedException("Unexpected certificate type: " + peerCertificate.getType()); + } + if (hostnameVerifier instanceof HttpClientHostnameVerifier) { + ((HttpClientHostnameVerifier) hostnameVerifier).verify(hostname, x509Certificate); + } else if (!hostnameVerifier.verify(hostname, sslsession)) { + final List subjectAlts = DefaultHostnameVerifier.getSubjectAltNames(x509Certificate); + throw new SSLPeerUnverifiedException("Certificate for <" + hostname + "> doesn't match any " + + "of the subject alternative names: " + subjectAlts); + } + } } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java index 9925c9a806..b8cbde45c7 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java @@ -36,17 +36,24 @@ import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; +import java.security.cert.Certificate; +import java.security.cert.X509Certificate; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.regex.Pattern; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLException; import javax.net.ssl.SSLHandshakeException; +import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocket; +import javax.security.auth.x500.X500Principal; import org.apache.hc.client5.http.config.TlsConfig; import org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory; @@ -128,7 +135,6 @@ static boolean isWeakCipherSuite(final String cipherSuite) { private final HostnameVerifier hostnameVerifier; private final String[] supportedProtocols; private final String[] supportedCipherSuites; - private final TlsSessionValidator tlsSessionValidator; public SSLConnectionSocketFactory(final SSLContext sslContext) { this(sslContext, HttpsSupport.getDefaultHostnameVerifier()); @@ -176,7 +182,6 @@ public SSLConnectionSocketFactory( this.supportedProtocols = supportedProtocols; this.supportedCipherSuites = supportedCipherSuites; this.hostnameVerifier = hostnameVerifier != null ? hostnameVerifier : HttpsSupport.getDefaultHostnameVerifier(); - this.tlsSessionValidator = new TlsSessionValidator(LOG); } /** @@ -379,7 +384,76 @@ private void verifyHostname(final SSLSocket sslsock, final String hostname) thro protected void verifySession( final String hostname, final SSLSession sslSession) throws SSLException { - tlsSessionValidator.verifySession(hostname, sslSession, hostnameVerifier); + verifySession(hostname, sslSession, hostnameVerifier); + } + + void verifySession( + final String hostname, + final SSLSession sslsession, + final HostnameVerifier hostnameVerifier) throws SSLException { + + if (LOG.isDebugEnabled()) { + LOG.debug("Secure session established"); + LOG.debug(" negotiated protocol: {}", sslsession.getProtocol()); + LOG.debug(" negotiated cipher suite: {}", sslsession.getCipherSuite()); + + try { + + final Certificate[] certs = sslsession.getPeerCertificates(); + final Certificate cert = certs[0]; + if (cert instanceof X509Certificate) { + final X509Certificate x509 = (X509Certificate) cert; + final X500Principal peer = x509.getSubjectX500Principal(); + + LOG.debug(" peer principal: {}", peer); + final Collection> altNames1 = x509.getSubjectAlternativeNames(); + if (altNames1 != null) { + final List altNames = new ArrayList<>(); + for (final List aC : altNames1) { + if (!aC.isEmpty()) { + altNames.add(Objects.toString(aC.get(1), null)); + } + } + LOG.debug(" peer alternative names: {}", altNames); + } + + final X500Principal issuer = x509.getIssuerX500Principal(); + LOG.debug(" issuer principal: {}", issuer); + final Collection> altNames2 = x509.getIssuerAlternativeNames(); + if (altNames2 != null) { + final List altNames = new ArrayList<>(); + for (final List aC : altNames2) { + if (!aC.isEmpty()) { + altNames.add(Objects.toString(aC.get(1), null)); + } + } + LOG.debug(" issuer alternative names: {}", altNames); + } + } + } catch (final Exception ignore) { + } + } + + if (hostnameVerifier != null) { + final Certificate[] certs = sslsession.getPeerCertificates(); + if (certs.length < 1) { + throw new SSLPeerUnverifiedException("Peer certificate chain is empty"); + } + final Certificate peerCertificate = certs[0]; + final X509Certificate x509Certificate; + if (peerCertificate instanceof X509Certificate) { + x509Certificate = (X509Certificate) peerCertificate; + } else { + throw new SSLPeerUnverifiedException("Unexpected certificate type: " + peerCertificate.getType()); + } + if (hostnameVerifier instanceof HttpClientHostnameVerifier) { + ((HttpClientHostnameVerifier) hostnameVerifier).verify(hostname, x509Certificate); + } else if (!hostnameVerifier.verify(hostname, sslsession)) { + final List subjectAlts = DefaultHostnameVerifier.getSubjectAltNames(x509Certificate); + throw new SSLPeerUnverifiedException("Certificate for <" + hostname + "> doesn't match any " + + "of the subject alternative names: " + subjectAlts); + } + } } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/TlsSessionValidator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/TlsSessionValidator.java deleted file mode 100644 index 76541ff468..0000000000 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/TlsSessionValidator.java +++ /dev/null @@ -1,122 +0,0 @@ -/* - * ==================================================================== - * 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 - * . - * - */ - -package org.apache.hc.client5.http.ssl; - -import java.security.cert.Certificate; -import java.security.cert.X509Certificate; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Objects; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLException; -import javax.net.ssl.SSLPeerUnverifiedException; -import javax.net.ssl.SSLSession; -import javax.security.auth.x500.X500Principal; - -import org.slf4j.Logger; - -final class TlsSessionValidator { - - private final Logger log; - - TlsSessionValidator(final Logger log) { - this.log = log; - } - - void verifySession( - final String hostname, - final SSLSession sslsession, - final HostnameVerifier hostnameVerifier) throws SSLException { - - if (log.isDebugEnabled()) { - log.debug("Secure session established"); - log.debug(" negotiated protocol: {}", sslsession.getProtocol()); - log.debug(" negotiated cipher suite: {}", sslsession.getCipherSuite()); - - try { - - final Certificate[] certs = sslsession.getPeerCertificates(); - final Certificate cert = certs[0]; - if (cert instanceof X509Certificate) { - final X509Certificate x509 = (X509Certificate) cert; - final X500Principal peer = x509.getSubjectX500Principal(); - - log.debug(" peer principal: {}", peer); - final Collection> altNames1 = x509.getSubjectAlternativeNames(); - if (altNames1 != null) { - final List altNames = new ArrayList<>(); - for (final List aC : altNames1) { - if (!aC.isEmpty()) { - altNames.add(Objects.toString(aC.get(1), null)); - } - } - log.debug(" peer alternative names: {}", altNames); - } - - final X500Principal issuer = x509.getIssuerX500Principal(); - log.debug(" issuer principal: {}", issuer); - final Collection> altNames2 = x509.getIssuerAlternativeNames(); - if (altNames2 != null) { - final List altNames = new ArrayList<>(); - for (final List aC : altNames2) { - if (!aC.isEmpty()) { - altNames.add(Objects.toString(aC.get(1), null)); - } - } - log.debug(" issuer alternative names: {}", altNames); - } - } - } catch (final Exception ignore) { - } - } - - if (hostnameVerifier != null) { - final Certificate[] certs = sslsession.getPeerCertificates(); - if (certs.length < 1) { - throw new SSLPeerUnverifiedException("Peer certificate chain is empty"); - } - final Certificate peerCertificate = certs[0]; - final X509Certificate x509Certificate; - if (peerCertificate instanceof X509Certificate) { - x509Certificate = (X509Certificate) peerCertificate; - } else { - throw new SSLPeerUnverifiedException("Unexpected certificate type: " + peerCertificate.getType()); - } - if (hostnameVerifier instanceof HttpClientHostnameVerifier) { - ((HttpClientHostnameVerifier) hostnameVerifier).verify(hostname, x509Certificate); - } else if (!hostnameVerifier.verify(hostname, sslsession)) { - final List subjectAlts = DefaultHostnameVerifier.getSubjectAltNames(x509Certificate); - throw new SSLPeerUnverifiedException("Certificate for <" + hostname + "> doesn't match any " + - "of the subject alternative names: " + subjectAlts); - } - } - } - -} diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/TlsSocketStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/TlsSocketStrategy.java new file mode 100644 index 0000000000..97af381f67 --- /dev/null +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/TlsSocketStrategy.java @@ -0,0 +1,64 @@ +/* + * ==================================================================== + * 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 + * . + * + */ + +package org.apache.hc.client5.http.ssl; + +import java.io.IOException; +import java.net.Socket; + +import javax.net.ssl.SSLSocket; + +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.ThreadingBehavior; +import org.apache.hc.core5.http.protocol.HttpContext; + +/** + * TLS protocol upgrade strategy for blocking {@link Socket}s. + * + * @since 5.4 + */ +@Contract(threading = ThreadingBehavior.STATELESS) +public interface TlsSocketStrategy { + + /** + * Upgrades the given plain socket and executes the TLS handshake over it. + * + * @param socket the existing plain socket + * @param target the name of the target host. + * @param port the port to connect to on the target host. + * @param context the actual HTTP context. + * @param attachment connect request attachment. + * @return socket upgraded to TLS. + */ + SSLSocket upgrade( + Socket socket, + String target, + int port, + Object attachment, + HttpContext context) throws IOException; + +} From e4ac7c2740fb276512e4171906b8b44614d3dc46 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 22 Jan 2024 09:33:59 +0100 Subject: [PATCH 3/4] Deprecated ConnectionSocketFactory, LayeredConnectionSocketFactory and their plain and SSL implementations in favor of DefaultClientTlsStrategy --- .../CachingHttpClientCompatibilityTest.java | 4 +- .../external/HttpClientCompatibilityTest.java | 4 +- ...java => TestDefaultClientTlsStrategy.java} | 164 +++++++----------- .../sync/extension/TestClientResources.java | 6 +- .../http/impl/async/H2AsyncClientBuilder.java | 4 +- .../http/impl/async/HttpAsyncClients.java | 2 +- .../io/BasicHttpClientConnectionManager.java | 85 ++++++--- .../DefaultHttpClientConnectionOperator.java | 71 +++++--- .../PoolingHttpClientConnectionManager.java | 53 ++++-- ...ingHttpClientConnectionManagerBuilder.java | 46 +++-- .../PoolingAsyncClientConnectionManager.java | 2 +- ...ngAsyncClientConnectionManagerBuilder.java | 4 +- .../http/socket/ConnectionSocketFactory.java | 3 +- .../LayeredConnectionSocketFactory.java | 3 +- .../socket/PlainConnectionSocketFactory.java | 7 +- .../http/ssl/DefaultClientTlsStrategy.java | 26 ++- .../http/ssl/SSLConnectionSocketFactory.java | 6 +- .../SSLConnectionSocketFactoryBuilder.java | 3 +- .../http/examples/ClientConfiguration.java | 7 +- .../ClientCustomPublicSuffixList.java | 7 +- .../http/examples/ClientCustomSSL.java | 10 +- .../TestBasicHttpClientConnectionManager.java | 27 +-- .../io/TestHttpClientConnectionOperator.java | 34 ++-- ...estPoolingHttpClientConnectionManager.java | 28 +-- .../http/ssl/TestSSLSocketFactory.java | 77 -------- 25 files changed, 349 insertions(+), 334 deletions(-) rename httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/{TestSSLSocketFactory.java => TestDefaultClientTlsStrategy.java} (60%) delete mode 100644 httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestSSLSocketFactory.java diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/CachingHttpClientCompatibilityTest.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/CachingHttpClientCompatibilityTest.java index 2c0eba70e1..59a7e8af81 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/CachingHttpClientCompatibilityTest.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/CachingHttpClientCompatibilityTest.java @@ -40,7 +40,7 @@ import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; @@ -76,7 +76,7 @@ public static void main(final String... args) throws Exception { final SSLContext sslContext = SSLContexts.custom() .loadTrustMaterial(getClass().getResource("/test-ca.keystore"), "nopassword".toCharArray()).build(); this.connManager = PoolingHttpClientConnectionManagerBuilder.create() - .setSSLSocketFactory(new SSLConnectionSocketFactory(sslContext)) + .setTlsSocketStrategy(new DefaultClientTlsStrategy(sslContext)) .build(); this.client = CachingHttpClients.custom() .setCacheConfig(CacheConfig.custom() diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/HttpClientCompatibilityTest.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/HttpClientCompatibilityTest.java index d8fd4f760e..bd8c7f4a66 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/HttpClientCompatibilityTest.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/external/HttpClientCompatibilityTest.java @@ -42,7 +42,7 @@ import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.protocol.HttpClientContext; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.HeaderElements; import org.apache.hc.core5.http.HttpHeaders; @@ -102,7 +102,7 @@ public static void main(final String... args) throws Exception { final SSLContext sslContext = SSLContexts.custom() .loadTrustMaterial(getClass().getResource("/test-ca.keystore"), "nopassword".toCharArray()).build(); this.connManager = PoolingHttpClientConnectionManagerBuilder.create() - .setSSLSocketFactory(new SSLConnectionSocketFactory(sslContext)) + .setTlsSocketStrategy(new DefaultClientTlsStrategy(sslContext)) .build(); this.client = HttpClients.custom() .setConnectionManager(this.connManager) diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestSSLSocketFactory.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestDefaultClientTlsStrategy.java similarity index 60% rename from httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestSSLSocketFactory.java rename to httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestDefaultClientTlsStrategy.java index 0a83b6ab50..01d80a2758 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestSSLSocketFactory.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestDefaultClientTlsStrategy.java @@ -30,12 +30,10 @@ import static org.hamcrest.MatcherAssert.assertThat; import java.io.IOException; -import java.net.InetSocketAddress; 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; @@ -43,9 +41,9 @@ import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocket; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.client5.http.ssl.NoopHostnameVerifier; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactoryBuilder; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.client5.testing.SSLTestContexts; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.impl.bootstrap.HttpServer; @@ -55,17 +53,15 @@ 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; import org.junit.jupiter.api.Test; /** - * Unit tests for {@link SSLConnectionSocketFactory}. + * Unit tests for {@link DefaultClientTlsStrategy}. */ -public class TestSSLSocketFactory { +public class TestDefaultClientTlsStrategy { private HttpServer server; @@ -103,16 +99,14 @@ public void testBasicSSL() throws Exception { final HttpContext context = new BasicHttpContext(); final TestX509HostnameVerifier hostVerifier = new TestX509HostnameVerifier(); - final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory( + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy( SSLTestContexts.createClientSSLContext(), hostVerifier); - 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, + final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort()); + try (final Socket socket = new Socket(target.getHostName(), target.getPort())) { + try (final SSLSocket sslSocket = tlsStrategy.upgrade( socket, - target, - remoteAddress, + target.getHostName(), + target.getPort(), null, context)) { final SSLSession sslsession = sslSocket.getSession(); @@ -123,44 +117,6 @@ public void testBasicSSL() throws Exception { } } - @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 @@ -171,17 +127,13 @@ public void testBasicDefaultHostnameVerifier() throws Exception { this.server.start(); final HttpContext context = new BasicHttpContext(); - final SSLConnectionSocketFactory socketFactory = SSLConnectionSocketFactoryBuilder.create() - .setSslContext(SSLTestContexts.createClientSSLContext()) - .build(); - 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, + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext()); + final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort()); + try (final Socket socket = new Socket(target.getHostName(), target.getPort())) { + try (final SSLSocket sslSocket = tlsStrategy.upgrade( socket, - target, - remoteAddress, + target.getHostName(), + target.getPort(), null, context)) { final SSLSession sslsession = sslSocket.getSession(); @@ -202,16 +154,14 @@ public void testClientAuthSSL() throws Exception { final HttpContext context = new BasicHttpContext(); final TestX509HostnameVerifier hostVerifier = new TestX509HostnameVerifier(); - final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory( + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy( SSLTestContexts.createClientSSLContext(), hostVerifier); - 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, + final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort()); + try (final Socket socket = new Socket(target.getHostName(), target.getPort())) { + try (final SSLSocket sslSocket = tlsStrategy.upgrade( socket, - target, - remoteAddress, + target.getHostName(), + target.getPort(), null, context)) { final SSLSession sslsession = sslSocket.getSession(); @@ -234,16 +184,15 @@ public void testClientAuthSSLFailure() throws Exception { final HttpContext context = new BasicHttpContext(); final TestX509HostnameVerifier hostVerifier = new TestX509HostnameVerifier(); - final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory( + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy( SSLTestContexts.createClientSSLContext(), hostVerifier); - 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()); + final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort()); + try (final Socket socket = new Socket(target.getHostName(), target.getPort())) { Assertions.assertThrows(IOException.class, () -> { - try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket( - TimeValue.ZERO_MILLISECONDS, - socket, target, - remoteAddress, + try (final SSLSocket sslSocket = tlsStrategy.upgrade( + socket, + target.getHostName(), + target.getPort(), null, context)) { final SSLSession sslsession = sslSocket.getSession(); @@ -269,15 +218,14 @@ public void testSSLTrustVerification() throws Exception { // Use default SSL context final SSLContext defaultSslContext = SSLContexts.createDefault(); - final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(defaultSslContext, + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(defaultSslContext, NoopHostnameVerifier.INSTANCE); - 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()); + final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort()); + try (final Socket socket = new Socket(target.getHostName(), target.getPort())) { Assertions.assertThrows(SSLException.class, () -> { - try (final SSLSocket sslSocket = (SSLSocket) socketFactory.connectSocket( - TimeValue.ZERO_MILLISECONDS, socket, target, remoteAddress, null, context)) { + try (final SSLSocket sslSocket = tlsStrategy.upgrade( + socket, target.getHostName(), target.getPort(), null, context)) { // empty for now } }); @@ -306,14 +254,17 @@ private void testSSLTrustVerificationOverride(final TrustStrategy trustStrategy) .loadTrustMaterial(null, trustStrategy) .build(); // @formatter:on - final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(sslContext, + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(sslContext, NoopHostnameVerifier.INSTANCE); - 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 HttpHost target = new HttpHost("https", "localhost", server.getLocalPort()); + try (final Socket socket = new Socket(target.getHostName(), target.getPort())) { + try (final SSLSocket sslSocket = tlsStrategy.upgrade( + socket, + target.getHostName(), + target.getPort(), + null, + context)) { // empty for now } } @@ -330,14 +281,17 @@ public void testSSLDisabledByDefault() throws Exception { this.server.start(); final HttpContext context = new BasicHttpContext(); - final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory( + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy( SSLTestContexts.createClientSSLContext()); - 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()); + final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort()); + try (final Socket socket = new Socket(target.getHostName(), target.getPort())) { Assertions.assertThrows(IOException.class, () -> - socketFactory.connectSocket( - TimeValue.ZERO_MILLISECONDS, socket, target, remoteAddress, null, context)); + tlsStrategy.upgrade( + socket, + target.getHostName(), + target.getPort(), + null, + context)); } } @@ -379,12 +333,16 @@ private void testWeakCipherDisabledByDefault(final String cipherSuite) throws Ex this.server.start(); final HttpContext context = new BasicHttpContext(); - final SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory( + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy( SSLTestContexts.createClientSSLContext()); - 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()); - socketFactory.connectSocket(TimeValue.ZERO_MILLISECONDS, socket, target, remoteAddress, null, context); + final HttpHost target = new HttpHost("https", "localhost", server.getLocalPort()); + try (final Socket socket = new Socket(target.getHostName(), target.getPort())) { + tlsStrategy.upgrade( + socket, + target.getHostName(), + target.getPort(), + null, + context); } } } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/extension/TestClientResources.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/extension/TestClientResources.java index fd79bed3ed..fd8f0000ae 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/extension/TestClientResources.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/extension/TestClientResources.java @@ -37,7 +37,7 @@ import org.apache.hc.client5.http.impl.classic.MinimalHttpClient; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.client5.testing.SSLTestContexts; import org.apache.hc.core5.function.Decorator; import org.apache.hc.core5.http.HttpHost; @@ -124,7 +124,7 @@ public CloseableHttpClient startClient( Assertions.assertNull(client); final PoolingHttpClientConnectionManagerBuilder connManagerBuilder = PoolingHttpClientConnectionManagerBuilder.create(); - connManagerBuilder.setSSLSocketFactory(new SSLConnectionSocketFactory(SSLTestContexts.createClientSSLContext())); + connManagerBuilder.setTlsSocketStrategy(new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext())); connManagerBuilder.setDefaultSocketConfig(SocketConfig.custom() .setSoTimeout(timeout) .build()); @@ -147,7 +147,7 @@ public MinimalHttpClient startMinimalClient() throws Exception { Assertions.assertNull(client); final PoolingHttpClientConnectionManagerBuilder connManagerBuilder = PoolingHttpClientConnectionManagerBuilder.create(); - connManagerBuilder.setSSLSocketFactory(new SSLConnectionSocketFactory(SSLTestContexts.createClientSSLContext())); + connManagerBuilder.setTlsSocketStrategy(new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext())); connManagerBuilder.setDefaultSocketConfig(SocketConfig.custom() .setSoTimeout(timeout) .build()); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java index 5b0e7e9566..56f2265d25 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java @@ -843,9 +843,9 @@ public CloseableHttpAsyncClient build() { TlsStrategy tlsStrategyCopy = this.tlsStrategy; if (tlsStrategyCopy == null) { if (systemProperties) { - tlsStrategyCopy = DefaultClientTlsStrategy.getSystemDefault(); + tlsStrategyCopy = DefaultClientTlsStrategy.createSystemDefault(); } else { - tlsStrategyCopy = DefaultClientTlsStrategy.getDefault(); + tlsStrategyCopy = DefaultClientTlsStrategy.createDefault(); } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java index cfe292dd1b..84b7b5536d 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java @@ -314,7 +314,7 @@ public static MinimalH2AsyncClient createHttp2Minimal( public static MinimalH2AsyncClient createHttp2Minimal( final H2Config h2Config, final IOReactorConfig ioReactorConfig) { - return createHttp2Minimal(h2Config, ioReactorConfig, DefaultClientTlsStrategy.getDefault()); + return createHttp2Minimal(h2Config, ioReactorConfig, DefaultClientTlsStrategy.createDefault()); } /** diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java index c2adfb7670..52155a4292 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java @@ -49,9 +49,8 @@ import org.apache.hc.client5.http.io.HttpClientConnectionOperator; import org.apache.hc.client5.http.io.LeaseRequest; import org.apache.hc.client5.http.io.ManagedHttpClientConnection; -import org.apache.hc.client5.http.socket.ConnectionSocketFactory; -import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.ClassicHttpRequest; @@ -60,7 +59,6 @@ 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.config.Registry; import org.apache.hc.core5.http.config.RegistryBuilder; import org.apache.hc.core5.http.impl.io.HttpRequestExecutor; import org.apache.hc.core5.http.io.HttpConnectionFactory; @@ -119,22 +117,6 @@ public class BasicHttpClientConnectionManager implements HttpClientConnectionMan private final AtomicBoolean closed; - private static Registry getDefaultRegistry() { - return RegistryBuilder.create() - .register(URIScheme.HTTP.id, PlainConnectionSocketFactory.getSocketFactory()) - .register(URIScheme.HTTPS.id, SSLConnectionSocketFactory.getSocketFactory()) - .build(); - } - - public BasicHttpClientConnectionManager( - final Lookup socketFactoryRegistry, - final HttpConnectionFactory connFactory, - final SchemePortResolver schemePortResolver, - final DnsResolver dnsResolver) { - this(new DefaultHttpClientConnectionOperator( - socketFactoryRegistry, schemePortResolver, dnsResolver), connFactory); - } - /** * @since 4.4 */ @@ -153,19 +135,76 @@ public BasicHttpClientConnectionManager( this.lock = new ReentrantLock(); } + /** + * @since 5.4 + */ + public static BasicHttpClientConnectionManager create( + final SchemePortResolver schemePortResolver, + final DnsResolver dnsResolver, + final Lookup tlsSocketStrategyRegistry, + final HttpConnectionFactory connFactory) { + return new BasicHttpClientConnectionManager( + new DefaultHttpClientConnectionOperator(schemePortResolver, dnsResolver, tlsSocketStrategyRegistry), + connFactory); + } + + /** + * @since 5.4 + */ + public static BasicHttpClientConnectionManager create( + final Lookup tlsSocketStrategyRegistry, + final HttpConnectionFactory connFactory) { + return new BasicHttpClientConnectionManager( + new DefaultHttpClientConnectionOperator(null, null, tlsSocketStrategyRegistry), connFactory); + } + + /** + * @since 5.4 + */ + public static BasicHttpClientConnectionManager create( + final Lookup tlsSocketStrategyRegistry) { + return new BasicHttpClientConnectionManager( + new DefaultHttpClientConnectionOperator(null, null, tlsSocketStrategyRegistry), null); + } + + /** + * @deprecated Use {@link #create(SchemePortResolver, DnsResolver, Lookup, HttpConnectionFactory)} + */ + @Deprecated public BasicHttpClientConnectionManager( - final Lookup socketFactoryRegistry, + final Lookup socketFactoryRegistry, + final HttpConnectionFactory connFactory, + final SchemePortResolver schemePortResolver, + final DnsResolver dnsResolver) { + this(new DefaultHttpClientConnectionOperator( + socketFactoryRegistry, schemePortResolver, dnsResolver), connFactory); + } + + /** + * @deprecated Use {@link #create(Lookup, HttpConnectionFactory)} + */ + @Deprecated + public BasicHttpClientConnectionManager( + final Lookup socketFactoryRegistry, final HttpConnectionFactory connFactory) { this(socketFactoryRegistry, connFactory, null, null); } + /** + * @deprecated Use {@link #create(Lookup)} + */ + @Deprecated public BasicHttpClientConnectionManager( - final Lookup socketFactoryRegistry) { + final Lookup socketFactoryRegistry) { this(socketFactoryRegistry, null, null, null); } public BasicHttpClientConnectionManager() { - this(getDefaultRegistry(), null, null, null); + this(new DefaultHttpClientConnectionOperator(null, null, + RegistryBuilder.create() + .register(URIScheme.HTTPS.id, DefaultClientTlsStrategy.createDefault()) + .build()), + null); } @Override diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java index 95b549e7d1..e2189d64d3 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java @@ -35,6 +35,8 @@ import java.net.UnknownHostException; import java.util.Arrays; +import javax.net.ssl.SSLSocket; + import org.apache.hc.client5.http.ConnectExceptionSupport; import org.apache.hc.client5.http.DnsResolver; import org.apache.hc.client5.http.SchemePortResolver; @@ -45,8 +47,7 @@ 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.socket.ConnectionSocketFactory; -import org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.Internal; import org.apache.hc.core5.annotation.ThreadingBehavior; @@ -86,29 +87,55 @@ public Socket create(final Proxy socksProxy) throws IOException { }; private final DetachedSocketFactory detachedSocketFactory; - private final Lookup socketFactoryRegistry; + private final Lookup tlsSocketStrategyLookup; private final SchemePortResolver schemePortResolver; private final DnsResolver dnsResolver; + /** + * @deprecated Provided for backward compatibility + */ + @Deprecated + static Lookup adapt(final Lookup lookup) { + + return name -> { + final org.apache.hc.client5.http.socket.ConnectionSocketFactory sf = lookup.lookup(name); + return sf instanceof org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory ? (socket, target, port, attachment, context) -> + (SSLSocket) ((org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory) sf).createLayeredSocket(socket, target, port, attachment, context) : null; + }; + + } + + public DefaultHttpClientConnectionOperator( final DetachedSocketFactory detachedSocketFactory, - final Lookup socketFactoryRegistry, final SchemePortResolver schemePortResolver, - final DnsResolver dnsResolver) { + final DnsResolver dnsResolver, + final Lookup tlsSocketStrategyLookup) { super(); this.detachedSocketFactory = Args.notNull(detachedSocketFactory, "Plain socket factory"); - this.socketFactoryRegistry = Args.notNull(socketFactoryRegistry, "Socket factory registry"); + this.tlsSocketStrategyLookup = Args.notNull(tlsSocketStrategyLookup, "Socket factory registry"); this.schemePortResolver = schemePortResolver != null ? schemePortResolver : - DefaultSchemePortResolver.INSTANCE; + DefaultSchemePortResolver.INSTANCE; this.dnsResolver = dnsResolver != null ? dnsResolver : - SystemDefaultDnsResolver.INSTANCE; + SystemDefaultDnsResolver.INSTANCE; } + /** + * @deprecated Do not use. + */ + @Deprecated public DefaultHttpClientConnectionOperator( - final Lookup socketFactoryRegistry, + final Lookup socketFactoryRegistry, final SchemePortResolver schemePortResolver, final DnsResolver dnsResolver) { - this(PLAIN_SOCKET_FACTORY, socketFactoryRegistry, schemePortResolver, dnsResolver); + this(PLAIN_SOCKET_FACTORY, schemePortResolver, dnsResolver, adapt(socketFactoryRegistry)); + } + + public DefaultHttpClientConnectionOperator( + final SchemePortResolver schemePortResolver, + final DnsResolver dnsResolver, + final Lookup tlsSocketStrategyLookup) { + this(PLAIN_SOCKET_FACTORY, schemePortResolver, dnsResolver, tlsSocketStrategyLookup); } @Override @@ -198,10 +225,9 @@ public void connect( 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); + final TlsSocketStrategy tlsSocketStrategy = tlsSocketStrategyLookup != null ? tlsSocketStrategyLookup.lookup(host.getSchemeName()) : null; + if (tlsSocketStrategy != null && URIScheme.HTTPS.same(host.getSchemeName())) { + final Socket upgradedSocket = tlsSocketStrategy.upgrade(socket, host.getHostName(), port, attachment, context); conn.bind(upgradedSocket); } return; @@ -240,23 +266,18 @@ public void upgrade( final HttpHost host, final Object attachment, final HttpContext context) throws IOException { - final ConnectionSocketFactory sf = socketFactoryRegistry.lookup(host.getSchemeName()); - if (sf == null) { + final TlsSocketStrategy tlsSocketStrategy = tlsSocketStrategyLookup != null ? tlsSocketStrategyLookup.lookup(host.getSchemeName()) : null; + if (tlsSocketStrategy == null) { throw new UnsupportedSchemeException(host.getSchemeName() + " protocol is not supported"); } - if (!(sf instanceof LayeredConnectionSocketFactory)) { - throw new UnsupportedSchemeException(host.getSchemeName() + - " protocol does not support connection upgrade"); - } - final LayeredConnectionSocketFactory lsf = (LayeredConnectionSocketFactory) sf; - Socket sock = conn.getSocket(); - if (sock == null) { + final Socket socket = conn.getSocket(); + if (socket == null) { throw new ConnectionClosedException("Connection is closed"); } final int port = this.schemePortResolver.resolve(host); - sock = lsf.createLayeredSocket(sock, host.getHostName(), port, attachment, context); - conn.bind(sock); + final SSLSocket upgradedSocket = tlsSocketStrategy.upgrade(socket, host.getHostName(), port, attachment, context); + conn.bind(upgradedSocket); } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java index 93b07cf358..8630559c7a 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java @@ -48,9 +48,8 @@ import org.apache.hc.client5.http.io.HttpClientConnectionOperator; import org.apache.hc.client5.http.io.LeaseRequest; import org.apache.hc.client5.http.io.ManagedHttpClientConnection; -import org.apache.hc.client5.http.socket.ConnectionSocketFactory; -import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.Internal; import org.apache.hc.core5.annotation.ThreadingBehavior; @@ -120,41 +119,65 @@ public class PoolingHttpClientConnectionManager private volatile Resolver tlsConfigResolver; public PoolingHttpClientConnectionManager() { - this(RegistryBuilder.create() - .register(URIScheme.HTTP.id, PlainConnectionSocketFactory.getSocketFactory()) - .register(URIScheme.HTTPS.id, SSLConnectionSocketFactory.getSocketFactory()) - .build()); + this(new DefaultHttpClientConnectionOperator(null, null, + RegistryBuilder.create() + .register(URIScheme.HTTPS.id, DefaultClientTlsStrategy.createDefault()) + .build()), + PoolConcurrencyPolicy.STRICT, + PoolReusePolicy.LIFO, + TimeValue.NEG_ONE_MILLISECOND, + null); } + /** + * @deprecated Use {@link PoolingHttpClientConnectionManagerBuilder} + */ + @Deprecated public PoolingHttpClientConnectionManager( - final Registry socketFactoryRegistry) { + final Registry socketFactoryRegistry) { this(socketFactoryRegistry, null); } + /** + * @deprecated Use {@link PoolingHttpClientConnectionManagerBuilder} + */ + @Deprecated public PoolingHttpClientConnectionManager( - final Registry socketFactoryRegistry, + final Registry socketFactoryRegistry, final HttpConnectionFactory connFactory) { this(socketFactoryRegistry, PoolConcurrencyPolicy.STRICT, TimeValue.NEG_ONE_MILLISECOND, connFactory); } + /** + * @deprecated Use {@link PoolingHttpClientConnectionManagerBuilder} + */ + @Deprecated public PoolingHttpClientConnectionManager( - final Registry socketFactoryRegistry, + final Registry socketFactoryRegistry, final PoolConcurrencyPolicy poolConcurrencyPolicy, final TimeValue timeToLive, final HttpConnectionFactory connFactory) { this(socketFactoryRegistry, poolConcurrencyPolicy, PoolReusePolicy.LIFO, timeToLive, connFactory); } + /** + * @deprecated Use {@link PoolingHttpClientConnectionManagerBuilder} + */ + @Deprecated public PoolingHttpClientConnectionManager( - final Registry socketFactoryRegistry, + final Registry socketFactoryRegistry, final PoolConcurrencyPolicy poolConcurrencyPolicy, final PoolReusePolicy poolReusePolicy, final TimeValue timeToLive) { this(socketFactoryRegistry, poolConcurrencyPolicy, poolReusePolicy, timeToLive, null); } + /** + * @deprecated Use {@link PoolingHttpClientConnectionManagerBuilder} + */ + @Deprecated public PoolingHttpClientConnectionManager( - final Registry socketFactoryRegistry, + final Registry socketFactoryRegistry, final PoolConcurrencyPolicy poolConcurrencyPolicy, final PoolReusePolicy poolReusePolicy, final TimeValue timeToLive, @@ -162,8 +185,12 @@ public PoolingHttpClientConnectionManager( this(socketFactoryRegistry, poolConcurrencyPolicy, poolReusePolicy, timeToLive, null, null, connFactory); } + /** + * @deprecated Use {@link PoolingHttpClientConnectionManagerBuilder} + */ + @Deprecated public PoolingHttpClientConnectionManager( - final Registry socketFactoryRegistry, + final Registry socketFactoryRegistry, final PoolConcurrencyPolicy poolConcurrencyPolicy, final PoolReusePolicy poolReusePolicy, final TimeValue timeToLive, diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManagerBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManagerBuilder.java index 799c6ba864..dd5c543e1d 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManagerBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManagerBuilder.java @@ -27,16 +27,16 @@ package org.apache.hc.client5.http.impl.io; +import javax.net.ssl.SSLSocket; + import org.apache.hc.client5.http.DnsResolver; import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.SchemePortResolver; import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.config.TlsConfig; import org.apache.hc.client5.http.io.ManagedHttpClientConnection; -import org.apache.hc.client5.http.socket.ConnectionSocketFactory; -import org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory; -import org.apache.hc.client5.http.socket.PlainConnectionSocketFactory; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.function.Resolver; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.URIScheme; @@ -76,7 +76,7 @@ public class PoolingHttpClientConnectionManagerBuilder { private HttpConnectionFactory connectionFactory; - private LayeredConnectionSocketFactory sslSocketFactory; + private TlsSocketStrategy tlsSocketStrategy; private SchemePortResolver schemePortResolver; private DnsResolver dnsResolver; private PoolConcurrencyPolicy poolConcurrencyPolicy; @@ -108,11 +108,23 @@ public final PoolingHttpClientConnectionManagerBuilder setConnectionFactory( } /** - * Assigns {@link LayeredConnectionSocketFactory} instance. + * Assigns {@link org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory} instance. + * + * @deprecated Use {@link #setTlsSocketStrategy(TlsSocketStrategy)} */ + @Deprecated public final PoolingHttpClientConnectionManagerBuilder setSSLSocketFactory( - final LayeredConnectionSocketFactory sslSocketFactory) { - this.sslSocketFactory = sslSocketFactory; + final org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory sslSocketFactory) { + this.tlsSocketStrategy = (socket, target, port, attachment, context) -> + (SSLSocket) sslSocketFactory.createLayeredSocket(socket, target, port, context); + return this; + } + + /** + * Assigns {@link TlsSocketStrategy} instance. + */ + public final PoolingHttpClientConnectionManagerBuilder setTlsSocketStrategy(final TlsSocketStrategy tlsSocketStrategy) { + this.tlsSocketStrategy = tlsSocketStrategy; return this; } @@ -262,19 +274,17 @@ public final PoolingHttpClientConnectionManagerBuilder useSystemProperties() { } public PoolingHttpClientConnectionManager build() { - @SuppressWarnings("resource") final PoolingHttpClientConnectionManager poolingmgr = new PoolingHttpClientConnectionManager( - RegistryBuilder.create() - .register(URIScheme.HTTP.id, PlainConnectionSocketFactory.getSocketFactory()) - .register(URIScheme.HTTPS.id, sslSocketFactory != null ? sslSocketFactory : - (systemProperties ? - SSLConnectionSocketFactory.getSystemSocketFactory() : - SSLConnectionSocketFactory.getSocketFactory())) - .build(), + final PoolingHttpClientConnectionManager poolingmgr = new PoolingHttpClientConnectionManager( + new DefaultHttpClientConnectionOperator(schemePortResolver, dnsResolver, + RegistryBuilder.create() + .register(URIScheme.HTTPS.id, tlsSocketStrategy != null ? tlsSocketStrategy : + (systemProperties ? + DefaultClientTlsStrategy.createSystemDefault() : + DefaultClientTlsStrategy.createDefault())) + .build()), poolConcurrencyPolicy, poolReusePolicy, null, - schemePortResolver, - dnsResolver, connectionFactory); poolingmgr.setSocketConfigResolver(socketConfigResolver); poolingmgr.setConnectionConfigResolver(connectionConfigResolver); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java index 6ebaf21294..3aa5c4618d 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java @@ -129,7 +129,7 @@ public class PoolingAsyncClientConnectionManager implements AsyncClientConnectio public PoolingAsyncClientConnectionManager() { this(RegistryBuilder.create() - .register(URIScheme.HTTPS.getId(), DefaultClientTlsStrategy.getDefault()) + .register(URIScheme.HTTPS.getId(), DefaultClientTlsStrategy.createDefault()) .build()); } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManagerBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManagerBuilder.java index 6f72c6bb0d..16feeaf7bb 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManagerBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManagerBuilder.java @@ -242,9 +242,9 @@ public PoolingAsyncClientConnectionManager build() { } } else { if (systemProperties) { - tlsStrategyCopy = DefaultClientTlsStrategy.getSystemDefault(); + tlsStrategyCopy = DefaultClientTlsStrategy.createSystemDefault(); } else { - tlsStrategyCopy = DefaultClientTlsStrategy.getDefault(); + tlsStrategyCopy = DefaultClientTlsStrategy.createDefault(); } } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/socket/ConnectionSocketFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/socket/ConnectionSocketFactory.java index 9b0c7b72c5..bffa96661b 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/socket/ConnectionSocketFactory.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/socket/ConnectionSocketFactory.java @@ -43,9 +43,10 @@ /** * A factory for creating and connecting connection sockets. * - * @since 4.3 + * @deprecated Do not use. */ @Contract(threading = ThreadingBehavior.STATELESS) +@Deprecated public interface ConnectionSocketFactory { /** diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/socket/LayeredConnectionSocketFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/socket/LayeredConnectionSocketFactory.java index 36dfa80f82..99624a449c 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/socket/LayeredConnectionSocketFactory.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/socket/LayeredConnectionSocketFactory.java @@ -37,8 +37,9 @@ /** * Extended {@link ConnectionSocketFactory} interface for layered sockets such as SSL/TLS. * - * @since 4.3 + * @deprecated Use {@link org.apache.hc.client5.http.ssl.TlsSocketStrategy}. */ +@Deprecated @Contract(threading = ThreadingBehavior.STATELESS) public interface LayeredConnectionSocketFactory extends ConnectionSocketFactory { diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/socket/PlainConnectionSocketFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/socket/PlainConnectionSocketFactory.java index ce2d8fb407..3396bcaa01 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/socket/PlainConnectionSocketFactory.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/socket/PlainConnectionSocketFactory.java @@ -46,8 +46,13 @@ /** * The default class for creating plain (unencrypted) sockets. * - * @since 4.3 + * @deprecated Use {@link org.apache.hc.client5.http.io.DetachedSocketFactory}. + * Please note this interface is considered internal. + * + * @see org.apache.hc.client5.http.impl.io.DefaultHttpClientConnectionOperator + * @see org.apache.hc.client5.http.io.DetachedSocketFactory */ +@Deprecated @Contract(threading = ThreadingBehavior.STATELESS) public class PlainConnectionSocketFactory implements ConnectionSocketFactory { diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java index 1267071a81..861b4c5f93 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/DefaultClientTlsStrategy.java @@ -48,13 +48,19 @@ @Contract(threading = ThreadingBehavior.STATELESS) public class DefaultClientTlsStrategy extends AbstractClientTlsStrategy { - public static TlsStrategy getDefault() { + /** + * @since 5.4 + */ + public static DefaultClientTlsStrategy createDefault() { return new DefaultClientTlsStrategy( SSLContexts.createDefault(), HttpsSupport.getDefaultHostnameVerifier()); } - public static TlsStrategy getSystemDefault() { + /** + * @since 5.4 + */ + public static DefaultClientTlsStrategy createSystemDefault() { return new DefaultClientTlsStrategy( SSLContexts.createSystemDefault(), HttpsSupport.getSystemProtocols(), @@ -63,6 +69,22 @@ public static TlsStrategy getSystemDefault() { HttpsSupport.getDefaultHostnameVerifier()); } + /** + * @deprecated Use {@link #createDefault()}. + */ + @Deprecated + public static TlsStrategy getDefault() { + return createDefault(); + } + + /** + * @deprecated Use {@link #createSystemDefault()}. + */ + @Deprecated + public static TlsStrategy getSystemDefault() { + return createSystemDefault(); + } + /** * @deprecated To be removed. */ diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java index b8cbde45c7..3ffe908e39 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java @@ -56,7 +56,6 @@ import javax.security.auth.x500.X500Principal; import org.apache.hc.client5.http.config.TlsConfig; -import org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.HttpHost; @@ -79,10 +78,11 @@ * SSLSocketFactory can be used to validate the identity of the HTTPS server against a list of * trusted certificates and to authenticate to the HTTPS server using a private key. * - * @since 4.3 + * @deprecated Use {@link DefaultClientTlsStrategy}. */ +@Deprecated @Contract(threading = ThreadingBehavior.STATELESS) -public class SSLConnectionSocketFactory implements LayeredConnectionSocketFactory { +public class SSLConnectionSocketFactory implements org.apache.hc.client5.http.socket.LayeredConnectionSocketFactory { private static final String WEAK_KEY_EXCHANGES = "^(TLS|SSL)_(NULL|ECDH_anon|DH_anon|DH_anon_EXPORT|DHE_RSA_EXPORT|DHE_DSS_EXPORT|" diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactoryBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactoryBuilder.java index 0926dce4c3..f911227585 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactoryBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactoryBuilder.java @@ -57,8 +57,9 @@ *
  • https.cipherSuites
    • * * - * @since 5.0 + * @deprecated Use {@link DefaultClientTlsStrategy} */ +@Deprecated public class SSLConnectionSocketFactoryBuilder { public static SSLConnectionSocketFactoryBuilder create() { diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientConfiguration.java b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientConfiguration.java index bb20747667..35246ff024 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientConfiguration.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientConfiguration.java @@ -57,7 +57,8 @@ import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.io.ManagedHttpClientConnection; import org.apache.hc.client5.http.protocol.HttpClientContext; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.Header; @@ -150,7 +151,7 @@ public Header parseHeader(final CharArrayBuffer buffer) { // Create a registry of custom connection socket factories for supported // protocol schemes. - final SSLConnectionSocketFactory sslConnectionSocketFactory = new SSLConnectionSocketFactory(sslContext); + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(sslContext); // Use custom DNS resolver to override the system DNS resolution. final DnsResolver dnsResolver = new SystemDefaultDnsResolver() { @@ -168,7 +169,7 @@ public InetAddress[] resolve(final String host) throws UnknownHostException { // Create a connection manager with custom configuration. final PoolingHttpClientConnectionManager connManager = PoolingHttpClientConnectionManagerBuilder.create() - .setSSLSocketFactory(sslConnectionSocketFactory) + .setTlsSocketStrategy(tlsStrategy) .setConnectionFactory(connFactory) .setDnsResolver(dnsResolver) .setPoolConcurrencyPolicy(PoolConcurrencyPolicy.STRICT) diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientCustomPublicSuffixList.java b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientCustomPublicSuffixList.java index 37be073694..3007490f67 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientCustomPublicSuffixList.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientCustomPublicSuffixList.java @@ -38,8 +38,9 @@ import org.apache.hc.client5.http.io.HttpClientConnectionManager; import org.apache.hc.client5.http.psl.PublicSuffixMatcher; import org.apache.hc.client5.http.psl.PublicSuffixMatcherLoader; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.http.config.Lookup; import org.apache.hc.core5.http.config.RegistryBuilder; import org.apache.hc.core5.http.io.entity.EntityUtils; @@ -66,11 +67,11 @@ public static void main(final String[] args) throws Exception { .register(StandardCookieSpec.RELAXED, cookieSpecFactory) .register(StandardCookieSpec.STRICT, cookieSpecFactory) .build(); - final SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory( + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy( SSLContexts.createDefault(), new DefaultHostnameVerifier(publicSuffixMatcher)); final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create() - .setSSLSocketFactory(sslsf) + .setTlsSocketStrategy(tlsStrategy) .build(); try (final CloseableHttpClient httpclient = HttpClients.custom() .setConnectionManager(cm) diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientCustomSSL.java b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientCustomSSL.java index d7b6252037..a130b6848d 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientCustomSSL.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/examples/ClientCustomSSL.java @@ -38,8 +38,8 @@ import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.io.HttpClientConnectionManager; import org.apache.hc.client5.http.protocol.HttpClientContext; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory; -import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactoryBuilder; +import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.http.message.StatusLine; import org.apache.hc.core5.http.ssl.TLS; @@ -67,12 +67,10 @@ public final static void main(final String[] args) throws Exception { return "CN=httpbin.org".equalsIgnoreCase(cert.getSubjectDN().getName()); }) .build(); - final SSLConnectionSocketFactory sslSocketFactory = SSLConnectionSocketFactoryBuilder.create() - .setSslContext(sslContext) - .build(); + final TlsSocketStrategy tlsStrategy = new DefaultClientTlsStrategy(sslContext); // Allow TLSv1.3 protocol only final HttpClientConnectionManager cm = PoolingHttpClientConnectionManagerBuilder.create() - .setSSLSocketFactory(sslSocketFactory) + .setTlsSocketStrategy(tlsStrategy) .setDefaultTlsConfig(TlsConfig.custom() .setHandshakeTimeout(Timeout.ofSeconds(30)) .setSupportedProtocols(TLS.V_1_3) diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java index c567e09bc1..94fd2d38c8 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestBasicHttpClientConnectionManager.java @@ -32,6 +32,8 @@ import java.net.Socket; import java.util.concurrent.TimeUnit; +import javax.net.ssl.SSLSocket; + import org.apache.hc.client5.http.DnsResolver; import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.SchemePortResolver; @@ -42,8 +44,7 @@ import org.apache.hc.client5.http.io.LeaseRequest; 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.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.config.Lookup; import org.apache.hc.core5.http.io.HttpConnectionFactory; @@ -65,14 +66,16 @@ public class TestBasicHttpClientConnectionManager { @Mock private HttpConnectionFactory connFactory; @Mock - private Lookup socketFactoryRegistry; + private Lookup tlsSocketStrategyLookup; @Mock private DetachedSocketFactory detachedSocketFactory; @Mock - private LayeredConnectionSocketFactory sslSocketFactory; + private TlsSocketStrategy tlsSocketStrategy; @Mock private Socket socket; @Mock + private SSLSocket upgradedSocket; + @Mock private SchemePortResolver schemePortResolver; @Mock private DnsResolver dnsResolver; @@ -83,7 +86,7 @@ public class TestBasicHttpClientConnectionManager { public void setup() throws Exception { MockitoAnnotations.openMocks(this); mgr = new BasicHttpClientConnectionManager(new DefaultHttpClientConnectionOperator( - detachedSocketFactory, socketFactoryRegistry, schemePortResolver, dnsResolver), + detachedSocketFactory, schemePortResolver, dnsResolver, tlsSocketStrategyLookup), connFactory); } @@ -385,13 +388,13 @@ public void testTargetConnect() throws Exception { Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443); Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); - Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory); - Mockito.when(sslSocketFactory.createLayeredSocket( + Mockito.when(tlsSocketStrategyLookup.lookup("https")).thenReturn(tlsSocketStrategy); + Mockito.when(tlsSocketStrategy.upgrade( Mockito.same(socket), Mockito.eq("somehost"), Mockito.eq(8443), Mockito.any(), - Mockito.any())).thenReturn(socket); + Mockito.any())).thenReturn(upgradedSocket); mgr.connect(endpoint1, null, context); @@ -399,7 +402,7 @@ public void testTargetConnect() throws Exception { Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target); Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null); Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 234); - Mockito.verify(sslSocketFactory).createLayeredSocket(socket, "somehost", 8443, tlsConfig, context); + Mockito.verify(tlsSocketStrategy).upgrade(socket, "somehost", 8443, tlsConfig, context); mgr.connect(endpoint1, TimeValue.ofMilliseconds(123), context); @@ -407,7 +410,7 @@ public void testTargetConnect() throws Exception { Mockito.verify(schemePortResolver, Mockito.times(2)).resolve(target); Mockito.verify(detachedSocketFactory, Mockito.times(2)).create(null); Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 123); - Mockito.verify(sslSocketFactory, Mockito.times(2)).createLayeredSocket(socket, "somehost", 8443, tlsConfig, context); + Mockito.verify(tlsSocketStrategy, Mockito.times(2)).upgrade(socket, "somehost", 8443, tlsConfig, context); } @Test @@ -441,7 +444,7 @@ 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("https")).thenReturn(sslSocketFactory); + Mockito.when(tlsSocketStrategyLookup.lookup("https")).thenReturn(tlsSocketStrategy); Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); mgr.connect(endpoint1, null, context); @@ -456,7 +459,7 @@ public void testProxyConnectAndUpgrade() throws Exception { mgr.upgrade(endpoint1, context); Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target); - Mockito.verify(sslSocketFactory, Mockito.times(1)).createLayeredSocket( + Mockito.verify(tlsSocketStrategy, Mockito.times(1)).upgrade( socket, "somehost", 8443, tlsConfig, context); } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java index 5d2050100b..fd5f1e600c 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java @@ -34,6 +34,8 @@ import java.net.SocketTimeoutException; import java.util.concurrent.TimeUnit; +import javax.net.ssl.SSLSocket; + import org.apache.hc.client5.http.ConnectTimeoutException; import org.apache.hc.client5.http.DnsResolver; import org.apache.hc.client5.http.HttpHostConnectException; @@ -42,8 +44,7 @@ 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; +import org.apache.hc.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.config.Lookup; import org.apache.hc.core5.http.io.SocketConfig; @@ -62,8 +63,8 @@ public class TestHttpClientConnectionOperator { private ManagedHttpClientConnection conn; private Socket socket; private DetachedSocketFactory detachedSocketFactory; - private LayeredConnectionSocketFactory sslSocketFactory; - private Lookup socketFactoryRegistry; + private TlsSocketStrategy tlsSocketStrategy; + private Lookup tlsSocketStrategyLookup; private SchemePortResolver schemePortResolver; private DnsResolver dnsResolver; private DefaultHttpClientConnectionOperator connectionOperator; @@ -73,12 +74,12 @@ public void setup() throws Exception { conn = Mockito.mock(ManagedHttpClientConnection.class); socket = Mockito.mock(Socket.class); detachedSocketFactory = Mockito.mock(DetachedSocketFactory.class); - sslSocketFactory = Mockito.mock(LayeredConnectionSocketFactory.class); - socketFactoryRegistry = Mockito.mock(Lookup.class); + tlsSocketStrategy = Mockito.mock(TlsSocketStrategy.class); + tlsSocketStrategyLookup = Mockito.mock(Lookup.class); schemePortResolver = Mockito.mock(SchemePortResolver.class); dnsResolver = Mockito.mock(DnsResolver.class); connectionOperator = new DefaultHttpClientConnectionOperator( - detachedSocketFactory, socketFactoryRegistry, schemePortResolver, dnsResolver); + detachedSocketFactory, schemePortResolver, dnsResolver, tlsSocketStrategyLookup); } @Test @@ -131,9 +132,9 @@ public void testConnectWithTLSUpgrade() throws Exception { 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.when(tlsSocketStrategyLookup.lookup("https")).thenReturn(tlsSocketStrategy); + final SSLSocket upgradedSocket = Mockito.mock(SSLSocket.class); + Mockito.when(tlsSocketStrategy.upgrade( Mockito.same(socket), Mockito.eq("somehost"), Mockito.eq(443), @@ -146,7 +147,7 @@ public void testConnectWithTLSUpgrade() throws Exception { 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(tlsSocketStrategy).upgrade(socket, "somehost", 443, tlsConfig, context); Mockito.verify(conn, Mockito.times(1)).bind(upgradedSocket); } @@ -240,19 +241,20 @@ public void testUpgrade() throws Exception { Mockito.when(conn.isOpen()).thenReturn(true); Mockito.when(conn.getSocket()).thenReturn(socket); - Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory); + Mockito.when(tlsSocketStrategyLookup.lookup("https")).thenReturn(tlsSocketStrategy); Mockito.when(schemePortResolver.resolve(host)).thenReturn(443); - Mockito.when(sslSocketFactory.createSocket(Mockito.any())).thenReturn(socket); - Mockito.when(sslSocketFactory.createLayeredSocket( + + final SSLSocket upgradedSocket = Mockito.mock(SSLSocket.class); + Mockito.when(tlsSocketStrategy.upgrade( Mockito.any(), Mockito.eq("somehost"), Mockito.eq(443), Mockito.eq(Timeout.ofMilliseconds(345)), - Mockito.any())).thenReturn(socket); + Mockito.any())).thenReturn(upgradedSocket); connectionOperator.upgrade(conn, host, Timeout.ofMilliseconds(345), context); - Mockito.verify(conn).bind(socket); + Mockito.verify(conn).bind(upgradedSocket); } @Test diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java index 4f43ce0c1f..8496a8c697 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestPoolingHttpClientConnectionManager.java @@ -34,6 +34,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import javax.net.ssl.SSLSocket; + import org.apache.hc.client5.http.DnsResolver; import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.SchemePortResolver; @@ -44,8 +46,7 @@ import org.apache.hc.client5.http.io.LeaseRequest; 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.client5.http.ssl.TlsSocketStrategy; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.config.Lookup; import org.apache.hc.core5.http.io.SocketConfig; @@ -68,14 +69,16 @@ public class TestPoolingHttpClientConnectionManager { @Mock private ManagedHttpClientConnection conn; @Mock - private Lookup socketFactoryRegistry; + private Lookup tlsSocketStrategyLookup; @Mock private DetachedSocketFactory detachedSocketFactory; @Mock - private LayeredConnectionSocketFactory sslSocketFactory; + private TlsSocketStrategy tlsSocketStrategy; @Mock private Socket socket; @Mock + private SSLSocket upgradedSocket; + @Mock private SchemePortResolver schemePortResolver; @Mock private DnsResolver dnsResolver; @@ -90,7 +93,7 @@ public class TestPoolingHttpClientConnectionManager { public void setup() throws Exception { MockitoAnnotations.openMocks(this); mgr = new PoolingHttpClientConnectionManager(new DefaultHttpClientConnectionOperator( - detachedSocketFactory, socketFactoryRegistry, schemePortResolver, dnsResolver), pool, + detachedSocketFactory, schemePortResolver, dnsResolver, tlsSocketStrategyLookup), pool, null); } @@ -263,13 +266,13 @@ public void testTargetConnect() throws Exception { Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443); Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); - Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory); - Mockito.when(sslSocketFactory.createLayeredSocket( + Mockito.when(tlsSocketStrategyLookup.lookup("https")).thenReturn(tlsSocketStrategy); + Mockito.when(tlsSocketStrategy.upgrade( Mockito.same(socket), Mockito.eq("somehost"), Mockito.eq(8443), Mockito.any(), - Mockito.any())).thenReturn(socket); + Mockito.any())).thenReturn(upgradedSocket); mgr.connect(endpoint1, null, context); @@ -277,7 +280,7 @@ public void testTargetConnect() throws Exception { Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target); Mockito.verify(detachedSocketFactory, Mockito.times(1)).create(null); Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 234); - Mockito.verify(sslSocketFactory).createLayeredSocket(socket, "somehost", 8443, tlsConfig, context); + Mockito.verify(tlsSocketStrategy).upgrade(socket, "somehost", 8443, tlsConfig, context); mgr.connect(endpoint1, TimeValue.ofMilliseconds(123), context); @@ -285,7 +288,7 @@ public void testTargetConnect() throws Exception { Mockito.verify(schemePortResolver, Mockito.times(2)).resolve(target); Mockito.verify(detachedSocketFactory, Mockito.times(2)).create(null); Mockito.verify(socket, Mockito.times(1)).connect(new InetSocketAddress(remote, 8443), 123); - Mockito.verify(sslSocketFactory, Mockito.times(2)).createLayeredSocket(socket, "somehost", 8443, tlsConfig, context); + Mockito.verify(tlsSocketStrategy, Mockito.times(2)).upgrade(socket, "somehost", 8443, tlsConfig, context); } @Test @@ -312,7 +315,6 @@ public void testProxyConnectAndUpgrade() throws Exception { final ConnectionEndpoint endpoint1 = connRequest1.get(Timeout.ofSeconds(1)); Assertions.assertNotNull(endpoint1); - final LayeredConnectionSocketFactory sslsf = Mockito.mock(LayeredConnectionSocketFactory.class); final HttpClientContext context = HttpClientContext.create(); final SocketConfig sconfig = SocketConfig.custom().build(); @@ -330,7 +332,7 @@ 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("https")).thenReturn(sslsf); + Mockito.when(tlsSocketStrategyLookup.lookup("https")).thenReturn(tlsSocketStrategy); Mockito.when(detachedSocketFactory.create(Mockito.any())).thenReturn(socket); mgr.connect(endpoint1, null, context); @@ -346,7 +348,7 @@ public void testProxyConnectAndUpgrade() throws Exception { mgr.upgrade(endpoint1, context); Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target); - Mockito.verify(sslsf, Mockito.times(1)).createLayeredSocket( + Mockito.verify(tlsSocketStrategy, Mockito.times(1)).upgrade( socket, "somehost", 8443, tlsConfig, context); } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestSSLSocketFactory.java b/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestSSLSocketFactory.java deleted file mode 100644 index a798d7ff9f..0000000000 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/ssl/TestSSLSocketFactory.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * ==================================================================== - * 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 - * . - * - */ - -package org.apache.hc.client5.http.ssl; - -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; - -/** - * Unit tests for {@link SSLConnectionSocketFactory}. - */ -public class TestSSLSocketFactory { - - @Test - public void testStrongCipherSuites() { - final String[] strongCipherSuites = { - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384", - "TLS_RSA_WITH_AES_256_CBC_SHA256", - "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256", - "TLS_RSA_WITH_AES_128_CBC_SHA", - "TLS_DHE_DSS_WITH_AES_128_CBC_SHA", - "TLS_RSA_WITH_AES_256_GCM_SHA384" - }; - for (final String cipherSuite : strongCipherSuites) { - Assertions.assertFalse(SSLConnectionSocketFactory.isWeakCipherSuite(cipherSuite)); - } - } - - @Test - public void testWeakCiphersDisabledByDefault() { - final String[] weakCiphersSuites = { - "SSL_RSA_WITH_RC4_128_SHA", - "SSL_RSA_WITH_3DES_EDE_CBC_SHA", - "TLS_DH_anon_WITH_AES_128_CBC_SHA", - "SSL_RSA_EXPORT_WITH_DES40_CBC_SHA", - "SSL_RSA_WITH_NULL_SHA", - "SSL_RSA_WITH_3DES_EDE_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA", - "TLS_ECDH_ECDSA_WITH_3DES_EDE_CBC_SHA", - "TLS_DH_anon_WITH_AES_256_GCM_SHA384", - "TLS_ECDH_anon_WITH_AES_256_CBC_SHA", - "TLS_RSA_WITH_NULL_SHA256", - "SSL_RSA_EXPORT_WITH_RC4_40_MD5", - "SSL_DH_anon_EXPORT_WITH_RC4_40_MD5", - "TLS_KRB5_EXPORT_WITH_RC4_40_SHA", - "SSL_RSA_EXPORT_WITH_RC2_CBC_40_MD5" - }; - for (final String cipherSuite : weakCiphersSuites) { - Assertions.assertTrue(SSLConnectionSocketFactory.isWeakCipherSuite(cipherSuite)); - } - } - -} From fd2961a1b1f8f87eba3490b998c646ee9e60c98c Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 22 Jan 2024 12:24:11 +0100 Subject: [PATCH 4/4] Realigned the behavior of TLS upgrade in the classic and async connection operators --- .../DefaultHttpClientConnectionOperator.java | 19 +++++++++---------- .../DefaultAsyncClientConnectionOperator.java | 7 ++++--- .../io/TestHttpClientConnectionOperator.java | 6 ++++++ 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java index e2189d64d3..64f0a64837 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultHttpClientConnectionOperator.java @@ -53,7 +53,6 @@ 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; @@ -226,7 +225,7 @@ public void connect( host.getHostName(), host.getPort(), localAddress, remoteAddress, ConnPoolSupport.getId(conn)); } final TlsSocketStrategy tlsSocketStrategy = tlsSocketStrategyLookup != null ? tlsSocketStrategyLookup.lookup(host.getSchemeName()) : null; - if (tlsSocketStrategy != null && URIScheme.HTTPS.same(host.getSchemeName())) { + if (tlsSocketStrategy != null) { final Socket upgradedSocket = tlsSocketStrategy.upgrade(socket, host.getHostName(), port, attachment, context); conn.bind(upgradedSocket); } @@ -266,18 +265,18 @@ public void upgrade( final HttpHost host, final Object attachment, final HttpContext context) throws IOException { - final TlsSocketStrategy tlsSocketStrategy = tlsSocketStrategyLookup != null ? tlsSocketStrategyLookup.lookup(host.getSchemeName()) : null; - if (tlsSocketStrategy == null) { - throw new UnsupportedSchemeException(host.getSchemeName() + - " protocol is not supported"); - } final Socket socket = conn.getSocket(); if (socket == null) { throw new ConnectionClosedException("Connection is closed"); } - final int port = this.schemePortResolver.resolve(host); - final SSLSocket upgradedSocket = tlsSocketStrategy.upgrade(socket, host.getHostName(), port, attachment, context); - conn.bind(upgradedSocket); + final TlsSocketStrategy tlsSocketStrategy = tlsSocketStrategyLookup != null ? tlsSocketStrategyLookup.lookup(host.getSchemeName()) : null; + if (tlsSocketStrategy != null) { + final int port = this.schemePortResolver.resolve(host); + final SSLSocket upgradedSocket = tlsSocketStrategy.upgrade(socket, host.getHostName(), port, attachment, context); + conn.bind(upgradedSocket); + } else { + throw new UnsupportedSchemeException(host.getSchemeName() + " protocol is not supported"); + } } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java index 9571af4e28..0a046887dd 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/DefaultAsyncClientConnectionOperator.java @@ -34,6 +34,7 @@ import org.apache.hc.client5.http.DnsResolver; 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.impl.DefaultSchemePortResolver; import org.apache.hc.client5.http.nio.AsyncClientConnectionOperator; @@ -44,7 +45,6 @@ import org.apache.hc.core5.concurrent.FutureCallback; import org.apache.hc.core5.concurrent.FutureContribution; 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.nio.ssl.TlsStrategy; import org.apache.hc.core5.http.protocol.HttpContext; @@ -109,7 +109,7 @@ public Future connect( 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())) { + if (tlsStrategy != null) { try { final Timeout socketTimeout = connection.getSocketTimeout(); final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout(); @@ -191,8 +191,9 @@ public void completed(final TransportSecurityLayer transportSecurityLayer) { } }); + } else { + callback.failed(new UnsupportedSchemeException(host.getSchemeName() + " protocol is not supported")); } - } } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java index fd5f1e600c..28a40fa315 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java @@ -262,6 +262,9 @@ public void testUpgradeUpsupportedScheme() throws Exception { final HttpContext context = new BasicHttpContext(); final HttpHost host = new HttpHost("httpsssss", "somehost", -1); + Mockito.when(conn.isOpen()).thenReturn(true); + Mockito.when(conn.getSocket()).thenReturn(socket); + Assertions.assertThrows(UnsupportedSchemeException.class, () -> connectionOperator.upgrade(conn, host, context)); } @@ -271,6 +274,9 @@ public void testUpgradeNonLayeringScheme() throws Exception { final HttpContext context = new BasicHttpContext(); final HttpHost host = new HttpHost("http", "somehost", -1); + Mockito.when(conn.isOpen()).thenReturn(true); + Mockito.when(conn.getSocket()).thenReturn(socket); + Assertions.assertThrows(UnsupportedSchemeException.class, () -> connectionOperator.upgrade(conn, host, context)); }