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/18 08:30:04 UTC

[httpcomponents-client] branch 5.0.x updated (99f2144 -> b95b917)

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

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


    from 99f2144  HTTPCLIENT-2170: Classic protocol layer no longer releases the underlying connection back to the pool prematurely while the NTLM handshake is still ongoing
     new 2af0bab  HTTPCLIENT-2177: keep successful tunnel connections alive regardless of `Connection: close`
     new d7aa782  HTTPCLIENT-2177: fixed incorrect route state tracking by the async connect executor when negotiating a tunnel via a proxy
     new b95b917  HTTPCLIENT-2177: automatically force HTTP/1.1 protocol policy when executing requests via a proxy tunnel

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:
 ...a => DefaultClientConnectionReuseStrategy.java} | 25 ++++++-----
 .../client5/http/impl/async/AsyncConnectExec.java  | 51 +++++++++++++---------
 .../http/impl/async/HttpAsyncClientBuilder.java    |  8 ++--
 .../async/HttpAsyncClientEventHandlerFactory.java  |  4 +-
 .../client5/http/impl/async/HttpAsyncClients.java  |  4 +-
 .../async/InternalAbstractHttpAsyncClient.java     |  5 ++-
 .../http/impl/async/InternalH2AsyncClient.java     |  3 +-
 .../http/impl/async/InternalHttpAsyncClient.java   | 13 +++++-
 .../hc/client5/http/impl/classic/ConnectExec.java  | 23 +++++-----
 .../http/impl/classic/HttpClientBuilder.java       |  8 ++--
 .../http/impl/classic/MinimalHttpClient.java       |  4 +-
 .../hc/client5/http/impl/classic/ProxyClient.java  |  6 +--
 .../client5/http/impl/classic/TestConnectExec.java |  8 ++--
 13 files changed, 93 insertions(+), 69 deletions(-)
 copy httpclient5/src/main/java/org/apache/hc/client5/http/impl/{NoopUserTokenHandler.java => DefaultClientConnectionReuseStrategy.java} (64%)

[httpcomponents-client] 03/03: HTTPCLIENT-2177: automatically force HTTP/1.1 protocol policy when executing requests via a proxy tunnel

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

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

commit b95b917cb2ba39773e9b3114cd873e8d9f95002d
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Sep 17 22:38:14 2021 +0200

    HTTPCLIENT-2177: automatically force HTTP/1.1 protocol policy when executing requests via a proxy tunnel
---
 .../http/impl/async/InternalAbstractHttpAsyncClient.java    |  5 +++--
 .../hc/client5/http/impl/async/InternalH2AsyncClient.java   |  3 ++-
 .../hc/client5/http/impl/async/InternalHttpAsyncClient.java | 13 +++++++++++--
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java
index ca9ffc0..57aec3d 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalAbstractHttpAsyncClient.java
@@ -142,7 +142,8 @@ abstract class InternalAbstractHttpAsyncClient extends AbstractHttpAsyncClientBa
         }
     }
 
-    abstract AsyncExecRuntime createAsyncExecRuntime(HandlerFactory<AsyncPushConsumer> pushHandlerFactory);
+    abstract AsyncExecRuntime createAsyncExecRuntime(
+            HandlerFactory<AsyncPushConsumer> pushHandlerFactory, HttpRoute route);
 
     abstract HttpRoute determineRoute(HttpHost httpHost, HttpClientContext clientContext) throws HttpException;
 
@@ -182,7 +183,7 @@ abstract class InternalAbstractHttpAsyncClient extends AbstractHttpAsyncClientBa
                     if (LOG.isDebugEnabled()) {
                         LOG.debug("{}: preparing request execution", exchangeId);
                     }
-                    final AsyncExecRuntime execRuntime = createAsyncExecRuntime(pushHandlerFactory);
+                    final AsyncExecRuntime execRuntime = createAsyncExecRuntime(pushHandlerFactory, route);
 
                     setupContext(clientContext);
 
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalH2AsyncClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalH2AsyncClient.java
index 6ebaf66..6841e27 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalH2AsyncClient.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalH2AsyncClient.java
@@ -90,7 +90,8 @@ public final class InternalH2AsyncClient extends InternalAbstractHttpAsyncClient
     }
 
     @Override
-    AsyncExecRuntime createAsyncExecRuntime(final HandlerFactory<AsyncPushConsumer> pushHandlerFactory) {
+    AsyncExecRuntime createAsyncExecRuntime(final HandlerFactory<AsyncPushConsumer> pushHandlerFactory,
+                                            final HttpRoute route) {
         return new InternalH2AsyncExecRuntime(LOG, connPool, pushHandlerFactory);
     }
 
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalHttpAsyncClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalHttpAsyncClient.java
index c2b7c5c..3ab0862 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalHttpAsyncClient.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalHttpAsyncClient.java
@@ -95,8 +95,17 @@ public final class InternalHttpAsyncClient extends InternalAbstractHttpAsyncClie
     }
 
     @Override
-    AsyncExecRuntime createAsyncExecRuntime(final HandlerFactory<AsyncPushConsumer> pushHandlerFactory) {
-        return new InternalHttpAsyncExecRuntime(LOG, manager, getConnectionInitiator(), pushHandlerFactory, versionPolicy);
+    AsyncExecRuntime createAsyncExecRuntime(final HandlerFactory<AsyncPushConsumer> pushHandlerFactory,
+                                            final HttpRoute route) {
+        // Automatically set protocol policy to force HTTP/1.1
+        // when executing requests via proxy tunnel
+        final HttpVersionPolicy actualVersionPolicy;
+        if (route.isTunnelled() && versionPolicy == HttpVersionPolicy.NEGOTIATE) {
+            actualVersionPolicy = HttpVersionPolicy.FORCE_HTTP_1;
+        } else {
+            actualVersionPolicy = versionPolicy;
+        }
+        return new InternalHttpAsyncExecRuntime(LOG, manager, getConnectionInitiator(), pushHandlerFactory, actualVersionPolicy);
     }
 
     @Override

[httpcomponents-client] 02/03: HTTPCLIENT-2177: fixed incorrect route state tracking by the async connect executor when negotiating a tunnel via a proxy

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

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

commit d7aa782971d87f405222d88dbd5c1e4111b4c1fe
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Sep 17 16:28:18 2021 +0200

    HTTPCLIENT-2177: fixed incorrect route state tracking by the async connect executor when negotiating a tunnel via a proxy
---
 .../client5/http/impl/async/AsyncConnectExec.java  | 51 +++++++++++++---------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncConnectExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncConnectExec.java
index 07fa739..7d5d01a 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncConnectExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncConnectExec.java
@@ -246,7 +246,10 @@ public final class AsyncConnectExec implements AsyncExecChainHandler {
                     try {
                         final HttpHost proxy = route.getProxyHost();
                         final HttpHost target = route.getTargetHost();
-                        createTunnel(state, proxy ,target, scope, chain, new AsyncExecCallback() {
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("{}: create tunnel", exchangeId);
+                        }
+                        createTunnel(state, proxy, target, scope, chain, new AsyncExecCallback() {
 
                             @Override
                             public AsyncDataConsumer handleResponse(
@@ -263,11 +266,32 @@ public final class AsyncConnectExec implements AsyncExecChainHandler {
 
                             @Override
                             public void completed() {
-                                if (LOG.isDebugEnabled()) {
-                                    LOG.debug("{}: tunnel to target created", exchangeId);
+                                if (!execRuntime.isEndpointConnected()) {
+                                    // Remote endpoint disconnected. Need to start over
+                                    if (LOG.isDebugEnabled()) {
+                                        LOG.debug("{}: proxy disconnected", exchangeId);
+                                    }
+                                    state.tracker.reset();
+                                }
+                                if (state.challenged) {
+                                    if (LOG.isDebugEnabled()) {
+                                        LOG.debug("{}: proxy authentication required", exchangeId);
+                                    }
+                                    proceedToNextHop(state, request, entityProducer, scope, chain, asyncExecCallback);
+                                } else {
+                                    if (state.tunnelRefused) {
+                                        if (LOG.isDebugEnabled()) {
+                                            LOG.debug("{}: tunnel refused", exchangeId);
+                                        }
+                                        asyncExecCallback.failed(new TunnelRefusedException("Tunnel refused", null));
+                                    } else {
+                                        if (LOG.isDebugEnabled()) {
+                                            LOG.debug("{}: tunnel to target created", exchangeId);
+                                        }
+                                        tracker.tunnelTarget(false);
+                                        proceedToNextHop(state, request, entityProducer, scope, chain, asyncExecCallback);
+                                    }
                                 }
-                                tracker.tunnelTarget(false);
-                                proceedToNextHop(state, request, entityProducer, scope, chain, asyncExecCallback);
                             }
 
                             @Override
@@ -371,22 +395,7 @@ public final class AsyncConnectExec implements AsyncExecChainHandler {
 
             @Override
             public void completed() {
-                if (!execRuntime.isEndpointConnected()) {
-                    state.tracker.reset();
-                }
-                if (state.challenged) {
-                    try {
-                        createTunnel(state, proxy, nextHop, scope, chain, asyncExecCallback);
-                    } catch (final HttpException | IOException ex) {
-                        asyncExecCallback.failed(ex);
-                    }
-                } else {
-                    if (state.tunnelRefused) {
-                        asyncExecCallback.failed(new TunnelRefusedException("Tunnel refused", null));
-                    } else {
-                        asyncExecCallback.completed();
-                    }
-                }
+                asyncExecCallback.completed();
             }
 
             @Override

[httpcomponents-client] 01/03: HTTPCLIENT-2177: keep successful tunnel connections alive regardless of `Connection: close`

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

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

commit 2af0bab75649093e0e621396c511ed3ca99298a9
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Sep 17 16:23:21 2021 +0200

    HTTPCLIENT-2177: keep successful tunnel connections alive regardless of `Connection: close`
---
 .../impl/DefaultClientConnectionReuseStrategy.java | 53 ++++++++++++++++++++++
 .../http/impl/async/HttpAsyncClientBuilder.java    |  8 ++--
 .../async/HttpAsyncClientEventHandlerFactory.java  |  4 +-
 .../client5/http/impl/async/HttpAsyncClients.java  |  4 +-
 .../hc/client5/http/impl/classic/ConnectExec.java  | 23 +++++-----
 .../http/impl/classic/HttpClientBuilder.java       |  8 ++--
 .../http/impl/classic/MinimalHttpClient.java       |  4 +-
 .../hc/client5/http/impl/classic/ProxyClient.java  |  6 +--
 .../client5/http/impl/classic/TestConnectExec.java |  8 ++--
 9 files changed, 86 insertions(+), 32 deletions(-)

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultClientConnectionReuseStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultClientConnectionReuseStrategy.java
new file mode 100644
index 0000000..c6b308e
--- /dev/null
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultClientConnectionReuseStrategy.java
@@ -0,0 +1,53 @@
+/*
+ * ====================================================================
+ * 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;
+
+import org.apache.hc.core5.annotation.Contract;
+import org.apache.hc.core5.annotation.Internal;
+import org.apache.hc.core5.annotation.ThreadingBehavior;
+import org.apache.hc.core5.http.HttpRequest;
+import org.apache.hc.core5.http.HttpResponse;
+import org.apache.hc.core5.http.HttpStatus;
+import org.apache.hc.core5.http.Method;
+import org.apache.hc.core5.http.impl.DefaultConnectionReuseStrategy;
+import org.apache.hc.core5.http.protocol.HttpContext;
+
+@Internal
+@Contract(threading = ThreadingBehavior.STATELESS)
+public class DefaultClientConnectionReuseStrategy extends DefaultConnectionReuseStrategy {
+
+    public static final DefaultClientConnectionReuseStrategy INSTANCE = new DefaultClientConnectionReuseStrategy();
+
+    @Override
+    public boolean keepAlive(final HttpRequest request, final HttpResponse response, final HttpContext context) {
+        if (Method.CONNECT.isSame(request.getMethod()) && response.getCode() == HttpStatus.SC_OK) {
+            return true;
+        }
+        return super.keepAlive(request, response, context);
+    }
+
+}
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientBuilder.java
index 2b9ca48..a88d281 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientBuilder.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientBuilder.java
@@ -45,8 +45,8 @@ import org.apache.hc.client5.http.SchemePortResolver;
 import org.apache.hc.client5.http.UserTokenHandler;
 import org.apache.hc.client5.http.async.AsyncExecChainHandler;
 import org.apache.hc.client5.http.auth.AuthSchemeFactory;
-import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.CredentialsProvider;
+import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.cookie.BasicCookieStore;
 import org.apache.hc.client5.http.cookie.CookieSpecFactory;
@@ -54,6 +54,7 @@ import org.apache.hc.client5.http.cookie.CookieStore;
 import org.apache.hc.client5.http.impl.ChainElement;
 import org.apache.hc.client5.http.impl.CookieSpecSupport;
 import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy;
+import org.apache.hc.client5.http.impl.DefaultClientConnectionReuseStrategy;
 import org.apache.hc.client5.http.impl.DefaultConnectionKeepAliveStrategy;
 import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy;
 import org.apache.hc.client5.http.impl.DefaultRedirectStrategy;
@@ -96,7 +97,6 @@ import org.apache.hc.core5.http.config.Http1Config;
 import org.apache.hc.core5.http.config.Lookup;
 import org.apache.hc.core5.http.config.NamedElementChain;
 import org.apache.hc.core5.http.config.RegistryBuilder;
-import org.apache.hc.core5.http.impl.DefaultConnectionReuseStrategy;
 import org.apache.hc.core5.http.nio.AsyncPushConsumer;
 import org.apache.hc.core5.http.nio.HandlerFactory;
 import org.apache.hc.core5.http.nio.command.ShutdownCommand;
@@ -897,7 +897,7 @@ public class HttpAsyncClientBuilder {
             if (systemProperties) {
                 final String s = getProperty("http.keepAlive", "true");
                 if ("true".equalsIgnoreCase(s)) {
-                    reuseStrategyCopy = DefaultConnectionReuseStrategy.INSTANCE;
+                    reuseStrategyCopy = DefaultClientConnectionReuseStrategy.INSTANCE;
                 } else {
                     reuseStrategyCopy = new ConnectionReuseStrategy() {
                         @Override
@@ -908,7 +908,7 @@ public class HttpAsyncClientBuilder {
                     };
                 }
             } else {
-                reuseStrategyCopy = DefaultConnectionReuseStrategy.INSTANCE;
+                reuseStrategyCopy = DefaultClientConnectionReuseStrategy.INSTANCE;
             }
         }
         final AsyncPushConsumerRegistry pushConsumerRegistry = new AsyncPushConsumerRegistry();
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientEventHandlerFactory.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientEventHandlerFactory.java
index 4d64958..e543421 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientEventHandlerFactory.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientEventHandlerFactory.java
@@ -31,6 +31,7 @@ import java.io.IOException;
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.hc.client5.http.impl.DefaultClientConnectionReuseStrategy;
 import org.apache.hc.core5.http.ConnectionReuseStrategy;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.HttpConnection;
@@ -38,7 +39,6 @@ import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.HttpResponse;
 import org.apache.hc.core5.http.config.CharCodingConfig;
 import org.apache.hc.core5.http.config.Http1Config;
-import org.apache.hc.core5.http.impl.DefaultConnectionReuseStrategy;
 import org.apache.hc.core5.http.impl.Http1StreamListener;
 import org.apache.hc.core5.http.impl.nio.ClientHttp1StreamDuplexerFactory;
 import org.apache.hc.core5.http.impl.nio.DefaultHttpRequestWriterFactory;
@@ -96,7 +96,7 @@ class HttpAsyncClientEventHandlerFactory implements IOEventHandlerFactory {
         this.h2Config = h2Config != null ? h2Config : H2Config.DEFAULT;
         this.h1Config = h1Config != null ? h1Config : Http1Config.DEFAULT;
         this.charCodingConfig = charCodingConfig != null ? charCodingConfig : CharCodingConfig.DEFAULT;
-        this.http1ConnectionReuseStrategy = connectionReuseStrategy != null ? connectionReuseStrategy : DefaultConnectionReuseStrategy.INSTANCE;
+        this.http1ConnectionReuseStrategy = connectionReuseStrategy != null ? connectionReuseStrategy : DefaultClientConnectionReuseStrategy.INSTANCE;
         this.http1ResponseParserFactory = new DefaultHttpResponseParserFactory(h1Config);
         this.http1RequestWriterFactory = DefaultHttpRequestWriterFactory.INSTANCE;
     }
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java
index 9c38f24..fe4a1cf 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClients.java
@@ -30,6 +30,7 @@ package org.apache.hc.client5.http.impl.async;
 import org.apache.hc.client5.http.DnsResolver;
 import org.apache.hc.client5.http.SchemePortResolver;
 import org.apache.hc.client5.http.SystemDefaultDnsResolver;
+import org.apache.hc.client5.http.impl.DefaultClientConnectionReuseStrategy;
 import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
 import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
 import org.apache.hc.client5.http.nio.AsyncClientConnectionManager;
@@ -39,7 +40,6 @@ import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.config.CharCodingConfig;
 import org.apache.hc.core5.http.config.Http1Config;
-import org.apache.hc.core5.http.impl.DefaultConnectionReuseStrategy;
 import org.apache.hc.core5.http.nio.AsyncPushConsumer;
 import org.apache.hc.core5.http.nio.HandlerFactory;
 import org.apache.hc.core5.http.nio.ssl.TlsStrategy;
@@ -169,7 +169,7 @@ public final class HttpAsyncClients {
                         h2Config,
                         h1Config,
                         CharCodingConfig.DEFAULT,
-                        DefaultConnectionReuseStrategy.INSTANCE),
+                        DefaultClientConnectionReuseStrategy.INSTANCE),
                 pushConsumerRegistry,
                 versionPolicy,
                 ioReactorConfig,
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ConnectExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ConnectExec.java
index 524a9dd..fb25979 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ConnectExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ConnectExec.java
@@ -226,22 +226,23 @@ public final class ConnectExec implements ExecChainHandler {
                 throw new HttpException("Unexpected response to CONNECT request: " + new StatusLine(response));
             }
 
+            if (this.reuseStrategy.keepAlive(connect, response, context)) {
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("{}: connection kept alive", exchangeId);
+                }
+                // Consume response content
+                final HttpEntity entity = response.getEntity();
+                EntityUtils.consume(entity);
+            } else {
+                execRuntime.disconnectEndpoint();
+            }
+
             if (config.isAuthenticationEnabled()) {
                 if (this.authenticator.isChallenged(proxy, ChallengeType.PROXY, response,
                         proxyAuthExchange, context)) {
                     if (this.authenticator.updateAuthState(proxy, ChallengeType.PROXY, response,
                             this.proxyAuthStrategy, proxyAuthExchange, context)) {
                         // Retry request
-                        if (this.reuseStrategy.keepAlive(request, response, context)) {
-                            if (LOG.isDebugEnabled()) {
-                                LOG.debug("{}: connection kept alive", exchangeId);
-                            }
-                            // Consume response content
-                            final HttpEntity entity = response.getEntity();
-                            EntityUtils.consume(entity);
-                        } else {
-                            execRuntime.disconnectEndpoint();
-                        }
                         response = null;
                     }
                 }
@@ -249,7 +250,7 @@ public final class ConnectExec implements ExecChainHandler {
         }
 
         final int status = response.getCode();
-        if (status >= HttpStatus.SC_REDIRECTION) {
+        if (status != HttpStatus.SC_OK) {
 
             // Buffer response content
             final HttpEntity entity = response.getEntity();
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java
index 2a6ad0e..a5f2655 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java
@@ -43,8 +43,8 @@ import org.apache.hc.client5.http.HttpRequestRetryStrategy;
 import org.apache.hc.client5.http.SchemePortResolver;
 import org.apache.hc.client5.http.UserTokenHandler;
 import org.apache.hc.client5.http.auth.AuthSchemeFactory;
-import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.CredentialsProvider;
+import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.classic.BackoffManager;
 import org.apache.hc.client5.http.classic.ConnectionBackoffStrategy;
 import org.apache.hc.client5.http.classic.ExecChainHandler;
@@ -56,6 +56,7 @@ import org.apache.hc.client5.http.entity.InputStreamFactory;
 import org.apache.hc.client5.http.impl.ChainElement;
 import org.apache.hc.client5.http.impl.CookieSpecSupport;
 import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy;
+import org.apache.hc.client5.http.impl.DefaultClientConnectionReuseStrategy;
 import org.apache.hc.client5.http.impl.DefaultConnectionKeepAliveStrategy;
 import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy;
 import org.apache.hc.client5.http.impl.DefaultRedirectStrategy;
@@ -95,7 +96,6 @@ import org.apache.hc.core5.http.config.Lookup;
 import org.apache.hc.core5.http.config.NamedElementChain;
 import org.apache.hc.core5.http.config.Registry;
 import org.apache.hc.core5.http.config.RegistryBuilder;
-import org.apache.hc.core5.http.impl.DefaultConnectionReuseStrategy;
 import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
 import org.apache.hc.core5.http.protocol.DefaultHttpProcessor;
 import org.apache.hc.core5.http.protocol.HttpContext;
@@ -746,7 +746,7 @@ public class HttpClientBuilder {
             if (systemProperties) {
                 final String s = System.getProperty("http.keepAlive", "true");
                 if ("true".equalsIgnoreCase(s)) {
-                    reuseStrategyCopy = DefaultConnectionReuseStrategy.INSTANCE;
+                    reuseStrategyCopy = DefaultClientConnectionReuseStrategy.INSTANCE;
                 } else {
                     reuseStrategyCopy = new ConnectionReuseStrategy() {
                         @Override
@@ -757,7 +757,7 @@ public class HttpClientBuilder {
                     };
                 }
             } else {
-                reuseStrategyCopy = DefaultConnectionReuseStrategy.INSTANCE;
+                reuseStrategyCopy = DefaultClientConnectionReuseStrategy.INSTANCE;
             }
         }
 
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/MinimalHttpClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/MinimalHttpClient.java
index 2144099..b6f4230 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/MinimalHttpClient.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/MinimalHttpClient.java
@@ -37,6 +37,7 @@ import org.apache.hc.client5.http.classic.ExecRuntime;
 import org.apache.hc.client5.http.config.Configurable;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.ConnectionShutdownException;
+import org.apache.hc.client5.http.impl.DefaultClientConnectionReuseStrategy;
 import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
 import org.apache.hc.client5.http.impl.ExecSupport;
 import org.apache.hc.client5.http.io.HttpClientConnectionManager;
@@ -52,7 +53,6 @@ import org.apache.hc.core5.http.ConnectionReuseStrategy;
 import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.HttpHost;
-import org.apache.hc.core5.http.impl.DefaultConnectionReuseStrategy;
 import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
 import org.apache.hc.core5.http.protocol.BasicHttpContext;
 import org.apache.hc.core5.http.protocol.DefaultHttpProcessor;
@@ -96,7 +96,7 @@ public class MinimalHttpClient extends CloseableHttpClient {
     MinimalHttpClient(final HttpClientConnectionManager connManager) {
         super();
         this.connManager = Args.notNull(connManager, "HTTP connection manager");
-        this.reuseStrategy = DefaultConnectionReuseStrategy.INSTANCE;
+        this.reuseStrategy = DefaultClientConnectionReuseStrategy.INSTANCE;
         this.schemePortResolver = DefaultSchemePortResolver.INSTANCE;
         this.requestExecutor = new HttpRequestExecutor(this.reuseStrategy);
         this.httpProcessor = new DefaultHttpProcessor(
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProxyClient.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProxyClient.java
index ef7a5ea..95cad63 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProxyClient.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProxyClient.java
@@ -36,12 +36,13 @@ import org.apache.hc.client5.http.RouteInfo.LayerType;
 import org.apache.hc.client5.http.RouteInfo.TunnelType;
 import org.apache.hc.client5.http.auth.AuthExchange;
 import org.apache.hc.client5.http.auth.AuthSchemeFactory;
-import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.AuthScope;
 import org.apache.hc.client5.http.auth.ChallengeType;
 import org.apache.hc.client5.http.auth.Credentials;
+import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.config.RequestConfig;
 import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy;
+import org.apache.hc.client5.http.impl.DefaultClientConnectionReuseStrategy;
 import org.apache.hc.client5.http.impl.TunnelRefusedException;
 import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider;
 import org.apache.hc.client5.http.impl.auth.BasicSchemeFactory;
@@ -65,7 +66,6 @@ import org.apache.hc.core5.http.config.CharCodingConfig;
 import org.apache.hc.core5.http.config.Http1Config;
 import org.apache.hc.core5.http.config.Lookup;
 import org.apache.hc.core5.http.config.RegistryBuilder;
-import org.apache.hc.core5.http.impl.DefaultConnectionReuseStrategy;
 import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
 import org.apache.hc.core5.http.io.HttpConnectionFactory;
 import org.apache.hc.core5.http.io.entity.EntityUtils;
@@ -119,7 +119,7 @@ public class ProxyClient {
                 .register(StandardAuthScheme.SPNEGO, SPNegoSchemeFactory.DEFAULT)
                 .register(StandardAuthScheme.KERBEROS, KerberosSchemeFactory.DEFAULT)
                 .build();
-        this.reuseStrategy = new DefaultConnectionReuseStrategy();
+        this.reuseStrategy = DefaultClientConnectionReuseStrategy.INSTANCE;
     }
 
     /**
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestConnectExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestConnectExec.java
index 82dd8fe..1827c5c 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestConnectExec.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestConnectExec.java
@@ -36,9 +36,9 @@ import org.apache.hc.client5.http.HttpRoute;
 import org.apache.hc.client5.http.RouteInfo;
 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.ChallengeType;
+import org.apache.hc.client5.http.auth.StandardAuthScheme;
 import org.apache.hc.client5.http.auth.UsernamePasswordCredentials;
 import org.apache.hc.client5.http.classic.ExecChain;
 import org.apache.hc.client5.http.classic.ExecRuntime;
@@ -233,7 +233,7 @@ public class TestConnectExec {
             exec.execute(request, scope, execChain);
         } catch (final TunnelRefusedException ex) {
             Assert.assertEquals("Ka-boom", ex.getResponseMessage());
-            Mockito.verify(execRuntime).disconnectEndpoint();
+            Mockito.verify(execRuntime, Mockito.atLeastOnce()).disconnectEndpoint();
             Mockito.verify(execRuntime).discardEndpoint();
             throw ex;
         }
@@ -260,7 +260,7 @@ public class TestConnectExec {
         Mockito.doAnswer(connectionState.connectAnswer()).when(execRuntime).connectEndpoint(Mockito.<HttpClientContext>any());
         Mockito.when(execRuntime.isEndpointConnected()).thenAnswer(connectionState.isConnectedAnswer());
         Mockito.when(reuseStrategy.keepAlive(
-                Mockito.same(request),
+                Mockito.<HttpRequest>any(),
                 Mockito.<HttpResponse>any(),
                 Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
         Mockito.when(execRuntime.execute(
@@ -319,7 +319,7 @@ public class TestConnectExec {
 
         Mockito.verify(execRuntime).connectEndpoint(context);
         Mockito.verify(inStream1, Mockito.never()).close();
-        Mockito.verify(execRuntime).disconnectEndpoint();
+        Mockito.verify(execRuntime, Mockito.atLeastOnce()).disconnectEndpoint();
     }
 
     @Test(expected = HttpException.class)