You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2021/09/17 09:37:17 UTC

[httpcomponents-client] branch master updated (783ddfe -> 677903e)

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git.


 discard 783ddfe  HTTPCLIENT-2045: BASIC auth scheme conformance to RFC 7617
 discard 4b7057e  HTTPCLIENT-2135: support for a distinct handshake timeout (mainly intended for TLS/SSL) by the connection management APIs
 discard e544e07  Extracted multi-home socket connect logic into a separate utility class; made more consistent with async multi-home connect code
     new 50f93ec  HTTPCLIENT-2045: BASIC auth scheme conformance to RFC 7617
     new da5168f  Better connect operation logging
     new 677903e  HTTPCLIENT-2135: support for a distinct handshake timeout (mainly intended for TLS/SSL) by the connection management APIs

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (783ddfe)
            \
             N -- N -- N   refs/heads/master (677903e)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../impl/io/BasicHttpClientConnectionManager.java  |   2 +
 .../io/DefaultHttpClientConnectionOperator.java    | 125 ++++++++++++++++-----
 .../http/impl/io/MultihomeSocketConnector.java     | 118 -------------------
 .../io/PoolingHttpClientConnectionManager.java     |   4 +-
 .../http/impl/nio/MultihomeIOSessionRequester.java |  19 ++--
 .../http/io/HttpClientConnectionOperator.java      |  20 ++++
 .../http/socket/ConnectionSocketFactory.java       |  32 ++++++
 .../socket/LayeredConnectionSocketFactory.java     |  27 +++++
 .../http/ssl/AbstractClientTlsStrategy.java        |   1 +
 .../http/ssl/SSLConnectionSocketFactory.java       |  47 ++++++--
 .../io/TestBasicHttpClientConnectionManager.java   |  58 ++++++++--
 .../impl/io/TestHttpClientConnectionOperator.java  |  34 ++++--
 .../io/TestPoolingHttpClientConnectionManager.java |  57 ++++++++--
 13 files changed, 352 insertions(+), 192 deletions(-)
 delete mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/MultihomeSocketConnector.java

[httpcomponents-client] 03/03: HTTPCLIENT-2135: support for a distinct handshake timeout (mainly intended for TLS/SSL) by the connection management APIs

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit 677903e186424e2785bf849ee3145c7d9b3297e0
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Sun Sep 12 14:54:32 2021 +0200

    HTTPCLIENT-2135: support for a distinct handshake timeout (mainly intended for TLS/SSL) by the connection management APIs
---
 .../testing/sync/TestConnectionManagement.java     | 14 +++---
 .../hc/client5/http/config/ConnectionConfig.java   | 47 ++++++++++++++++--
 .../impl/io/BasicHttpClientConnectionManager.java  |  6 ++-
 .../io/DefaultHttpClientConnectionOperator.java    | 34 +++++++++++--
 .../io/PoolingHttpClientConnectionManager.java     | 10 ++--
 .../nio/DefaultAsyncClientConnectionOperator.java  | 16 +++++-
 .../nio/PoolingAsyncClientConnectionManager.java   |  9 +++-
 .../http/io/HttpClientConnectionOperator.java      | 44 ++++++++++++++++
 .../http/nio/AsyncClientConnectionOperator.java    | 24 +++++++++
 .../http/socket/ConnectionSocketFactory.java       | 32 ++++++++++++
 .../socket/LayeredConnectionSocketFactory.java     | 27 ++++++++++
 .../http/ssl/AbstractClientTlsStrategy.java        |  1 +
 .../http/ssl/SSLConnectionSocketFactory.java       | 47 ++++++++++++++----
 .../io/TestBasicHttpClientConnectionManager.java   | 58 ++++++++++++++++++----
 .../impl/io/TestHttpClientConnectionOperator.java  | 32 ++++++++----
 .../io/TestPoolingHttpClientConnectionManager.java | 57 +++++++++++++++++----
 16 files changed, 401 insertions(+), 57 deletions(-)

diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java
index 9b1f8fd..824e4fa 100644
--- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java
+++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java
@@ -82,7 +82,7 @@ public class TestConnectionManagement extends LocalServerTestBase {
         final LeaseRequest leaseRequest1 = this.connManager.lease("id1", route,null);
         final ConnectionEndpoint endpoint1 = leaseRequest1.get(Timeout.ZERO_MILLISECONDS);
 
-        this.connManager.connect(endpoint1, TimeValue.NEG_ONE_MILLISECOND, context);
+        this.connManager.connect(endpoint1, null, context);
 
         final HttpProcessor httpProcessor = new DefaultHttpProcessor(
                 new RequestTargetHost(), new RequestContent(), new RequestConnControl());
@@ -104,7 +104,7 @@ public class TestConnectionManagement extends LocalServerTestBase {
         final ConnectionEndpoint endpoint2 = leaseRequest3.get(Timeout.ZERO_MILLISECONDS);
         Assert.assertFalse(endpoint2.isConnected());
 
-        this.connManager.connect(endpoint2, TimeValue.NEG_ONE_MILLISECOND, context);
+        this.connManager.connect(endpoint2, null, context);
 
         try (final ClassicHttpResponse response2 = endpoint2.execute("id2", request, exec, context)) {
             Assert.assertEquals(HttpStatus.SC_OK, response2.getCode());
@@ -145,7 +145,7 @@ public class TestConnectionManagement extends LocalServerTestBase {
 
         final LeaseRequest leaseRequest1 = this.connManager.lease("id1", route,null);
         final ConnectionEndpoint endpoint1 = leaseRequest1.get(Timeout.ZERO_MILLISECONDS);
-        this.connManager.connect(endpoint1, TimeValue.NEG_ONE_MILLISECOND, context);
+        this.connManager.connect(endpoint1, null, context);
 
         final HttpProcessor httpProcessor = new DefaultHttpProcessor(
                 new RequestTargetHost(), new RequestContent(), new RequestConnControl());
@@ -168,7 +168,7 @@ public class TestConnectionManagement extends LocalServerTestBase {
         final ConnectionEndpoint endpoint2 = leaseRequest3.get(Timeout.ZERO_MILLISECONDS);
         Assert.assertFalse(endpoint2.isConnected());
 
-        this.connManager.connect(endpoint2, TimeValue.NEG_ONE_MILLISECOND, context);
+        this.connManager.connect(endpoint2, null, context);
 
         try (final ClassicHttpResponse response2 = endpoint2.execute("id2", request, exec, context)) {
             Assert.assertEquals(HttpStatus.SC_OK, response2.getCode());
@@ -193,7 +193,7 @@ public class TestConnectionManagement extends LocalServerTestBase {
         Assert.assertFalse(endpoint4.isConnected());
 
         // repeat the communication, no need to prepare the request again
-        this.connManager.connect(endpoint4, TimeValue.NEG_ONE_MILLISECOND, context);
+        this.connManager.connect(endpoint4, null, context);
 
         try (final ClassicHttpResponse response4 = endpoint4.execute("id4", request, exec, context)) {
             Assert.assertEquals(HttpStatus.SC_OK, response4.getCode());
@@ -213,7 +213,7 @@ public class TestConnectionManagement extends LocalServerTestBase {
 
         final LeaseRequest leaseRequest1 = this.connManager.lease("id1", route,null);
         final ConnectionEndpoint endpoint1 = leaseRequest1.get(Timeout.ZERO_MILLISECONDS);
-        this.connManager.connect(endpoint1, TimeValue.NEG_ONE_MILLISECOND, context);
+        this.connManager.connect(endpoint1, null, context);
 
         Assert.assertEquals(1, this.connManager.getTotalStats().getLeased());
         Assert.assertEquals(1, this.connManager.getStats(route).getLeased());
@@ -262,7 +262,7 @@ public class TestConnectionManagement extends LocalServerTestBase {
 
         final LeaseRequest leaseRequest1 = this.connManager.lease("id1", route,null);
         final ConnectionEndpoint endpoint1 = leaseRequest1.get(Timeout.ZERO_MILLISECONDS);
-        this.connManager.connect(endpoint1, TimeValue.NEG_ONE_MILLISECOND, context);
+        this.connManager.connect(endpoint1, null, context);
 
         Assert.assertEquals(1, this.connManager.getTotalStats().getLeased());
         Assert.assertEquals(1, this.connManager.getStats(route).getLeased());
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/config/ConnectionConfig.java b/httpclient5/src/main/java/org/apache/hc/client5/http/config/ConnectionConfig.java
index 97ed585..c8f6fb4 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/config/ConnectionConfig.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/config/ConnectionConfig.java
@@ -48,22 +48,25 @@ public class ConnectionConfig implements Cloneable {
 
     private final Timeout connectTimeout;
     private final Timeout socketTimeout;
+    private final Timeout handshakeTimeout;
     private final TimeValue validateAfterInactivity;
 
     /**
      * Intended for CDI compatibility
      */
     protected ConnectionConfig() {
-        this(DEFAULT_CONNECT_TIMEOUT, null, null);
+        this(DEFAULT_CONNECT_TIMEOUT, null, null, null);
     }
 
     ConnectionConfig(
             final Timeout connectTimeout,
             final Timeout socketTimeout,
+            final Timeout handshakeTimeout,
             final TimeValue validateAfterInactivity) {
         super();
         this.connectTimeout = connectTimeout;
         this.socketTimeout = socketTimeout;
+        this.handshakeTimeout = handshakeTimeout;
         this.validateAfterInactivity = validateAfterInactivity;
     }
 
@@ -82,6 +85,13 @@ public class ConnectionConfig implements Cloneable {
     }
 
     /**
+     * @see Builder#setHandshakeTimeout(Timeout)
+     */
+    public Timeout getHandshakeTimeout() {
+        return handshakeTimeout;
+    }
+
+    /**
      * @see Builder#setValidateAfterInactivity(TimeValue)
      */
     public TimeValue getValidateAfterInactivity() {
@@ -99,6 +109,7 @@ public class ConnectionConfig implements Cloneable {
         builder.append("[");
         builder.append(", connectTimeout=").append(connectTimeout);
         builder.append(", socketTimeout=").append(socketTimeout);
+        builder.append(", handshakeTimeout=").append(handshakeTimeout);
         builder.append(", validateAfterInactivity=").append(validateAfterInactivity);
         builder.append("]");
         return builder.toString();
@@ -112,6 +123,7 @@ public class ConnectionConfig implements Cloneable {
         return new Builder()
                 .setConnectTimeout(config.getConnectTimeout())
                 .setSocketTimeout(config.getSocketTimeout())
+                .setHandshakeTimeout(config.getHandshakeTimeout())
                 .setValidateAfterInactivity(config.getValidateAfterInactivity());
     }
 
@@ -119,6 +131,7 @@ public class ConnectionConfig implements Cloneable {
 
         private Timeout socketTimeout;
         private Timeout connectTimeout;
+        private Timeout handshakeTimeout;
         private TimeValue validateAfterInactivity;
 
         Builder() {
@@ -126,7 +139,6 @@ public class ConnectionConfig implements Cloneable {
             this.connectTimeout = DEFAULT_CONNECT_TIMEOUT;
         }
 
-
         /**
          * @see #setSocketTimeout(Timeout)
          */
@@ -138,7 +150,7 @@ public class ConnectionConfig implements Cloneable {
         /**
          * Determines the default socket timeout value for I/O operations.
          * <p>
-         * Default: {@code null}
+         * Default: {@code null} (undefined)
          * </p>
          *
          * @return the default socket timeout value for I/O operations.
@@ -151,7 +163,8 @@ public class ConnectionConfig implements Cloneable {
         /**
          * Determines the timeout until a new connection is fully established.
          * This may also include transport security negotiation exchanges
-         * such as {@code SSL} or {@code TLS} protocol negotiation).
+         * such as {@code SSL} or {@code TLS} protocol negotiation unless
+         * a different timeout value set with {@link #setHandshakeTimeout(Timeout)}.
          * <p>
          * A timeout value of zero is interpreted as an infinite timeout.
          * </p>
@@ -173,11 +186,34 @@ public class ConnectionConfig implements Cloneable {
         }
 
         /**
+         * Determines the timeout used by transport security negotiation exchanges
+         * such as {@code SSL} or {@code TLS} protocol negotiation).
+         * <p>
+         * A timeout value of zero is interpreted as an infinite timeout.
+         * </p>
+         * <p>
+         * Default: {@code null} (undefined)
+         * </p>
+         */
+        public Builder setHandshakeTimeout(final Timeout handshakeTimeout) {
+            this.handshakeTimeout = connectTimeout;
+            return this;
+        }
+
+        /**
+         * @see #setHandshakeTimeout(Timeout)
+         */
+        public Builder setHandshakeTimeout(final long handshakeTimeout, final TimeUnit timeUnit) {
+            this.handshakeTimeout = Timeout.of(handshakeTimeout, timeUnit);
+            return this;
+        }
+
+        /**
          * Defines period of inactivity after which persistent connections must
          * be re-validated prior to being leased to the consumer. Negative values passed
          * to this method disable connection validation.
          * <p>
-         * Default: {@code null}
+         * Default: {@code null} (undefined)
          * </p>
          */
         public Builder setValidateAfterInactivity(final TimeValue validateAfterInactivity) {
@@ -197,6 +233,7 @@ public class ConnectionConfig implements Cloneable {
             return new ConnectionConfig(
                     connectTimeout != null ? connectTimeout : DEFAULT_CONNECT_TIMEOUT,
                     socketTimeout,
+                    handshakeTimeout,
                     validateAfterInactivity);
         }
 
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 b9866c1..bc702b8 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
@@ -359,7 +359,8 @@ public class BasicHttpClientConnectionManager implements HttpClientConnectionMan
             host = route.getTargetHost();
         }
         final ConnectionConfig config = connectionConfig != null ? connectionConfig : ConnectionConfig.DEFAULT;
-        final TimeValue connectTimeout = timeout != null ? timeout : config.getConnectTimeout();
+        final Timeout connectTimeout = timeout != null ? Timeout.of(timeout.getDuration(), timeout.getTimeUnit()) : config.getConnectTimeout();
+        final Timeout handshakeTimeout = config.getHandshakeTimeout();
         final ManagedHttpClientConnection connection = internalEndpoint.getConnection();
         if (LOG.isDebugEnabled()) {
             LOG.debug("{} connecting endpoint to {} ({})", ConnPoolSupport.getId(endpoint), host, connectTimeout);
@@ -369,6 +370,7 @@ public class BasicHttpClientConnectionManager implements HttpClientConnectionMan
                 host,
                 route.getLocalSocketAddress(),
                 connectTimeout,
+                handshakeTimeout,
                 this.socketConfig,
                 context);
         if (LOG.isDebugEnabled()) {
@@ -387,9 +389,11 @@ public class BasicHttpClientConnectionManager implements HttpClientConnectionMan
         Args.notNull(endpoint, "Endpoint");
         Args.notNull(route, "HTTP route");
         final InternalConnectionEndpoint internalEndpoint = cast(endpoint);
+        final ConnectionConfig config = connectionConfig != null ? connectionConfig : ConnectionConfig.DEFAULT;
         this.connectionOperator.upgrade(
                 internalEndpoint.getConnection(),
                 internalEndpoint.getRoute().getTargetHost(),
+                config.getHandshakeTimeout(),
                 context);
     }
 
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 926f588..c33f230 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
@@ -54,6 +54,7 @@ import org.apache.hc.core5.http.io.SocketConfig;
 import org.apache.hc.core5.http.protocol.HttpContext;
 import org.apache.hc.core5.util.Args;
 import org.apache.hc.core5.util.TimeValue;
+import org.apache.hc.core5.util.Timeout;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -107,6 +108,19 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection
             final TimeValue connectTimeout,
             final SocketConfig socketConfig,
             final HttpContext context) throws IOException {
+        final Timeout timeout = connectTimeout != null ? Timeout.of(connectTimeout.getDuration(), connectTimeout.getTimeUnit()) : null;
+        connect(conn, host, localAddress, timeout, timeout, socketConfig, context);
+    }
+
+    @Override
+    public void connect(
+            final ManagedHttpClientConnection conn,
+            final HttpHost host,
+            final InetSocketAddress localAddress,
+            final Timeout connectTimeout,
+            final Timeout handshakeTimeout,
+            final SocketConfig socketConfig,
+            final HttpContext context) throws IOException {
         Args.notNull(conn, "Connection");
         Args.notNull(host, "Host");
         Args.notNull(socketConfig, "Socket config");
@@ -131,13 +145,17 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection
             }
         }
 
+        final Timeout soTimeout = socketConfig.getSoTimeout();
+
         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(context);
-            sock.setSoTimeout(socketConfig.getSoTimeout().toMillisecondsIntBound());
+            if (soTimeout != null) {
+                sock.setSoTimeout(soTimeout.toMillisecondsIntBound());
+            }
             sock.setReuseAddress(socketConfig.isSoReuseAddress());
             sock.setTcpNoDelay(socketConfig.isTcpNoDelay());
             sock.setKeepAlive(socketConfig.isSoKeepAlive());
@@ -160,8 +178,9 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection
                         host.getHostName(), host.getPort(), localAddress, remoteAddress, connectTimeout);
             }
             try {
-                sock = sf.connectSocket(connectTimeout, sock, host, remoteAddress, localAddress, context);
+                sock = sf.connectSocket(sock, host, remoteAddress, localAddress, connectTimeout, handshakeTimeout, context);
                 conn.bind(sock);
+                conn.setSocketTimeout(soTimeout);
                 if (LOG.isDebugEnabled()) {
                     LOG.debug("{}:{} connected {}->{} as {}",
                             host.getHostName(), host.getPort(), localAddress, remoteAddress, ConnPoolSupport.getId(conn));
@@ -189,6 +208,15 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection
             final ManagedHttpClientConnection conn,
             final HttpHost host,
             final HttpContext context) throws IOException {
+        upgrade(conn, host, null, context);
+    }
+
+    @Override
+    public void upgrade(
+            final ManagedHttpClientConnection conn,
+            final HttpHost host,
+            final Timeout handshakeTimeout,
+            final HttpContext context) throws IOException {
         final HttpClientContext clientContext = HttpClientContext.adapt(context);
         final Lookup<ConnectionSocketFactory> registry = getSocketFactoryRegistry(clientContext);
         final ConnectionSocketFactory sf = registry.lookup(host.getSchemeName());
@@ -206,7 +234,7 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection
             throw new ConnectionClosedException("Connection is closed");
         }
         final int port = this.schemePortResolver.resolve(host);
-        sock = lsf.createLayeredSocket(sock, host.getHostName(), port, context);
+        sock = lsf.createLayeredSocket(sock, host.getHostName(), port, handshakeTimeout, context);
         conn.bind(sock);
     }
 
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 22a29e9..45fe784 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
@@ -409,7 +409,8 @@ public class PoolingHttpClientConnectionManager
         }
         final SocketConfig socketConfig = resolveSocketConfig(route);
         final ConnectionConfig connectionConfig = resolveConnectionConfig(route);
-        final TimeValue connectTimeout = timeout != null ? timeout : connectionConfig.getConnectTimeout();
+        final Timeout connectTimeout = timeout != null ? Timeout.of(timeout.getDuration(), timeout.getTimeUnit()) : connectionConfig.getConnectTimeout();
+        final Timeout handshakeTimeout = connectionConfig.getHandshakeTimeout();
         if (LOG.isDebugEnabled()) {
             LOG.debug("{} connecting endpoint to {} ({})", ConnPoolSupport.getId(endpoint), host, connectTimeout);
         }
@@ -418,7 +419,8 @@ public class PoolingHttpClientConnectionManager
                 conn,
                 host,
                 route.getLocalSocketAddress(),
-                timeout,
+                connectTimeout,
+                handshakeTimeout,
                 socketConfig,
                 context);
         if (LOG.isDebugEnabled()) {
@@ -436,7 +438,9 @@ public class PoolingHttpClientConnectionManager
         final InternalConnectionEndpoint internalEndpoint = cast(endpoint);
         final PoolEntry<HttpRoute, ManagedHttpClientConnection> poolEntry = internalEndpoint.getValidatedPoolEntry();
         final HttpRoute route = poolEntry.getRoute();
-        this.connectionOperator.upgrade(poolEntry.getConnection(), route.getTargetHost(), context);
+        final ConnectionConfig connectionConfig = resolveConnectionConfig(route);
+        final Timeout handshakeTimeout = connectionConfig.getHandshakeTimeout();
+        this.connectionOperator.upgrade(poolEntry.getConnection(), route.getTargetHost(), handshakeTimeout, context);
     }
 
     @Override
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 a781e9b..a5dc368 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
@@ -73,6 +73,7 @@ final class DefaultAsyncClientConnectionOperator implements AsyncClientConnectio
             final HttpHost host,
             final SocketAddress localAddress,
             final Timeout connectTimeout,
+            final Timeout handshakeTimeout,
             final Object attachment,
             final FutureCallback<ManagedAsyncClientConnection> callback) {
         Args.notNull(connectionInitiator, "Connection initiator");
@@ -95,15 +96,17 @@ final class DefaultAsyncClientConnectionOperator implements AsyncClientConnectio
                         final DefaultManagedAsyncClientConnection connection = new DefaultManagedAsyncClientConnection(session);
                         if (tlsStrategy != null && URIScheme.HTTPS.same(host.getSchemeName())) {
                             try {
+                                final Timeout socketTimeout = connection.getSocketTimeout();
                                 tlsStrategy.upgrade(
                                         connection,
                                         host,
                                         attachment,
-                                        null,
+                                        handshakeTimeout != null ? handshakeTimeout : connectTimeout,
                                         new FutureContribution<TransportSecurityLayer>(future) {
 
                                             @Override
                                             public void completed(final TransportSecurityLayer transportSecurityLayer) {
+                                                connection.setSocketTimeout(socketTimeout);
                                                 future.completed(connection);
                                             }
 
@@ -132,6 +135,17 @@ final class DefaultAsyncClientConnectionOperator implements AsyncClientConnectio
     }
 
     @Override
+    public Future<ManagedAsyncClientConnection> connect(
+            final ConnectionInitiator connectionInitiator,
+            final HttpHost host,
+            final SocketAddress localAddress,
+            final Timeout connectTimeout,
+            final Object attachment,
+            final FutureCallback<ManagedAsyncClientConnection> callback) {
+        return connect(connectionInitiator, host, localAddress, connectTimeout, connectTimeout, attachment, callback);
+    }
+
+    @Override
     public void upgrade(
             final ManagedAsyncClientConnection connection,
             final HttpHost host,
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 3bc64ab..7c8089d 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
@@ -400,12 +400,19 @@ public class PoolingAsyncClientConnectionManager implements AsyncClientConnectio
         final InetSocketAddress localAddress = route.getLocalSocketAddress();
         final ConnectionConfig connectionConfig = resolveConnectionConfig(route);
         final Timeout connectTimeout = timeout != null ? timeout : connectionConfig.getConnectTimeout();
+        final Timeout handshakeTimeout = connectionConfig.getHandshakeTimeout();
 
         if (LOG.isDebugEnabled()) {
             LOG.debug("{} connecting endpoint to {} ({})", ConnPoolSupport.getId(endpoint), host, connectTimeout);
         }
         final Future<ManagedAsyncClientConnection> connectFuture = connectionOperator.connect(
-                connectionInitiator, host, localAddress, connectTimeout, attachment, new FutureCallback<ManagedAsyncClientConnection>() {
+                connectionInitiator,
+                host,
+                localAddress,
+                connectTimeout,
+                handshakeTimeout,
+                attachment,
+                new FutureCallback<ManagedAsyncClientConnection>() {
 
                     @Override
                     public void completed(final ManagedAsyncClientConnection connection) {
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/io/HttpClientConnectionOperator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/io/HttpClientConnectionOperator.java
index d1bf4fa..5308548 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/io/HttpClientConnectionOperator.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/io/HttpClientConnectionOperator.java
@@ -37,6 +37,7 @@ import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.io.SocketConfig;
 import org.apache.hc.core5.http.protocol.HttpContext;
 import org.apache.hc.core5.util.TimeValue;
+import org.apache.hc.core5.util.Timeout;
 
 /**
  * Connection operator that performs connection connect and upgrade operations.
@@ -66,6 +67,30 @@ public interface HttpClientConnectionOperator {
             HttpContext context) throws IOException;
 
     /**
+     * Connect the given managed connection to the remote endpoint.
+     *
+     * @param conn the managed connection.
+     * @param host the address of the opposite endpoint.
+     * @param localAddress the address of the local endpoint.
+     * @param connectTimeout the timeout of the connect operation.
+     * @param handshakeTimeout the handshake timeout, if applicable.
+     * @param socketConfig the socket configuration.
+     * @param context the execution context.
+     *
+     * @since 5.2
+     */
+    default void connect(
+            ManagedHttpClientConnection conn,
+            HttpHost host,
+            InetSocketAddress localAddress,
+            Timeout connectTimeout,
+            Timeout handshakeTimeout,
+            SocketConfig socketConfig,
+            HttpContext context) throws IOException {
+        connect(conn, host, localAddress, connectTimeout, socketConfig, context);
+    }
+
+    /**
      * Upgrades transport security of the given managed connection
      * by using the TLS security protocol.
      *
@@ -78,4 +103,23 @@ public interface HttpClientConnectionOperator {
             HttpHost host,
             HttpContext context) throws IOException;
 
+    /**
+     * Upgrades transport security of the given managed connection
+     * by using the TLS security protocol.
+     *
+     * @param conn the managed connection.
+     * @param host the address of the opposite endpoint with TLS security.
+     * @param handshakeTimeout the handshake timeout, if applicable.
+     * @param context the execution context.
+     *
+     * @since 5.2
+     */
+    default void upgrade(
+            ManagedHttpClientConnection conn,
+            HttpHost host,
+            Timeout handshakeTimeout,
+            HttpContext context) throws IOException {
+        upgrade(conn, host, context);
+    }
+
 }
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/nio/AsyncClientConnectionOperator.java b/httpclient5/src/main/java/org/apache/hc/client5/http/nio/AsyncClientConnectionOperator.java
index af12604..2ad7dee 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/nio/AsyncClientConnectionOperator.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/nio/AsyncClientConnectionOperator.java
@@ -67,6 +67,30 @@ public interface AsyncClientConnectionOperator {
             Object attachment,
             FutureCallback<ManagedAsyncClientConnection> callback);
 
+    /**
+     * Initiates operation to create a connection to the remote endpoint using
+     * the provided {@link ConnectionInitiator}.
+     *
+     * @param connectionInitiator the connection initiator.
+     * @param host the address of the opposite endpoint.
+     * @param localAddress the address of the local endpoint.
+     * @param connectTimeout the timeout of the connect operation.
+     * @param attachment the attachment, which can be any object representing custom parameter
+     *                    of the operation.
+     * @param callback the future result callback.
+     *
+     * @since 5.2
+     */
+    default Future<ManagedAsyncClientConnection> connect(
+            ConnectionInitiator connectionInitiator,
+            HttpHost host,
+            SocketAddress localAddress,
+            Timeout connectTimeout,
+            Timeout handshakeTimeout,
+            Object attachment,
+            FutureCallback<ManagedAsyncClientConnection> callback) {
+        return connect(connectionInitiator, host, localAddress, connectTimeout, handshakeTimeout, attachment, callback);
+    }
 
     /**
      * Upgrades transport security of the given managed connection
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 88cafd0..b7f5dde 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
@@ -36,6 +36,7 @@ import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.protocol.HttpContext;
 import org.apache.hc.core5.util.TimeValue;
+import org.apache.hc.core5.util.Timeout;
 
 /**
  * A factory for creating and connecting connection sockets.
@@ -81,4 +82,35 @@ public interface ConnectionSocketFactory {
         InetSocketAddress localAddress,
         HttpContext context) throws IOException;
 
+    /**
+     * Connects the socket to the target host with the given resolved remote address.
+     *
+     * @param socket the socket to connect, as obtained from {@link #createSocket(HttpContext)}.
+     * {@code null} indicates that a new socket should be created and connected.
+     * @param host target host as specified by the caller (end user).
+     * @param remoteAddress the resolved remote address to connect to.
+     * @param localAddress the local address to bind the socket to, or {@code null} for any.
+     * @param connectTimeout connect timeout.
+     * @param handshakeTimeout handshake timeout, if applicable.
+     * @param context the actual HTTP context.
+     *
+     * @return  the connected socket. The returned object may be different
+     *          from the {@code sock} argument if this factory supports
+     *          a layered protocol.
+     *
+     * @throws IOException if an I/O error occurs
+     *
+     * @since 5.2
+     */
+    default Socket connectSocket(
+            Socket socket,
+            HttpHost host,
+            InetSocketAddress remoteAddress,
+            InetSocketAddress localAddress,
+            Timeout connectTimeout,
+            Timeout handshakeTimeout,
+            HttpContext context) throws IOException {
+        return connectSocket(connectTimeout, socket, host, remoteAddress, localAddress, context);
+    }
+
 }
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 d35c296..80661fc 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
@@ -33,6 +33,7 @@ import java.net.Socket;
 import org.apache.hc.core5.annotation.Contract;
 import org.apache.hc.core5.annotation.ThreadingBehavior;
 import org.apache.hc.core5.http.protocol.HttpContext;
+import org.apache.hc.core5.util.Timeout;
 
 /**
  * Extended {@link ConnectionSocketFactory} interface for layered sockets such as SSL/TLS.
@@ -62,4 +63,30 @@ public interface LayeredConnectionSocketFactory extends ConnectionSocketFactory
         int port,
         HttpContext context) throws IOException;
 
+    /**
+     * Returns a socket connected to the given host that is layered over an
+     * existing socket.  Used primarily for creating secure sockets through
+     * proxies.
+     *
+     * @param socket the existing 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 handshakeTimeout handshake timeout, if applicable.
+     *
+     * @return Socket a new socket
+     *
+     * @throws IOException if an I/O error occurs while creating the socket
+     *
+     * @since 5.2
+     */
+    default Socket createLayeredSocket(
+            Socket socket,
+            String target,
+            int port,
+            Timeout handshakeTimeout,
+            HttpContext context) throws IOException {
+        return createLayeredSocket(socket, target, port, context);
+    }
+
 }
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 45a66f7..93a1f4a 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
@@ -135,6 +135,7 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy {
             if (LOG.isDebugEnabled()) {
                 LOG.debug("Enabled protocols: {}", Arrays.asList(sslEngine.getEnabledProtocols()));
                 LOG.debug("Enabled cipher suites:{}", Arrays.asList(sslEngine.getEnabledCipherSuites()));
+                LOG.debug("Starting handshake ({})", handshakeTimeout);
             }
         }, (e, sslEngine) -> {
             verifySession(endpoint.getHostName(), sslEngine.getSession());
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 2a29e91..a07a389 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
@@ -60,6 +60,7 @@ import org.apache.hc.core5.ssl.SSLInitializationException;
 import org.apache.hc.core5.util.Args;
 import org.apache.hc.core5.util.Asserts;
 import org.apache.hc.core5.util.TimeValue;
+import org.apache.hc.core5.util.Timeout;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -200,6 +201,19 @@ public class SSLConnectionSocketFactory implements LayeredConnectionSocketFactor
             final InetSocketAddress remoteAddress,
             final InetSocketAddress localAddress,
             final HttpContext context) throws IOException {
+        final Timeout timeout = connectTimeout != null ? Timeout.of(connectTimeout.getDuration(), connectTimeout.getTimeUnit()) : null;
+        return connectSocket(socket, host, remoteAddress, localAddress, timeout, timeout, context);
+    }
+
+    @Override
+    public Socket connectSocket(
+            final Socket socket,
+            final HttpHost host,
+            final InetSocketAddress remoteAddress,
+            final InetSocketAddress localAddress,
+            final Timeout connectTimeout,
+            final Timeout handshakeTimeout,
+            final HttpContext context) throws IOException {
         Args.notNull(host, "HTTP host");
         Args.notNull(remoteAddress, "Remote address");
         final Socket sock = socket != null ? socket : createSocket(context);
@@ -214,7 +228,7 @@ public class SSLConnectionSocketFactory implements LayeredConnectionSocketFactor
             // only to this library
             try {
                 AccessController.doPrivileged((PrivilegedExceptionAction<Object>) () -> {
-                    sock.connect(remoteAddress, connectTimeout != null ? connectTimeout.toMillisecondsIntBound() : 0);
+                    sock.connect(remoteAddress, Timeout.defaultsToDisabled(connectTimeout).toMillisecondsIntBound());
                     return null;
                 });
             } catch (final PrivilegedActionException e) {
@@ -230,12 +244,19 @@ public class SSLConnectionSocketFactory implements LayeredConnectionSocketFactor
         // Setup SSL layering if necessary
         if (sock instanceof SSLSocket) {
             final SSLSocket sslsock = (SSLSocket) sock;
-            LOG.debug("Starting handshake");
-            sslsock.startHandshake();
-            verifyHostname(sslsock, host.getHostName());
+            executeHandshake(sslsock, host.getHostName(), handshakeTimeout);
             return sock;
         }
-        return createLayeredSocket(sock, host.getHostName(), remoteAddress.getPort(), context);
+        return createLayeredSocket(sock, host.getHostName(), remoteAddress.getPort(), handshakeTimeout, context);
+    }
+
+    @Override
+    public Socket createLayeredSocket(
+            final Socket socket,
+            final String target,
+            final int port,
+            final HttpContext context) throws IOException {
+        return createLayeredSocket(socket, target, port, context);
     }
 
     @Override
@@ -243,12 +264,18 @@ public class SSLConnectionSocketFactory implements LayeredConnectionSocketFactor
             final Socket socket,
             final String target,
             final int port,
+            final Timeout handshakeTimeout,
             final HttpContext context) throws IOException {
         final SSLSocket sslsock = (SSLSocket) this.socketFactory.createSocket(
                 socket,
                 target,
                 port,
                 true);
+        executeHandshake(sslsock, target, handshakeTimeout);
+        return sslsock;
+    }
+
+    private void executeHandshake(final SSLSocket sslsock, final String target, final Timeout handshakeTimeout) throws IOException {
         if (supportedProtocols != null) {
             sslsock.setEnabledProtocols(supportedProtocols);
         } else {
@@ -259,17 +286,19 @@ public class SSLConnectionSocketFactory implements LayeredConnectionSocketFactor
         } else {
             sslsock.setEnabledCipherSuites(TlsCiphers.excludeWeak(sslsock.getEnabledCipherSuites()));
         }
+        if (handshakeTimeout != null) {
+            sslsock.setSoTimeout(handshakeTimeout.toMillisecondsIntBound());
+        }
+
+        prepareSocket(sslsock);
 
         if (LOG.isDebugEnabled()) {
             LOG.debug("Enabled protocols: {}", (Object) sslsock.getEnabledProtocols());
             LOG.debug("Enabled cipher suites: {}", (Object) sslsock.getEnabledCipherSuites());
+            LOG.debug("Starting handshake ({})", handshakeTimeout);
         }
-
-        prepareSocket(sslsock);
-        LOG.debug("Starting handshake");
         sslsock.startHandshake();
         verifyHostname(sslsock, target);
-        return sslsock;
     }
 
     private void verifyHostname(final SSLSocket sslsock, final String hostname) 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 76f199d..a6ac973 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
@@ -30,10 +30,12 @@ package org.apache.hc.client5.http.impl.io;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.Socket;
+import java.util.concurrent.TimeUnit;
 
 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.io.ConnectionEndpoint;
 import org.apache.hc.client5.http.io.LeaseRequest;
 import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
@@ -361,26 +363,52 @@ public class TestBasicHttpClientConnectionManager {
 
         mgr.setSocketConfig(sconfig);
 
+        final ConnectionConfig connectionConfig = ConnectionConfig.custom()
+                .setConnectTimeout(234, TimeUnit.MILLISECONDS)
+                .setHandshakeTimeout(345, TimeUnit.MILLISECONDS)
+                .build();
+        mgr.setConnectionConfig(connectionConfig);
+
         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())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
-                Mockito.any(),
                 Mockito.eq(socket),
                 Mockito.any(),
                 Mockito.any(),
                 Mockito.any(),
+                Mockito.any(),
+                Mockito.any(),
                 Mockito.any())).thenReturn(socket);
 
-        mgr.connect(endpoint1, TimeValue.ofMilliseconds(123), context);
+        mgr.connect(endpoint1, null, context);
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target);
         Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(context);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(TimeValue.ofMilliseconds(123), socket, target,
+        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+                socket,
+                target,
+                new InetSocketAddress(remote, 8443),
+                new InetSocketAddress(local, 0),
+                Timeout.ofMilliseconds(234),
+                Timeout.ofMilliseconds(345),
+                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(context);
+        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+                socket,
+                target,
                 new InetSocketAddress(remote, 8443),
-                new InetSocketAddress(local, 0), context);
+                new InetSocketAddress(local, 0),
+                Timeout.ofMilliseconds(123),
+                Timeout.ofMilliseconds(345),
+                context);
     }
 
     @Test
@@ -402,6 +430,12 @@ public class TestBasicHttpClientConnectionManager {
 
         mgr.setSocketConfig(sconfig);
 
+        final ConnectionConfig connectionConfig = ConnectionConfig.custom()
+                .setConnectTimeout(234, TimeUnit.MILLISECONDS)
+                .setHandshakeTimeout(345, TimeUnit.MILLISECONDS)
+                .build();
+        mgr.setConnectionConfig(connectionConfig);
+
         Mockito.when(dnsResolver.resolve("someproxy")).thenReturn(new InetAddress[] {remote});
         Mockito.when(schemePortResolver.resolve(proxy)).thenReturn(8080);
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
@@ -409,21 +443,27 @@ public class TestBasicHttpClientConnectionManager {
         Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslSocketFactory);
         Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
-                Mockito.any(),
                 Mockito.eq(socket),
                 Mockito.any(),
                 Mockito.any(),
                 Mockito.any(),
+                Mockito.any(),
+                Mockito.any(),
                 Mockito.any())).thenReturn(socket);
 
-        mgr.connect(endpoint1, TimeValue.ofMilliseconds(123), context);
+        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(context);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(TimeValue.ofMilliseconds(123), socket, proxy,
+        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+                socket,
+                proxy,
                 new InetSocketAddress(remote, 8080),
-                new InetSocketAddress(local, 0), context);
+                new InetSocketAddress(local, 0),
+                Timeout.ofMilliseconds(234),
+                Timeout.ofMilliseconds(345),
+                context);
 
         Mockito.when(conn.getSocket()).thenReturn(socket);
 
@@ -431,7 +471,7 @@ public class TestBasicHttpClientConnectionManager {
 
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target);
         Mockito.verify(sslSocketFactory, Mockito.times(1)).createLayeredSocket(
-                socket, "somehost", 8443, context);
+                socket, "somehost", 8443, Timeout.ofMilliseconds(345), 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 34bb77a..c860bf3 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
@@ -48,6 +48,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.util.TimeValue;
+import org.apache.hc.core5.util.Timeout;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -96,6 +97,7 @@ public class TestHttpClientConnectionOperator {
                 Mockito.any(),
                 Mockito.any(),
                 Mockito.any(),
+                Mockito.any(),
                 Mockito.any())).thenReturn(socket);
 
         final SocketConfig socketConfig = SocketConfig.custom()
@@ -106,7 +108,8 @@ public class TestHttpClientConnectionOperator {
             .setSoLinger(50, TimeUnit.MILLISECONDS)
             .build();
         final InetSocketAddress localAddress = new InetSocketAddress(local, 0);
-        connectionOperator.connect(conn, host, localAddress, TimeValue.ofMilliseconds(1000), socketConfig, context);
+        connectionOperator.connect(conn, host, localAddress,
+                Timeout.ofMilliseconds(123), Timeout.ofMilliseconds(234), socketConfig, context);
 
         Mockito.verify(socket).setKeepAlive(true);
         Mockito.verify(socket).setReuseAddress(true);
@@ -115,11 +118,12 @@ public class TestHttpClientConnectionOperator {
         Mockito.verify(socket).setTcpNoDelay(true);
 
         Mockito.verify(plainSocketFactory).connectSocket(
-                TimeValue.ofMilliseconds(1000),
                 socket,
                 host,
                 new InetSocketAddress(ip1, 80),
                 localAddress,
+                Timeout.ofMilliseconds(123),
+                Timeout.ofMilliseconds(234),
                 context);
         Mockito.verify(conn, Mockito.times(2)).bind(socket);
     }
@@ -141,6 +145,7 @@ public class TestHttpClientConnectionOperator {
                 Mockito.any(),
                 Mockito.any(),
                 Mockito.any(),
+                Mockito.any(),
                 Mockito.any())).thenThrow(new SocketTimeoutException());
 
         Assert.assertThrows(ConnectTimeoutException.class, () ->
@@ -165,6 +170,7 @@ public class TestHttpClientConnectionOperator {
                 Mockito.any(),
                 Mockito.any(),
                 Mockito.any(),
+                Mockito.any(),
                 Mockito.any())).thenThrow(new ConnectException());
 
         Assert.assertThrows(HttpHostConnectException.class, () ->
@@ -187,27 +193,31 @@ public class TestHttpClientConnectionOperator {
         Mockito.when(plainSocketFactory.connectSocket(
                 Mockito.any(),
                 Mockito.any(),
-                Mockito.any(),
                 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.any(),
                 Mockito.eq(new InetSocketAddress(ip2, 80)),
                 Mockito.any(),
+                Mockito.any(),
+                Mockito.any(),
                 Mockito.any())).thenReturn(socket);
 
         final InetSocketAddress localAddress = new InetSocketAddress(local, 0);
-        connectionOperator.connect(conn, host, localAddress, TimeValue.ofMilliseconds(1000), SocketConfig.DEFAULT, context);
+        connectionOperator.connect(conn, host, localAddress,
+                Timeout.ofMilliseconds(123), Timeout.ofMilliseconds(234), SocketConfig.DEFAULT, context);
 
         Mockito.verify(plainSocketFactory).connectSocket(
-                TimeValue.ofMilliseconds(1000),
                 socket,
                 host,
                 new InetSocketAddress(ip2, 80),
                 localAddress,
+                Timeout.ofMilliseconds(123),
+                Timeout.ofMilliseconds(234),
                 context);
         Mockito.verify(conn, Mockito.times(3)).bind(socket);
     }
@@ -228,17 +238,20 @@ public class TestHttpClientConnectionOperator {
                 Mockito.any(),
                 Mockito.any(),
                 Mockito.any(),
+                Mockito.any(),
                 Mockito.any())).thenReturn(socket);
 
         final InetSocketAddress localAddress = new InetSocketAddress(local, 0);
-        connectionOperator.connect(conn, host, localAddress, TimeValue.ofMilliseconds(1000), SocketConfig.DEFAULT, context);
+        connectionOperator.connect(conn, host, localAddress,
+                Timeout.ofMilliseconds(123), Timeout.ofMilliseconds(234), SocketConfig.DEFAULT, context);
 
         Mockito.verify(plainSocketFactory).connectSocket(
-                TimeValue.ofMilliseconds(1000),
                 socket,
                 host,
                 new InetSocketAddress(ip, 80),
                 localAddress,
+                Timeout.ofMilliseconds(123),
+                Timeout.ofMilliseconds(234),
                 context);
         Mockito.verify(dnsResolver, Mockito.never()).resolve(Mockito.anyString());
         Mockito.verify(conn, Mockito.times(2)).bind(socket);
@@ -258,9 +271,10 @@ public class TestHttpClientConnectionOperator {
                 Mockito.any(),
                 Mockito.eq("somehost"),
                 Mockito.eq(443),
+                Mockito.eq(Timeout.ofMilliseconds(345)),
                 Mockito.any())).thenReturn(socket);
 
-        connectionOperator.upgrade(conn, host, context);
+        connectionOperator.upgrade(conn, host, Timeout.ofMilliseconds(345), context);
 
         Mockito.verify(conn).bind(socket);
     }
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 29dbdd9..a24c5c8 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
@@ -37,6 +37,7 @@ import java.util.concurrent.TimeoutException;
 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.io.ConnectionEndpoint;
 import org.apache.hc.client5.http.io.LeaseRequest;
 import org.apache.hc.client5.http.io.ManagedHttpClientConnection;
@@ -247,26 +248,52 @@ public class TestPoolingHttpClientConnectionManager {
 
         mgr.setDefaultSocketConfig(sconfig);
 
+        final ConnectionConfig connectionConfig = ConnectionConfig.custom()
+                .setConnectTimeout(234, TimeUnit.MILLISECONDS)
+                .setHandshakeTimeout(345, TimeUnit.MILLISECONDS)
+                .build();
+        mgr.setDefaultConnectionConfig(connectionConfig);
+
         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())).thenReturn(socket);
         Mockito.when(plainSocketFactory.connectSocket(
-                Mockito.any(),
                 Mockito.eq(socket),
                 Mockito.any(),
                 Mockito.any(),
                 Mockito.any(),
+                Mockito.any(),
+                Mockito.any(),
                 Mockito.any())).thenReturn(socket);
 
-        mgr.connect(endpoint1, TimeValue.ofMilliseconds(123), context);
+        mgr.connect(endpoint1, null, context);
 
         Mockito.verify(dnsResolver, Mockito.times(1)).resolve("somehost");
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target);
         Mockito.verify(plainSocketFactory, Mockito.times(1)).createSocket(context);
-        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(TimeValue.ofMilliseconds(123), socket, target,
+        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+                socket,
+                target,
+                new InetSocketAddress(remote, 8443),
+                new InetSocketAddress(local, 0),
+                Timeout.ofMilliseconds(234),
+                Timeout.ofMilliseconds(345),
+                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(context);
+        Mockito.verify(plainSocketFactory, Mockito.times(1)).connectSocket(
+                socket,
+                target,
                 new InetSocketAddress(remote, 8443),
-                new InetSocketAddress(local, 0), context);
+                new InetSocketAddress(local, 0),
+                Timeout.ofMilliseconds(123),
+                Timeout.ofMilliseconds(345),
+                context);
     }
 
     @Test
@@ -301,6 +328,12 @@ public class TestPoolingHttpClientConnectionManager {
 
         mgr.setDefaultSocketConfig(sconfig);
 
+        final ConnectionConfig connectionConfig = ConnectionConfig.custom()
+                .setConnectTimeout(234, TimeUnit.MILLISECONDS)
+                .setHandshakeTimeout(345, TimeUnit.MILLISECONDS)
+                .build();
+        mgr.setDefaultConnectionConfig(connectionConfig);
+
         Mockito.when(dnsResolver.resolve("someproxy")).thenReturn(new InetAddress[] {remote});
         Mockito.when(schemePortResolver.resolve(proxy)).thenReturn(8080);
         Mockito.when(schemePortResolver.resolve(target)).thenReturn(8443);
@@ -308,21 +341,27 @@ public class TestPoolingHttpClientConnectionManager {
         Mockito.when(socketFactoryRegistry.lookup("https")).thenReturn(sslsf);
         Mockito.when(plainsf.createSocket(Mockito.any())).thenReturn(mockSock);
         Mockito.when(plainsf.connectSocket(
-                Mockito.any(),
                 Mockito.eq(mockSock),
                 Mockito.any(),
                 Mockito.any(),
                 Mockito.any(),
+                Mockito.any(),
+                Mockito.any(),
                 Mockito.any())).thenReturn(mockSock);
 
-        mgr.connect(endpoint1, TimeValue.ofMilliseconds(123), context);
+        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(context);
-        Mockito.verify(plainsf, Mockito.times(1)).connectSocket(TimeValue.ofMilliseconds(123), mockSock, proxy,
+        Mockito.verify(plainsf, Mockito.times(1)).connectSocket(
+                mockSock,
+                proxy,
                 new InetSocketAddress(remote, 8080),
-                new InetSocketAddress(local, 0), context);
+                new InetSocketAddress(local, 0),
+                Timeout.ofMilliseconds(234),
+                Timeout.ofMilliseconds(345),
+                context);
 
         Mockito.when(conn.isOpen()).thenReturn(true);
         Mockito.when(conn.getSocket()).thenReturn(mockSock);
@@ -331,7 +370,7 @@ public class TestPoolingHttpClientConnectionManager {
 
         Mockito.verify(schemePortResolver, Mockito.times(1)).resolve(target);
         Mockito.verify(sslsf, Mockito.times(1)).createLayeredSocket(
-                mockSock, "somehost", 8443, context);
+                mockSock, "somehost", 8443, Timeout.ofMilliseconds(345), context);
     }
 
 }

[httpcomponents-client] 02/03: Better connect operation logging

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit da5168f28f212db996e48a8bafff3ab3490ec821
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Thu Sep 16 17:20:52 2021 +0200

    Better connect operation logging
---
 .../impl/io/BasicHttpClientConnectionManager.java  |  6 +++
 .../io/DefaultHttpClientConnectionOperator.java    | 44 ++++++++++++++++------
 .../http/impl/nio/MultihomeIOSessionRequester.java | 21 ++++++-----
 3 files changed, 51 insertions(+), 20 deletions(-)

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 a529dfb..b9866c1 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
@@ -361,6 +361,9 @@ public class BasicHttpClientConnectionManager implements HttpClientConnectionMan
         final ConnectionConfig config = connectionConfig != null ? connectionConfig : ConnectionConfig.DEFAULT;
         final TimeValue connectTimeout = timeout != null ? timeout : config.getConnectTimeout();
         final ManagedHttpClientConnection connection = internalEndpoint.getConnection();
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("{} connecting endpoint to {} ({})", ConnPoolSupport.getId(endpoint), host, connectTimeout);
+        }
         this.connectionOperator.connect(
                 connection,
                 host,
@@ -368,6 +371,9 @@ public class BasicHttpClientConnectionManager implements HttpClientConnectionMan
                 connectTimeout,
                 this.socketConfig,
                 context);
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("{} connected {}", ConnPoolSupport.getId(endpoint), ConnPoolSupport.getId(conn));
+        }
         final Timeout socketTimeout = config.getSocketTimeout();
         if (socketTimeout != null) {
             connection.setSocketTimeout(socketTimeout);
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 14a45f0..926f588 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
@@ -30,6 +30,7 @@ import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.Socket;
+import java.util.Arrays;
 
 import org.apache.hc.client5.http.ConnectExceptionSupport;
 import org.apache.hc.client5.http.DnsResolver;
@@ -115,12 +116,25 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection
         if (sf == null) {
             throw new UnsupportedSchemeException(host.getSchemeName() + " protocol is not supported");
         }
-        final InetAddress[] addresses = host.getAddress() != null ?
-                new InetAddress[] { host.getAddress() } : this.dnsResolver.resolve(host.getHostName());
+        final InetAddress[] remoteAddresses;
+        if (host.getAddress() != null) {
+            remoteAddresses = new InetAddress[] { host.getAddress() };
+        } else {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} resolving remote address", host.getHostName());
+            }
+
+            remoteAddresses = this.dnsResolver.resolve(host.getHostName());
+
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("{} resolved to {}", host.getHostName(), Arrays.asList(remoteAddresses));
+            }
+        }
+
         final int port = this.schemePortResolver.resolve(host);
-        for (int i = 0; i < addresses.length; i++) {
-            final InetAddress address = addresses[i];
-            final boolean last = i == addresses.length - 1;
+        for (int i = 0; i < remoteAddresses.length; i++) {
+            final InetAddress address = remoteAddresses[i];
+            final boolean last = i == remoteAddresses.length - 1;
 
             Socket sock = sf.createSocket(context);
             sock.setSoTimeout(socketConfig.getSoTimeout().toMillisecondsIntBound());
@@ -142,23 +156,31 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection
 
             final InetSocketAddress remoteAddress = new InetSocketAddress(address, port);
             if (LOG.isDebugEnabled()) {
-                LOG.debug("{} connecting to {}", ConnPoolSupport.getId(conn), remoteAddress);
+                LOG.debug("{}:{} connecting {}->{} ({})",
+                        host.getHostName(), host.getPort(), localAddress, remoteAddress, connectTimeout);
             }
             try {
                 sock = sf.connectSocket(connectTimeout, sock, host, remoteAddress, localAddress, context);
                 conn.bind(sock);
                 if (LOG.isDebugEnabled()) {
-                    LOG.debug("{} connection established {}", ConnPoolSupport.getId(conn), conn);
+                    LOG.debug("{}:{} connected {}->{} as {}",
+                            host.getHostName(), host.getPort(), localAddress, remoteAddress, ConnPoolSupport.getId(conn));
                 }
                 return;
             } catch (final IOException ex) {
                 if (last) {
-                    throw ConnectExceptionSupport.enhance(ex, host, addresses);
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("{}:{} connection to {} failed ({}); terminating operation",
+                                host.getHostName(), host.getPort(), remoteAddress, ex.getClass());
+                    }
+                    throw ConnectExceptionSupport.enhance(ex, host, remoteAddresses);
+                } else {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("{}:{} connection to {} failed ({}); retrying connection to the next address",
+                                host.getHostName(), host.getPort(), remoteAddress, ex.getClass());
+                    }
                 }
             }
-            if (LOG.isDebugEnabled()) {
-                LOG.debug("{} connect to {} timed out. Connection will be retried using another IP address", ConnPoolSupport.getId(conn), remoteAddress);
-            }
         }
     }
 
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/MultihomeIOSessionRequester.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/MultihomeIOSessionRequester.java
index 4d90b85..bb4264b 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/MultihomeIOSessionRequester.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/MultihomeIOSessionRequester.java
@@ -68,13 +68,14 @@ final class MultihomeIOSessionRequester {
 
         if (remoteAddress != null) {
             if (LOG.isDebugEnabled()) {
-                LOG.debug("{} connecting {} to {} ({})", remoteEndpoint, localAddress, remoteAddress, connectTimeout);
+                LOG.debug("{}:{} connecting {} to {} ({})",
+                        remoteEndpoint.getHostName(), remoteEndpoint.getPort(), localAddress, remoteAddress, connectTimeout);
             }
             return connectionInitiator.connect(remoteEndpoint, remoteAddress, localAddress, connectTimeout, attachment, callback);
         }
 
         if (LOG.isDebugEnabled()) {
-            LOG.debug("{} resolving remote address", remoteEndpoint);
+            LOG.debug("{} resolving remote address", remoteEndpoint.getHostName());
         }
 
         final ComplexFuture<IOSession> future = new ComplexFuture<>(callback);
@@ -87,7 +88,7 @@ final class MultihomeIOSessionRequester {
         }
 
         if (LOG.isDebugEnabled()) {
-            LOG.debug("{} resolved to {}", remoteEndpoint, Arrays.asList(remoteAddresses));
+            LOG.debug("{} resolved to {}", remoteEndpoint.getHostName(), Arrays.asList(remoteAddresses));
         }
 
         final Runnable runnable = new Runnable() {
@@ -99,7 +100,8 @@ final class MultihomeIOSessionRequester {
                 final InetSocketAddress remoteAddress = new InetSocketAddress(remoteAddresses[index], remoteEndpoint.getPort());
 
                 if (LOG.isDebugEnabled()) {
-                    LOG.debug("{} connecting {} to {} ({})", remoteEndpoint, localAddress, remoteAddress, connectTimeout);
+                    LOG.debug("{}:{} connecting {}->{} ({})",
+                            remoteEndpoint.getHostName(), remoteEndpoint.getPort(), localAddress, remoteAddress, connectTimeout);
                 }
 
                 final Future<IOSession> sessionFuture = connectionInitiator.connect(
@@ -113,9 +115,8 @@ final class MultihomeIOSessionRequester {
                             @Override
                             public void completed(final IOSession session) {
                                 if (LOG.isDebugEnabled()) {
-                                    if (LOG.isDebugEnabled()) {
-                                        LOG.debug("{} connected {} {}->{}", remoteEndpoint, session.getId(), session.getLocalAddress(), session.getRemoteAddress());
-                                    }
+                                    LOG.debug("{}:{} connected {}->{} as {}",
+                                            remoteEndpoint.getHostName(), remoteEndpoint.getPort(), localAddress, remoteAddress, session.getId());
                                 }
                                 future.completed(session);
                             }
@@ -124,7 +125,8 @@ final class MultihomeIOSessionRequester {
                             public void failed(final Exception cause) {
                                 if (attempt.get() >= remoteAddresses.length) {
                                     if (LOG.isDebugEnabled()) {
-                                        LOG.debug("{} connection to {} failed ({}); terminating operation", remoteEndpoint, remoteAddress, cause.getClass());
+                                        LOG.debug("{}:{} connection to {} failed ({}); terminating operation",
+                                                remoteEndpoint.getHostName(), remoteEndpoint.getPort(), remoteAddress, cause.getClass());
                                     }
                                     if (cause instanceof IOException) {
                                         future.failed(ConnectExceptionSupport.enhance((IOException) cause, remoteEndpoint, remoteAddresses));
@@ -133,7 +135,8 @@ final class MultihomeIOSessionRequester {
                                     }
                                 } else {
                                     if (LOG.isDebugEnabled()) {
-                                        LOG.debug("{} connection to {} failed ({}); retrying connection to the next address", remoteEndpoint, remoteAddress, cause.getClass());
+                                        LOG.debug("{}:{} connection to {} failed ({}); retrying connection to the next address",
+                                                remoteEndpoint.getHostName(), remoteEndpoint.getPort(), remoteAddress, cause.getClass());
                                     }
                                     executeNext();
                                 }

[httpcomponents-client] 01/03: HTTPCLIENT-2045: BASIC auth scheme conformance to RFC 7617

Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit 50f93ec18be8d6f49138825356051c4c0b60dce4
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Mon Sep 13 19:15:22 2021 +0200

    HTTPCLIENT-2045: BASIC auth scheme conformance to RFC 7617
---
 .../client5/http/impl/auth/AuthSchemeSupport.java  |  50 +++++++++
 .../hc/client5/http/impl/auth/BasicScheme.java     |  54 +++++++---
 .../hc/client5/http/impl/auth/DigestScheme.java    |  10 +-
 .../hc/client5/http/impl/auth/TestBasicScheme.java | 112 +++++++++++++++------
 4 files changed, 169 insertions(+), 57 deletions(-)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthSchemeSupport.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthSchemeSupport.java
new file mode 100644
index 0000000..a659351
--- /dev/null
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthSchemeSupport.java
@@ -0,0 +1,50 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.hc.client5.http.impl.auth;
+
+import java.nio.charset.Charset;
+import java.nio.charset.UnsupportedCharsetException;
+
+import org.apache.hc.client5.http.auth.AuthenticationException;
+import org.apache.hc.core5.annotation.Internal;
+
+/**
+ * @since 5.2
+ */
+@Internal
+public class AuthSchemeSupport {
+
+    public static Charset parseCharset(
+            final String charsetName, final Charset defaultCharset) throws AuthenticationException {
+        try {
+            return charsetName != null ? Charset.forName(charsetName) : defaultCharset;
+        } catch (final UnsupportedCharsetException ex) {
+            throw new AuthenticationException("Unsupported charset: " + charsetName);
+        }
+    }
+
+}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/BasicScheme.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/BasicScheme.java
index 438cc68..6826029 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/BasicScheme.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/BasicScheme.java
@@ -42,13 +42,13 @@ import java.util.Map;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.hc.client5.http.auth.AuthChallenge;
 import org.apache.hc.client5.http.auth.AuthScheme;
-import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.AuthScope;
 import org.apache.hc.client5.http.auth.AuthStateCacheable;
 import org.apache.hc.client5.http.auth.AuthenticationException;
 import org.apache.hc.client5.http.auth.Credentials;
 import org.apache.hc.client5.http.auth.CredentialsProvider;
 import org.apache.hc.client5.http.auth.MalformedChallengeException;
+import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.protocol.HttpClientContext;
 import org.apache.hc.client5.http.utils.ByteArrayBuilder;
 import org.apache.hc.core5.http.HttpHost;
@@ -72,7 +72,7 @@ public class BasicScheme implements AuthScheme, Serializable {
     private static final Logger LOG = LoggerFactory.getLogger(BasicScheme.class);
 
     private final Map<String, String> paramMap;
-    private transient Charset charset;
+    private transient Charset defaultCharset;
     private transient ByteArrayBuilder buffer;
     private transient Base64 base64codec;
     private boolean complete;
@@ -85,7 +85,7 @@ public class BasicScheme implements AuthScheme, Serializable {
      */
     public BasicScheme(final Charset charset) {
         this.paramMap = new HashMap<>();
-        this.charset = charset != null ? charset : StandardCharsets.US_ASCII;
+        this.defaultCharset = charset != null ? charset : StandardCharsets.US_ASCII;
         this.complete = false;
     }
 
@@ -93,13 +93,21 @@ public class BasicScheme implements AuthScheme, Serializable {
         this(StandardCharsets.US_ASCII);
     }
 
+    private void applyCredentials(final Credentials credentials) {
+        this.username = credentials.getUserPrincipal().getName();
+        this.password = credentials.getPassword();
+    }
+
+    private void clearCredentials() {
+        this.username = null;
+        this.password = null;
+    }
+
     public void initPreemptive(final Credentials credentials) {
         if (credentials != null) {
-            this.username = credentials.getUserPrincipal().getName();
-            this.password = credentials.getPassword();
+            applyCredentials(credentials);
         } else {
-            this.username = null;
-            this.password = null;
+            clearCredentials();
         }
     }
 
@@ -150,8 +158,7 @@ public class BasicScheme implements AuthScheme, Serializable {
         final Credentials credentials = credentialsProvider.getCredentials(
                 authScope, context);
         if (credentials != null) {
-            this.username = credentials.getUserPrincipal().getName();
-            this.password = credentials.getPassword();
+            applyCredentials(credentials);
             return true;
         }
 
@@ -160,8 +167,7 @@ public class BasicScheme implements AuthScheme, Serializable {
             final String exchangeId = clientContext.getExchangeId();
             LOG.debug("{} No credentials found for auth scope [{}]", exchangeId, authScope);
         }
-        this.username = null;
-        this.password = null;
+        clearCredentials();
         return false;
     }
 
@@ -170,16 +176,34 @@ public class BasicScheme implements AuthScheme, Serializable {
         return null;
     }
 
+    private void validateUsername() throws AuthenticationException {
+        if (username == null) {
+            throw new AuthenticationException("User credentials not set");
+        }
+        for (int i = 0; i < username.length(); i++) {
+            final char ch = username.charAt(i);
+            if (Character.isISOControl(ch)) {
+                throw new AuthenticationException("Username must not contain any control characters");
+            }
+            if (ch == ':') {
+                throw new AuthenticationException("Username contains a colon character and is invalid");
+            }
+        }
+    }
+
     @Override
     public String generateAuthResponse(
             final HttpHost host,
             final HttpRequest request,
             final HttpContext context) throws AuthenticationException {
+        validateUsername();
         if (this.buffer == null) {
-            this.buffer = new ByteArrayBuilder(64).charset(this.charset);
+            this.buffer = new ByteArrayBuilder(64);
         } else {
             this.buffer.reset();
         }
+        final Charset charset = AuthSchemeSupport.parseCharset(paramMap.get("charset"), defaultCharset);
+        this.buffer.charset(charset);
         this.buffer.append(this.username).append(":").append(this.password);
         if (this.base64codec == null) {
             this.base64codec = new Base64(0);
@@ -191,16 +215,16 @@ public class BasicScheme implements AuthScheme, Serializable {
 
     private void writeObject(final ObjectOutputStream out) throws IOException {
         out.defaultWriteObject();
-        out.writeUTF(this.charset.name());
+        out.writeUTF(this.defaultCharset.name());
     }
 
     @SuppressWarnings("unchecked")
     private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException {
         in.defaultReadObject();
         try {
-            this.charset = Charset.forName(in.readUTF());
+            this.defaultCharset = Charset.forName(in.readUTF());
         } catch (final UnsupportedCharsetException ex) {
-            this.charset = StandardCharsets.US_ASCII;
+            this.defaultCharset = StandardCharsets.US_ASCII;
         }
     }
 
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java
index ab6eabd..812eaee 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java
@@ -32,7 +32,6 @@ import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
-import java.nio.charset.UnsupportedCharsetException;
 import java.security.MessageDigest;
 import java.security.Principal;
 import java.security.SecureRandom;
@@ -275,14 +274,7 @@ public class DigestScheme implements AuthScheme, Serializable {
             throw new AuthenticationException("None of the qop methods is supported: " + qoplist);
         }
 
-        final String charsetName = this.paramMap.get("charset");
-        final Charset charset;
-        try {
-            charset = charsetName != null ? Charset.forName(charsetName) : defaultCharset;
-        } catch (final UnsupportedCharsetException ex) {
-            throw new AuthenticationException("Unsupported charset: " + charsetName);
-        }
-
+        final Charset charset = AuthSchemeSupport.parseCharset(paramMap.get("charset"), defaultCharset);
         String digAlg = algorithm;
         if (digAlg.equalsIgnoreCase("MD5-sess")) {
             digAlg = "MD5";
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestBasicScheme.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestBasicScheme.java
index 9115aad..de2f4ce 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestBasicScheme.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestBasicScheme.java
@@ -36,9 +36,10 @@ import java.util.List;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.hc.client5.http.auth.AuthChallenge;
 import org.apache.hc.client5.http.auth.AuthScheme;
-import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.AuthScope;
+import org.apache.hc.client5.http.auth.AuthenticationException;
 import org.apache.hc.client5.http.auth.ChallengeType;
+import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
 import org.apache.hc.core5.http.HttpHost;
 import org.apache.hc.core5.http.HttpRequest;
@@ -73,29 +74,8 @@ public class TestBasicScheme {
     }
 
     @Test
-    public void testBasicAuthenticationWith88591Chars() throws Exception {
-        final int[] germanChars = { 0xE4, 0x2D, 0xF6, 0x2D, 0xFc };
-        final StringBuilder buffer = new StringBuilder();
-        for (final int germanChar : germanChars) {
-            buffer.append((char)germanChar);
-        }
-
-        final HttpHost host  = new HttpHost("somehost", 80);
-        final AuthScope authScope = new AuthScope(host, "some realm", null);
-        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("dh", buffer.toString().toCharArray());
-        final BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();
-        credentialsProvider.setCredentials(authScope, creds);
-        final BasicScheme authscheme = new BasicScheme(StandardCharsets.ISO_8859_1);
-
-        Assert.assertTrue(authscheme.isResponseReady(host, credentialsProvider, null));
-        final HttpRequest request = new BasicHttpRequest("GET", "/");
-        final String authResponse = authscheme.generateAuthResponse(host, request, null);
-        Assert.assertEquals(StandardAuthScheme.BASIC + " ZGg65C32Lfw=", authResponse);
-    }
-
-    @Test
     public void testBasicAuthentication() throws Exception {
-        final AuthChallenge authChallenge = parse(StandardAuthScheme.BASIC + " realm=\"test\"");
+        final AuthChallenge authChallenge = parse("Basic realm=\"test\"");
 
         final BasicScheme authscheme = new BasicScheme();
         authscheme.processChallenge(authChallenge, null);
@@ -110,7 +90,7 @@ public class TestBasicScheme {
         Assert.assertTrue(authscheme.isResponseReady(host, credentialsProvider, null));
         final String authResponse = authscheme.generateAuthResponse(host, request, null);
 
-        final String expected = StandardAuthScheme.BASIC + " " + new String(
+        final String expected = "Basic " + new String(
                 Base64.encodeBase64("testuser:testpass".getBytes(StandardCharsets.US_ASCII)),
                 StandardCharsets.US_ASCII);
         Assert.assertEquals(expected, authResponse);
@@ -119,27 +99,58 @@ public class TestBasicScheme {
         Assert.assertFalse(authscheme.isConnectionBased());
     }
 
+    static final String TEST_UTF8_PASSWORD = "123\u00A3";
+
+    @Test
+    public void testBasicAuthenticationDefaultCharsetASCII() throws Exception {
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray());
+        final BasicScheme authscheme = new BasicScheme(StandardCharsets.US_ASCII);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(creds);
+        final String authResponse = authscheme.generateAuthResponse(host, request, null);
+        Assert.assertEquals("Basic dGVzdDoxMjM/", authResponse);
+    }
+
+    @Test
+    public void testBasicAuthenticationDefaultCharsetISO88591() throws Exception {
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray());
+        final BasicScheme authscheme = new BasicScheme(StandardCharsets.ISO_8859_1);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(creds);
+        final String authResponse = authscheme.generateAuthResponse(host, request, null);
+        Assert.assertEquals("Basic dGVzdDoxMjOj", authResponse);
+    }
+
     @Test
-    public void testBasicProxyAuthentication() throws Exception {
-        final AuthChallenge authChallenge = parse(StandardAuthScheme.BASIC + " realm=\"test\"");
+    public void testBasicAuthenticationDefaultCharsetUTF8() throws Exception {
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray());
+        final BasicScheme authscheme = new BasicScheme(StandardCharsets.UTF_8);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(creds);
+        final String authResponse = authscheme.generateAuthResponse(host, request, null);
+        Assert.assertEquals("Basic dGVzdDoxMjPCow==", authResponse);
+    }
+
+    @Test
+    public void testBasicAuthenticationWithCharset() throws Exception {
+        final AuthChallenge authChallenge = parse("Basic realm=\"test\", charset=\"utf-8\"");
 
         final BasicScheme authscheme = new BasicScheme();
         authscheme.processChallenge(authChallenge, null);
 
         final HttpHost host  = new HttpHost("somehost", 80);
         final AuthScope authScope = new AuthScope(host, "test", null);
-        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("testuser", "testpass".toCharArray());
+        final UsernamePasswordCredentials creds = new UsernamePasswordCredentials("test", TEST_UTF8_PASSWORD.toCharArray());
         final BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();
         credentialsProvider.setCredentials(authScope, creds);
 
         final HttpRequest request = new BasicHttpRequest("GET", "/");
         Assert.assertTrue(authscheme.isResponseReady(host, credentialsProvider, null));
         final String authResponse = authscheme.generateAuthResponse(host, request, null);
-
-        final String expected = StandardAuthScheme.BASIC + " " + new String(
-                Base64.encodeBase64("testuser:testpass".getBytes(StandardCharsets.US_ASCII)),
-                StandardCharsets.US_ASCII);
-        Assert.assertEquals(expected, authResponse);
+        Assert.assertEquals("Basic dGVzdDoxMjPCow==", authResponse);
         Assert.assertEquals("test", authscheme.getRealm());
         Assert.assertTrue(authscheme.isChallengeComplete());
         Assert.assertFalse(authscheme.isConnectionBased());
@@ -147,7 +158,7 @@ public class TestBasicScheme {
 
     @Test
     public void testSerialization() throws Exception {
-        final AuthChallenge authChallenge = parse(StandardAuthScheme.BASIC + " realm=\"test\"");
+        final AuthChallenge authChallenge = parse("Basic realm=\"test\"");
 
         final BasicScheme basicScheme = new BasicScheme();
         basicScheme.processChallenge(authChallenge, null);
@@ -182,4 +193,39 @@ public class TestBasicScheme {
         Assert.assertEquals(basicScheme.isChallengeComplete(), authScheme.isChallengeComplete());
     }
 
+    @Test
+    public void testBasicAuthenticationUserCredentialsMissing() throws Exception {
+        final BasicScheme authscheme = new BasicScheme();
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        Assert.assertThrows(AuthenticationException.class, () -> authscheme.generateAuthResponse(host, request, null));
+    }
+
+    @Test
+    public void testBasicAuthenticationUsernameWithBlank() throws Exception {
+        final BasicScheme authscheme = new BasicScheme();
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(new UsernamePasswordCredentials("blah blah", null));
+        authscheme.generateAuthResponse(host, request, null);
+    }
+
+    @Test
+    public void testBasicAuthenticationUsernameWithTab() throws Exception {
+        final BasicScheme authscheme = new BasicScheme();
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(new UsernamePasswordCredentials("blah\tblah", null));
+        Assert.assertThrows(AuthenticationException.class, () -> authscheme.generateAuthResponse(host, request, null));
+    }
+
+    @Test
+    public void testBasicAuthenticationUsernameWithColon() throws Exception {
+        final BasicScheme authscheme = new BasicScheme();
+        final HttpHost host  = new HttpHost("somehost", 80);
+        final HttpRequest request = new BasicHttpRequest("GET", "/");
+        authscheme.initPreemptive(new UsernamePasswordCredentials("blah:blah", null));
+        Assert.assertThrows(AuthenticationException.class, () -> authscheme.generateAuthResponse(host, request, null));
+    }
+
 }