From 306f6063b637ec10245a897a03a5d23395c48826 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 14 Sep 2025 21:07:23 +0200 Subject: [PATCH] Bug fix: connections managers to ensure open connections have socket timeout set based on ConnectionConfig upon lease --- .../async/TestAsyncConnectionManagement.java | 62 ++++++++++++++++--- .../sync/TestConnectionManagement.java | 48 ++++++++++++++ .../io/BasicHttpClientConnectionManager.java | 3 + .../PoolingHttpClientConnectionManager.java | 3 + .../PoolingAsyncClientConnectionManager.java | 3 + 5 files changed, 110 insertions(+), 9 deletions(-) diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionManagement.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionManagement.java index fa6f35ab55..b44df5d5c5 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionManagement.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionManagement.java @@ -37,6 +37,7 @@ import org.apache.hc.client5.http.async.methods.SimpleRequestProducer; import org.apache.hc.client5.http.async.methods.SimpleResponseConsumer; import org.apache.hc.client5.http.config.ConnectionConfig; +import org.apache.hc.client5.http.impl.ConnectionHolder; import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager; import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder; import org.apache.hc.client5.http.nio.AsyncConnectionEndpoint; @@ -45,6 +46,7 @@ import org.apache.hc.client5.testing.extension.async.ServerProtocolLevel; import org.apache.hc.client5.testing.extension.async.TestAsyncClient; import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.HttpConnection; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpStatus; @@ -89,11 +91,9 @@ protected SimpleHttpResponse handle( void testReleaseConnection() throws Exception { final HttpHost target = startServer(); - final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create() - .build(); - configureClient(builder -> builder.setConnectionManager(connManager)); final TestAsyncClient client = startClient(); + final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager(); connManager.setMaxTotal(1); final HttpRoute route = new HttpRoute(target, null, false); @@ -159,11 +159,9 @@ void testReleaseConnection() throws Exception { void testReleaseConnectionWithTimeLimits() throws Exception { final HttpHost target = startServer(); - final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create() - .build(); - configureClient(builder -> builder.setConnectionManager(connManager)); final TestAsyncClient client = startClient(); + final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager(); connManager.setMaxTotal(1); final HttpRoute route = new HttpRoute(target, null, false); @@ -218,11 +216,9 @@ void testReleaseConnectionWithTimeLimits() throws Exception { void testCloseExpiredIdleConnections() throws Exception { final HttpHost target = startServer(); - final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create() - .build(); - configureClient(builder -> builder.setConnectionManager(connManager)); final TestAsyncClient client = startClient(); + final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager(); connManager.setMaxTotal(1); final HttpRoute route = new HttpRoute(target, null, false); @@ -312,4 +308,52 @@ void testCloseExpiredTTLConnections() throws Exception { connManager.close(); } + @Test + void testConnectionTimeoutSetting() throws Exception { + final HttpHost target = startServer(); + + final TestAsyncClient client = startClient(); + + final Timeout connectionSocketTimeout = Timeout.ofMinutes(5); + + final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager(); + connManager.setMaxTotal(1); + connManager.setDefaultConnectionConfig(ConnectionConfig.custom() + .setSocketTimeout(connectionSocketTimeout) + .build()); + + final HttpRoute route = new HttpRoute(target, null, false); + + final SimpleHttpRequest request = SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/") + .addHeader(HttpHeaders.HOST, target.toHostString()) + .build(); + final HttpClientContext context = HttpClientContext.create(); + + final Future endpointFuture1 = connManager.lease("id1", route, null, TIMEOUT, null); + final AsyncConnectionEndpoint endpoint1 = endpointFuture1.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit()); + + final Future connectFuture1 = connManager.connect(endpoint1, client.getImplementation(), TIMEOUT, null, context, null); + final AsyncConnectionEndpoint openEndpoint1 = connectFuture1.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + + // Modify socket timeout of the endpoint + endpoint1.setSocketTimeout(Timeout.ofSeconds(30)); + + final Future responseFuture1 = openEndpoint1.execute("ex-1", SimpleRequestProducer.create(request), SimpleResponseConsumer.create(), null); + final SimpleHttpResponse response1 = responseFuture1.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + Assertions.assertEquals(HttpStatus.SC_OK, response1.getCode()); + + connManager.release(endpoint1, null, TimeValue.NEG_ONE_MILLISECOND); + + final Future endpointFuture2 = connManager.lease("id2", route, null, TIMEOUT, null); + final AsyncConnectionEndpoint endpoint2 = endpointFuture2.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit()); + Assertions.assertTrue(endpoint2.isConnected()); + + final HttpConnection connection = ((ConnectionHolder) endpoint2).get(); + Assertions.assertEquals(connectionSocketTimeout, connection.getSocketTimeout()); + + connManager.close(); + } + } 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 c46f98e87c..9380ad25a3 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 @@ -32,6 +32,7 @@ import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.config.ConnectionConfig; +import org.apache.hc.client5.http.impl.ConnectionHolder; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder; import org.apache.hc.client5.http.io.ConnectionEndpoint; @@ -42,6 +43,7 @@ import org.apache.hc.client5.testing.extension.sync.TestClient; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.HttpConnection; import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpStatus; @@ -326,4 +328,50 @@ void testCloseExpiredTTLConnections() throws Exception { connManager.close(); } + @Test + void testConnectionTimeoutSetting() throws Exception { + configureServer(bootstrap -> bootstrap + .register("/random/*", new RandomHandler())); + final HttpHost target = startServer(); + + final Timeout connectionSocketTimeout = Timeout.ofMinutes(5); + + final TestClient client = client(); + final PoolingHttpClientConnectionManager connManager = client.getConnectionManager(); + connManager.setMaxTotal(1); + connManager.setDefaultConnectionConfig(ConnectionConfig.custom() + .setSocketTimeout(connectionSocketTimeout) + .build()); + + final HttpRoute route = new HttpRoute(target, null, false); + final int rsplen = 8; + final String uri = "/random/" + rsplen; + + final ClassicHttpRequest request = new BasicClassicHttpRequest("GET", target, uri); + final HttpClientContext context = HttpClientContext.create(); + + final LeaseRequest leaseRequest1 = connManager.lease("id1", route, null); + final ConnectionEndpoint endpoint1 = leaseRequest1.get(Timeout.ZERO_MILLISECONDS); + + connManager.connect(endpoint1, null, context); + + // Modify socket timeout of the endpoint + endpoint1.setSocketTimeout(Timeout.ofSeconds(30)); + + try (final ClassicHttpResponse response1 = endpoint1.execute("id1", request, exec, context)) { + Assertions.assertEquals(HttpStatus.SC_OK, response1.getCode()); + } + + connManager.release(endpoint1, null, TimeValue.NEG_ONE_MILLISECOND); + + final LeaseRequest leaseRequest2 = connManager.lease("id2", route, null); + final ConnectionEndpoint endpoint2 = leaseRequest2.get(Timeout.ZERO_MILLISECONDS); + Assertions.assertTrue(endpoint2.isConnected()); + + final HttpConnection connection = ((ConnectionHolder) endpoint2).get(); + Assertions.assertEquals(connectionSocketTimeout, connection.getSocketTimeout()); + + connManager.close(); + } + } 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 9ca40f9986..d634d94682 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 @@ -401,6 +401,9 @@ ManagedHttpClientConnection getConnection(final HttpRoute route, final Object st this.created = System.currentTimeMillis(); } else { this.conn.activate(); + if (connectionConfig.getSocketTimeout() != null) { + conn.setSocketTimeout(connectionConfig.getSocketTimeout()); + } } this.leased = true; if (LOG.isDebugEnabled()) { 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 e027f56a81..6d8ccea485 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 @@ -386,6 +386,9 @@ public ConnectionEndpoint get( final ManagedHttpClientConnection conn = poolEntry.getConnection(); if (conn != null) { conn.activate(); + if (connectionConfig.getSocketTimeout() != null) { + conn.setSocketTimeout(connectionConfig.getSocketTimeout()); + } } else { poolEntry.assignConnection(connFactory.createConnection(null)); } 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 be7b5348e4..512276181d 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 @@ -326,6 +326,9 @@ void leaseCompleted(final PoolEntry poo final ManagedAsyncClientConnection connection = poolEntry.getConnection(); if (connection != null) { connection.activate(); + if (connectionConfig.getSocketTimeout() != null) { + connection.setSocketTimeout(connectionConfig.getSocketTimeout()); + } } if (LOG.isDebugEnabled()) { LOG.debug("{} endpoint leased {}", id, ConnPoolSupport.formatStats(route, state, pool));