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:23 UTC
[httpcomponents-client] branch master updated (c5b260a -> 6f791af)
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 c5b260a HTTPCLIENT-2177: fixed incorrect route state tracking by the async connect executor when negotiating a tunnel via a proxy
discard 1686495 HTTPCLIENT-2177: keep successful tunnel connections alive regardless of `Connection: close`
discard 677903e HTTPCLIENT-2135: support for a distinct handshake timeout (mainly intended for TLS/SSL) by the connection management APIs
discard da5168f Better connect operation logging
new 4ce032c HTTPCLIENT-2177: keep successful tunnel connections alive regardless of `Connection: close`
new 90f69c8 HTTPCLIENT-2177: fixed incorrect route state tracking by the async connect executor when negotiating a tunnel via a proxy
new 9496bb8 Better connect operation logging
new 6f791af 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 (c5b260a)
\
N -- N -- N refs/heads/master (6f791af)
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 4 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:
.../main/java/org/apache/hc/client5/http/config/ConnectionConfig.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[httpcomponents-client] 04/04: 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 6f791afa6151ed6394959f1bcaf11943bf6f93e2
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..34111f1 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 = handshakeTimeout;
+ 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/04: 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 90f69c87b27b721ea8f0e23bdb4baf92bd7cde06
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
[httpcomponents-client] 03/04: 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 9496bb8475792ffd1cf695c26c0671df3640b9d2
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/04: 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 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