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:44:22 UTC

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

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.


    from 677903e  HTTPCLIENT-2135: support for a distinct handshake timeout (mainly intended for TLS/SSL) by the connection management APIs
     new 1686495  HTTPCLIENT-2177: keep successful tunnel connections alive regardless of `Connection: close`
     new c5b260a  HTTPCLIENT-2177: fixed incorrect route state tracking by the async connect executor when negotiating a tunnel via a proxy

The 2 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} | 23 +++++---
 .../client5/http/impl/async/AsyncConnectExec.java  | 65 ++++++++++++----------
 .../http/impl/async/HttpAsyncClientBuilder.java    |  6 +-
 .../async/HttpAsyncClientEventHandlerFactory.java  |  4 +-
 .../client5/http/impl/async/HttpAsyncClients.java  |  4 +-
 .../hc/client5/http/impl/classic/ConnectExec.java  | 23 ++++----
 .../http/impl/classic/HttpClientBuilder.java       |  6 +-
 .../http/impl/classic/MinimalHttpClient.java       |  4 +-
 .../hc/client5/http/impl/classic/ProxyClient.java  |  6 +-
 .../client5/http/impl/classic/TestConnectExec.java | 14 ++---
 10 files changed, 84 insertions(+), 71 deletions(-)
 copy httpclient5/src/main/java/org/apache/hc/client5/http/impl/{NoopUserTokenHandler.java => DefaultClientConnectionReuseStrategy.java} (61%)

[httpcomponents-client] 01/02: 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 master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit 1686495e7398a618965314b385982b7e8e5f9061
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 | 57 ++++++++++++++++++++++
 .../http/impl/async/HttpAsyncClientBuilder.java    |  6 +--
 .../async/HttpAsyncClientEventHandlerFactory.java  |  4 +-
 .../client5/http/impl/async/HttpAsyncClients.java  |  4 +-
 .../hc/client5/http/impl/classic/ConnectExec.java  | 23 ++++-----
 .../http/impl/classic/HttpClientBuilder.java       |  6 +--
 .../http/impl/classic/MinimalHttpClient.java       |  4 +-
 .../hc/client5/http/impl/classic/ProxyClient.java  |  6 +--
 .../client5/http/impl/classic/TestConnectExec.java | 14 ++----
 9 files changed, 89 insertions(+), 35 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..f17accf
--- /dev/null
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultClientConnectionReuseStrategy.java
@@ -0,0 +1,57 @@
+/*
+ * ====================================================================
+ * 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.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;
+
+/**
+ * Extension of core {@link DefaultConnectionReuseStrategy} that treats
+ * CONNECT method exchnages involved in proxy tunnelling as a special case.
+ *
+ * @since 5.2
+ */
+@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 18c100e..44a6759 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
@@ -53,6 +53,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;
@@ -92,7 +93,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.command.ShutdownCommand;
 import org.apache.hc.core5.http.protocol.DefaultHttpProcessor;
 import org.apache.hc.core5.http.protocol.HttpProcessor;
@@ -888,12 +888,12 @@ public class HttpAsyncClientBuilder {
             if (systemProperties) {
                 final String s = getProperty("http.keepAlive", "true");
                 if ("true".equalsIgnoreCase(s)) {
-                    reuseStrategyCopy = DefaultConnectionReuseStrategy.INSTANCE;
+                    reuseStrategyCopy = DefaultClientConnectionReuseStrategy.INSTANCE;
                 } else {
                     reuseStrategyCopy = (request, response, context) -> false;
                 }
             } 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 3e07327..f7214ed 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;
@@ -37,7 +38,6 @@ import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
 import org.apache.hc.core5.concurrent.DefaultThreadFactory;
 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.ssl.TlsStrategy;
 import org.apache.hc.core5.http.protocol.DefaultHttpProcessor;
 import org.apache.hc.core5.http.protocol.HttpProcessor;
@@ -157,7 +157,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 45a7ae7..357915f 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
@@ -227,22 +227,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;
                     }
                 }
@@ -250,7 +251,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 3e0144b..13e7b3b 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
@@ -55,6 +55,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;
@@ -92,7 +93,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.HttpProcessor;
@@ -742,12 +742,12 @@ 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 = (request, response, context) -> false;
                 }
             } 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 b7e63ef..1c8a4e5 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;
@@ -66,7 +67,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;
@@ -125,7 +125,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 1fde3b6..cae10cf 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
@@ -221,7 +221,7 @@ public class TestConnectExec {
         final TunnelRefusedException exception = Assert.assertThrows(TunnelRefusedException.class, () ->
                 exec.execute(request, scope, execChain));
         Assert.assertEquals("Ka-boom", exception.getResponseMessage());
-        Mockito.verify(execRuntime).disconnectEndpoint();
+        Mockito.verify(execRuntime, Mockito.atLeastOnce()).disconnectEndpoint();
         Mockito.verify(execRuntime).discardEndpoint();
     }
 
@@ -246,7 +246,7 @@ public class TestConnectExec {
         Mockito.doAnswer(connectionState.connectAnswer()).when(execRuntime).connectEndpoint(Mockito.any());
         Mockito.when(execRuntime.isEndpointConnected()).thenAnswer(connectionState.isConnectedAnswer());
         Mockito.when(reuseStrategy.keepAlive(
-                Mockito.same(request),
+                Mockito.any(),
                 Mockito.any(),
                 Mockito.<HttpClientContext>any())).thenReturn(Boolean.TRUE);
         Mockito.when(execRuntime.execute(
@@ -257,7 +257,7 @@ public class TestConnectExec {
         Mockito.when(proxyAuthStrategy.select(
                 Mockito.eq(ChallengeType.PROXY),
                 Mockito.any(),
-                Mockito.<HttpClientContext>any())).thenReturn(Collections.singletonList(new BasicScheme()));
+                Mockito.any())).thenReturn(Collections.singletonList(new BasicScheme()));
 
         final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context);
         exec.execute(request, scope, execChain);
@@ -286,10 +286,6 @@ public class TestConnectExec {
         final ConnectionState connectionState = new ConnectionState();
         Mockito.doAnswer(connectionState.connectAnswer()).when(execRuntime).connectEndpoint(Mockito.any());
         Mockito.when(execRuntime.isEndpointConnected()).thenAnswer(connectionState.isConnectedAnswer());
-        Mockito.when(reuseStrategy.keepAlive(
-                Mockito.same(request),
-                Mockito.any(),
-                Mockito.<HttpClientContext>any())).thenReturn(Boolean.FALSE);
         Mockito.when(execRuntime.execute(
                 Mockito.anyString(),
                 Mockito.any(),
@@ -298,14 +294,14 @@ public class TestConnectExec {
         Mockito.when(proxyAuthStrategy.select(
                 Mockito.eq(ChallengeType.PROXY),
                 Mockito.any(),
-                Mockito.<HttpClientContext>any())).thenReturn(Collections.singletonList(new BasicScheme()));
+                Mockito.any())).thenReturn(Collections.singletonList(new BasicScheme()));
 
         final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context);
         exec.execute(request, scope, execChain);
 
         Mockito.verify(execRuntime).connectEndpoint(context);
         Mockito.verify(inStream1, Mockito.never()).close();
-        Mockito.verify(execRuntime).disconnectEndpoint();
+        Mockito.verify(execRuntime, Mockito.atLeastOnce()).disconnectEndpoint();
     }
 
     @Test

[httpcomponents-client] 02/02: 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 master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git

commit c5b260acd12546351993e4a7b506697082c4aa3c
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  | 65 ++++++++++++----------
 1 file changed, 37 insertions(+), 28 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 0ca255e..b0288c7 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
@@ -242,11 +242,14 @@ public final class AsyncConnectExec implements AsyncExecChainHandler {
                 }));
                 break;
 
-            case HttpRouteDirector.TUNNEL_TARGET:
-                try {
-                    final HttpHost proxy = route.getProxyHost();
-                    final HttpHost target = route.getTargetHost();
-                    createTunnel(state, proxy ,target, scope, chain, new AsyncExecCallback() {
+                case HttpRouteDirector.TUNNEL_TARGET:
+                    try {
+                        final HttpHost proxy = route.getProxyHost();
+                        final HttpHost target = route.getTargetHost();
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("{} create tunnel", exchangeId);
+                        }
+                        createTunnel(state, proxy, target, scope, chain, new AsyncExecCallback() {
 
                         @Override
                         public AsyncDataConsumer handleResponse(
@@ -261,14 +264,35 @@ public final class AsyncConnectExec implements AsyncExecChainHandler {
                             asyncExecCallback.handleInformationResponse(response);
                         }
 
-                        @Override
-                        public void completed() {
-                            if (LOG.isDebugEnabled()) {
-                                LOG.debug("{} tunnel to target created", exchangeId);
+                            @Override
+                            public void completed() {
+                                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
                         public void failed(final Exception cause) {
@@ -387,22 +411,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