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 19:36:24 UTC
[httpcomponents-client] 01/04: HTTPCLIENT-2177: keep successful
tunnel connections alive regardless of `Connection: close`
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 4ce032c92c6f1f7beaafbb5622647de8db93586f
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