From 9ed51fb595e5361feb8b75c18311e587512844d3 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Thu, 5 Jun 2025 18:17:09 +0200 Subject: [PATCH 01/29] HTTPCLIENT-2372 - Normalize HttpHost port comparison to treat implicit default ports as equal (#643) --- .../http/impl/DefaultRedirectStrategy.java | 31 ++++++++++++- .../http/impl/async/H2AsyncClientBuilder.java | 2 +- .../impl/async/HttpAsyncClientBuilder.java | 2 +- .../http/impl/classic/HttpClientBuilder.java | 2 +- .../impl/TestDefaultRedirectStrategy.java | 44 +++++++++++++++++++ 5 files changed, 77 insertions(+), 4 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java index dbfaed4c44..e794c7b508 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRedirectStrategy.java @@ -32,9 +32,11 @@ import java.util.Iterator; import java.util.Locale; +import org.apache.hc.client5.http.SchemePortResolver; import org.apache.hc.client5.http.protocol.RedirectStrategy; import org.apache.hc.client5.http.utils.URIUtils; import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.Internal; import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpException; @@ -56,18 +58,31 @@ @Contract(threading = ThreadingBehavior.STATELESS) public class DefaultRedirectStrategy implements RedirectStrategy { + private final SchemePortResolver schemePortResolver; + /** * Default instance of {@link DefaultRedirectStrategy}. */ public static final DefaultRedirectStrategy INSTANCE = new DefaultRedirectStrategy(); + @Internal + public DefaultRedirectStrategy(final SchemePortResolver schemePortResolver) { + this.schemePortResolver = schemePortResolver != null ? schemePortResolver : DefaultSchemePortResolver.INSTANCE; + } + + public DefaultRedirectStrategy() { + this(null); + } + @Override public boolean isRedirectAllowed( final HttpHost currentTarget, final HttpHost newTarget, final HttpRequest redirect, final HttpContext context) { - if (!currentTarget.equals(newTarget)) { + + // If authority (host + effective port) differs, strip sensitive headers + if (!isSameAuthority(currentTarget, newTarget)) { for (final Iterator
it = redirect.headerIterator(); it.hasNext(); ) { final Header header = it.next(); if (header.isSensitive() @@ -80,6 +95,20 @@ public boolean isRedirectAllowed( return true; } + private boolean isSameAuthority(final HttpHost h1, final HttpHost h2) { + if (h1 == null || h2 == null) { + return false; + } + final String host1 = h1.getHostName(); + final String host2 = h2.getHostName(); + if (!host1.equalsIgnoreCase(host2)) { + return false; + } + final int port1 = schemePortResolver.resolve(h1); + final int port2 = schemePortResolver.resolve(h2); + return port1 == port2; + } + @Override public boolean isRedirected( final HttpRequest request, diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java index 1e85bc955c..56de4b0401 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java @@ -836,7 +836,7 @@ public CloseableHttpAsyncClient build() { if (!redirectHandlingDisabled) { RedirectStrategy redirectStrategyCopy = this.redirectStrategy; if (redirectStrategyCopy == null) { - redirectStrategyCopy = DefaultRedirectStrategy.INSTANCE; + redirectStrategyCopy = schemePortResolver != null ? new DefaultRedirectStrategy(schemePortResolver) : DefaultRedirectStrategy.INSTANCE; } execChainDefinition.addFirst( new AsyncRedirectExec(routePlannerCopy, redirectStrategyCopy), 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 933b8ae781..4f56b46804 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 @@ -1023,7 +1023,7 @@ public CloseableHttpAsyncClient build() { if (!redirectHandlingDisabled) { RedirectStrategy redirectStrategyCopy = this.redirectStrategy; if (redirectStrategyCopy == null) { - redirectStrategyCopy = DefaultRedirectStrategy.INSTANCE; + redirectStrategyCopy = schemePortResolver != null ? new DefaultRedirectStrategy(schemePortResolver) : DefaultRedirectStrategy.INSTANCE; } execChainDefinition.addFirst( new AsyncRedirectExec(routePlannerCopy, redirectStrategyCopy), 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 4ba68b9192..3f871ff902 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 @@ -1011,7 +1011,7 @@ public CloseableHttpClient build() { if (!redirectHandlingDisabled) { RedirectStrategy redirectStrategyCopy = this.redirectStrategy; if (redirectStrategyCopy == null) { - redirectStrategyCopy = DefaultRedirectStrategy.INSTANCE; + redirectStrategyCopy = schemePortResolver != null ? new DefaultRedirectStrategy(schemePortResolver) : DefaultRedirectStrategy.INSTANCE; } execChainDefinition.addFirst( new RedirectExec(routePlannerCopy, redirectStrategyCopy), diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java index 406cb12772..ccbfc916f7 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRedirectStrategy.java @@ -324,4 +324,48 @@ void testRedirectAllowed() throws Exception { null)); } + + + + @Test + void testRedirectAllowedDefaultPortNormalization() { + final DefaultRedirectStrategy redirectStrategy = new DefaultRedirectStrategy(); + + // HTTPS with explicit 443 vs HTTPS with no port (defaults to 443) + final HttpHost explicitHttps = new HttpHost("https", "example.com", 443); + final HttpHost implicitHttps = new HttpHost("https", "example.com", -1); + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + explicitHttps, + implicitHttps, + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.AUTHORIZATION, "token") + .build(), + null)); + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + implicitHttps, + explicitHttps, + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.COOKIE, "cookie=123") + .build(), + null)); + + final HttpHost explicitHttp = new HttpHost("http", "example.org", 80); + final HttpHost implicitHttp = new HttpHost("http", "example.org", -1); + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + explicitHttp, + implicitHttp, + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.AUTHORIZATION, "token123") + .build(), + null)); + Assertions.assertTrue(redirectStrategy.isRedirectAllowed( + implicitHttp, + explicitHttp, + BasicRequestBuilder.get("/") + .addHeader(HttpHeaders.COOKIE, "cookie=abc") + .build(), + null)); + } + + } From a3c2ba30f8a11c04d451e34c64595903a98cb565 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 8 Jun 2025 14:49:26 +0200 Subject: [PATCH 02/29] Added Protocol conformance section to README --- README.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/README.md b/README.md index 80e48173c8..fcf3f5580c 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,22 @@ Other dependencies are optional. (for detailed information on external dependencies please see [pom.xml](./pom.xml)) +Protocol conformance +-------------------- + +- [RFC 9110](https://datatracker.ietf.org/doc/html/rfc9110) - HTTP Semantics +- [RFC 9111](https://datatracker.ietf.org/doc/html/rfc9111) - HTTP Caching +- [RFC 9112](https://datatracker.ietf.org/doc/html/rfc9112) - Hypertext Transfer Protocol Version 1.1 (HTTP/1.1) +- [RFC 7540](https://datatracker.ietf.org/doc/html/rfc7540) - Hypertext Transfer Protocol Version 2 (HTTP/2) +- [RFC 7541](https://datatracker.ietf.org/doc/html/rfc7541) - HPACK: Header Compression for HTTP/2 +- [RFC 1945](https://datatracker.ietf.org/doc/html/rfc1945) - Hypertext Transfer Protocol -- HTTP/1.0 +- [RFC 3986](https://datatracker.ietf.org/doc/html/rfc3986) - Uniform Resource Identifier (URI): Generic Syntax +- [RFC 6265](https://datatracker.ietf.org/doc/html/rfc6265) - HTTP State Management Mechanism (Cookies) +- [RFC 7616](https://datatracker.ietf.org/doc/html/rfc7616) - HTTP Digest Access Authentication +- [RFC 7617](https://datatracker.ietf.org/doc/html/rfc7617) - HTTP 'Basic' Authentication Scheme +- [RFC 5861](https://datatracker.ietf.org/doc/html/rfc5861) - HTTP Cache-Control Extensions for Stale Content +- [RFC 2817](https://datatracker.ietf.org/doc/html/rfc2817) - Upgrading to TLS Within HTTP/1.1 + Licensing --------- From 2d7abf5b9b68bc03082e527a8d0553c6dc10aa7d Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Wed, 18 Jun 2025 15:37:29 +0200 Subject: [PATCH 03/29] HTTPCLIENT-2376: fixed ContentCompressionExec not taking `acceptEncoding` parameter into account --- .../impl/classic/ContentCompressionExec.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java index acb62b2ccf..ac3cd5a559 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java @@ -86,16 +86,19 @@ public ContentCompressionExec( final Lookup decoderRegistry, final boolean ignoreUnknown) { - final boolean brotliSupported = BrotliDecompressingEntity.isAvailable(); - final List encodings = new ArrayList<>(4); - encodings.add("gzip"); - encodings.add("x-gzip"); - encodings.add("deflate"); - if (brotliSupported) { - encodings.add("br"); + final boolean brotliSupported = decoderRegistry == null && BrotliDecompressingEntity.isAvailable(); + if (acceptEncoding != null) { + this.acceptEncoding = MessageSupport.headerOfTokens(HttpHeaders.ACCEPT_ENCODING, acceptEncoding); + } else { + final List encodings = new ArrayList<>(4); + encodings.add("gzip"); + encodings.add("x-gzip"); + encodings.add("deflate"); + if (brotliSupported) { + encodings.add("br"); + } + this.acceptEncoding = MessageSupport.headerOfTokens(HttpHeaders.ACCEPT_ENCODING, encodings); } - this.acceptEncoding = MessageSupport.headerOfTokens(HttpHeaders.ACCEPT_ENCODING, encodings); - if (decoderRegistry != null) { this.decoderRegistry = decoderRegistry; } else { @@ -108,8 +111,6 @@ public ContentCompressionExec( } this.decoderRegistry = builder.build(); } - - this.ignoreUnknown = ignoreUnknown; } From 2c159a572a6ba46e147cf4acc582e16405d82b01 Mon Sep 17 00:00:00 2001 From: yhzdys Date: Thu, 3 Jul 2025 14:05:13 +0800 Subject: [PATCH 04/29] HTTPCLIENT-2379: Add H2SharingConnPool test for multiple removal of same PoolEntry (#665) (cherry picked from commit 5c6c135a8b33eadd3b6867fe280c7fd9b69dc1d9) --- .../http/impl/nio/H2SharingConnPoolTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPoolTest.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPoolTest.java index 26fa2e1cb6..a53043885e 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPoolTest.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPoolTest.java @@ -346,6 +346,36 @@ void testReleaseReusableAndClosedInCacheReturnedToPool() throws Exception { Mockito.eq(true)); } + /** + * Same connection can only be released once. + * Attempting to release it again will throw: IllegalStateException("Pool entry is not present in the set of leased entries") + * + * @see org.apache.hc.core5.pool.LaxConnPool.PerRoutePool#removeLeased(PoolEntry) + * @see org.apache.hc.core5.pool.StrictConnPool#release(PoolEntry, boolean) + */ + @Test + void testReleaseNonReusableNotInCacheReturnedToPool() throws Exception { + final PoolEntry poolEntry = new PoolEntry<>(DEFAULT_ROUTE); + poolEntry.assignConnection(connection); + Mockito.when(connection.isOpen()).thenReturn(false); + final H2SharingConnPool.PerRoutePool routePool = h2SharingPool.getPerRoutePool(DEFAULT_ROUTE); + routePool.track(poolEntry); + routePool.track(poolEntry); + + final AtomicReference connRef = new AtomicReference<>(connection); + Mockito.doAnswer(invocation -> { + final PoolEntry entry = invocation.getArgument(0); + if (!connRef.compareAndSet(entry.getConnection(), null)) { + throw new IllegalStateException("Pool entry is not present in the set of leased entries"); + } + return null; + }).when(connPool).release(Mockito.eq(poolEntry), Mockito.anyBoolean()); + + h2SharingPool.release(poolEntry, false); + // for reproduce https://issues.apache.org/jira/browse/HTTPCLIENT-2379 + Assertions.assertThrows(IllegalStateException.class, () -> h2SharingPool.release(poolEntry, false)); + } + @Test void testClose() throws Exception { h2SharingPool.close(); From 1ad3c31a6bf7ad524fa30a29b212561519a1e511 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Thu, 3 Jul 2025 11:52:53 +0200 Subject: [PATCH 05/29] HTTPASYNC-173 - Expose a protected constructor in DefaultAsyncClientConnectionOperator. (#666) (cherry picked from commit 7dbe7a29373131e2b6335a7ddaf44631c71cd1d2) --- .../impl/nio/DefaultAsyncClientConnectionOperator.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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 1d3531373f..4f04025236 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 @@ -69,7 +69,15 @@ public class DefaultAsyncClientConnectionOperator implements AsyncClientConnecti private final MultihomeIOSessionRequester sessionRequester; private final Lookup tlsStrategyLookup; - DefaultAsyncClientConnectionOperator( + /** + * Constructs a new {@code DefaultAsyncClientConnectionOperator}. + * + *

Note: this class is marked {@code @Internal}; rely on it + * only if you are prepared for incompatible changes in a future major + * release. Typical client code should use the high-level builders in + * {@code HttpAsyncClients} instead.

+ */ + protected DefaultAsyncClientConnectionOperator( final Lookup tlsStrategyLookup, final SchemePortResolver schemePortResolver, final DnsResolver dnsResolver) { From 3730eda581323c20781b50d7f6771136da8597ea Mon Sep 17 00:00:00 2001 From: Ryan Schmitt Date: Thu, 3 Jul 2025 19:16:59 -0700 Subject: [PATCH 06/29] Fix `validateAfterInactivity` on the async client `validateAfterInactivity` is primarily a synchronous client feature, since it is necessary to attempt to read from the underlying socket in order to discover that the connection has been closed by the remote peer. The async client doesn't have this issue; the IOReactor is automatically notified when connections are closed and removes them from the pool (see `SingleCoreIOReactor::processClosedSessions`). For some reason, however, the async client's connection manager treats `validateAfterInactivity` as a kind of connection TTL: if the connection has been idle longer than the `validateAfterInactivity` config value, then the connection is closed immediately unless it is an HTTP/2 connection, in which case a PING frame is used to check for liveness. Since stale connections are removed from the pool automatically as mentioned above, this change simply removes this connection close behavior. --- .../http/impl/nio/PoolingAsyncClientConnectionManager.java | 4 ---- 1 file changed, 4 deletions(-) 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 cd5473f2f1..3fbf34d197 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 @@ -314,10 +314,6 @@ public void completed(final PoolEntry p })), Command.Priority.IMMEDIATE); return; } - if (LOG.isDebugEnabled()) { - LOG.debug("{} connection {} is closed", id, ConnPoolSupport.getId(connection)); - } - poolEntry.discardConnection(CloseMode.IMMEDIATE); } } } From 4086951973ef9c83e8340e1eca88e3cb61dacea5 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sun, 6 Jul 2025 13:57:57 +0200 Subject: [PATCH 07/29] =?UTF-8?q?HTTPCLIENT-2379:=20PerRoutePool.release?= =?UTF-8?q?=20now=20decrements=20the=20counter=20first=20and=20keeps=20the?= =?UTF-8?q?=20entry=20in=20the=20map=20until=20the=20count=20reaches=20zer?= =?UTF-8?q?o,=20regardless=20of=20whether=20the=20connection=20is=20still?= =?UTF-8?q?=20open.=20The=20entry=20is=20handed=20back=20to=20the=20parent?= =?UTF-8?q?=20pool=20exactly=20once=E2=80=94on=20that=20final=20release?= =?UTF-8?q?=E2=80=94and=20any=20attempt=20to=20release=20an=20entry=20that?= =?UTF-8?q?=20was=20never=20leased=20now=20raises=20an=20IllegalStateExcep?= =?UTF-8?q?tion.=20(#663)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (cherry picked from commit b1efda3a35e5b991353c54e0d2ec17d7c6dcc299) --- .../http/impl/nio/H2SharingConnPool.java | 41 +++++++++-------- .../http/impl/nio/H2SharingConnPoolTest.java | 45 ++++--------------- .../impl/nio/H2SharingPerRoutePoolTest.java | 6 +-- 3 files changed, 32 insertions(+), 60 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPool.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPool.java index 2efc867fe1..57c219d3c4 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPool.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPool.java @@ -270,16 +270,17 @@ long track(final PoolEntry entry) { PoolEntry lease() { lock.lock(); try { - final PoolEntry entry = entryMap.entrySet().stream() + return entryMap.entrySet().stream() + .filter(e -> { + final C conn = e.getKey().getConnection(); + return conn != null && conn.isOpen(); + }) .min(Comparator.comparingLong(e -> e.getValue().get())) - .map(Map.Entry::getKey) + .map(e -> { + e.getValue().incrementAndGet(); + return e.getKey(); + }) .orElse(null); - if (entry == null) { - return null; - } - final AtomicLong counter = getCounter(entry); - counter.incrementAndGet(); - return entry; } finally { lock.unlock(); } @@ -288,20 +289,18 @@ PoolEntry lease() { long release(final PoolEntry entry, final boolean reusable) { lock.lock(); try { - final C connection = entry.getConnection(); - if (!reusable || connection == null || !connection.isOpen()) { - entryMap.remove(entry); - return 0; - } else { - final AtomicLong counter = entryMap.compute(entry, (e, c) -> { - if (c == null) { - return null; - } - final long count = c.decrementAndGet(); - return count > 0 ? c : null; - }); - return counter != null ? counter.get() : 0L; + if (!reusable) { + entry.discardConnection(CloseMode.GRACEFUL); } + + final AtomicLong counter = entryMap.compute(entry, (e, c) -> { + if (c == null) { + return null; + } + final long count = c.decrementAndGet(); + return count > 0 ? c : null; + }); + return counter != null ? counter.get() : 0L; } finally { lock.unlock(); } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPoolTest.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPoolTest.java index a53043885e..077db0ff81 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPoolTest.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPoolTest.java @@ -86,10 +86,16 @@ void testLeaseFutureReturned() throws Exception { @Test void testLeaseExistingConnectionReturned() throws Exception { final PoolEntry poolEntry = new PoolEntry<>(DEFAULT_ROUTE); - final H2SharingConnPool.PerRoutePool routePool = h2SharingPool.getPerRoutePool(DEFAULT_ROUTE); + final HttpConnection conn = Mockito.mock(HttpConnection.class); + Mockito.when(conn.isOpen()).thenReturn(true); + poolEntry.assignConnection(conn); + + final H2SharingConnPool.PerRoutePool routePool = + h2SharingPool.getPerRoutePool(DEFAULT_ROUTE); routePool.track(poolEntry); + final Future> future = + h2SharingPool.lease(DEFAULT_ROUTE, null, Timeout.ONE_MILLISECOND, callback); - final Future> future = h2SharingPool.lease(DEFAULT_ROUTE, null, Timeout.ONE_MILLISECOND, callback); Assertions.assertNotNull(future); Assertions.assertSame(poolEntry, future.get()); @@ -98,8 +104,7 @@ void testLeaseExistingConnectionReturned() throws Exception { Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.verify(callback).completed( - Mockito.same(poolEntry)); + Mockito.verify(callback).completed(Mockito.same(poolEntry)); } @Test @@ -314,38 +319,6 @@ void testReleaseReusableInCacheNotReturnedToPool() throws Exception { Mockito.anyBoolean()); } - @Test - void testReleaseNonReusableInCacheReturnedToPool() throws Exception { - final PoolEntry poolEntry = new PoolEntry<>(DEFAULT_ROUTE); - poolEntry.assignConnection(connection); - Mockito.when(connection.isOpen()).thenReturn(true); - final H2SharingConnPool.PerRoutePool routePool = h2SharingPool.getPerRoutePool(DEFAULT_ROUTE); - routePool.track(poolEntry); - routePool.track(poolEntry); - - h2SharingPool.release(poolEntry, false); - - Mockito.verify(connPool).release( - Mockito.same(poolEntry), - Mockito.eq(false)); - } - - @Test - void testReleaseReusableAndClosedInCacheReturnedToPool() throws Exception { - final PoolEntry poolEntry = new PoolEntry<>(DEFAULT_ROUTE); - poolEntry.assignConnection(connection); - Mockito.when(connection.isOpen()).thenReturn(false); - final H2SharingConnPool.PerRoutePool routePool = h2SharingPool.getPerRoutePool(DEFAULT_ROUTE); - routePool.track(poolEntry); - routePool.track(poolEntry); - - h2SharingPool.release(poolEntry, true); - - Mockito.verify(connPool).release( - Mockito.same(poolEntry), - Mockito.eq(true)); - } - /** * Same connection can only be released once. * Attempting to release it again will throw: IllegalStateException("Pool entry is not present in the set of leased entries") diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingPerRoutePoolTest.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingPerRoutePoolTest.java index dc1a573fa5..b956fb08c5 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingPerRoutePoolTest.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingPerRoutePoolTest.java @@ -98,7 +98,7 @@ void testReleaseNonReusable() { pool.track(poolEntry1); pool.track(poolEntry1); - Assertions.assertEquals(0, pool.release(poolEntry1, false)); + Assertions.assertEquals(2, pool.release(poolEntry1, false)); // 3 โ†’ 2 } @Test @@ -114,7 +114,7 @@ void testReleaseConnectionClosed() { pool.track(poolEntry1); Mockito.when(poolEntry1.getConnection().isOpen()).thenReturn(false); - Assertions.assertEquals(0, pool.release(poolEntry1, true)); + Assertions.assertEquals(2, pool.release(poolEntry1, true)); // 3 โ†’ 2 } @Test @@ -124,7 +124,7 @@ void testReleaseConnectionMissing() { pool.track(poolEntry1); poolEntry1.discardConnection(CloseMode.IMMEDIATE); - Assertions.assertEquals(0, pool.release(poolEntry1, true)); + Assertions.assertEquals(2, pool.release(poolEntry1, true)); // 3 โ†’ 2 } } From 7046d99fa77b8146da1d32e287c763205170f360 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 6 Jul 2025 17:51:11 +0200 Subject: [PATCH 08/29] Fixed JUnit 5 dependency declaration in httpclient5-testing --- httpclient5-testing/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/httpclient5-testing/pom.xml b/httpclient5-testing/pom.xml index 88baa4329f..125e16d896 100644 --- a/httpclient5-testing/pom.xml +++ b/httpclient5-testing/pom.xml @@ -79,7 +79,7 @@ org.junit.jupiter - junit-jupiter-params + junit-jupiter test From aab688bb4a6732be20f587f3a1620125c7d2aa35 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sat, 5 Jul 2025 17:40:03 +0200 Subject: [PATCH 09/29] HTTPCLIENT-2141: revision of the original implementation / better debug logging / additional integration tests --- .../classic/ServiceUnavailableDecorator.java | 80 ++++++++++++ .../testing/sync/HttpIntegrationTests.java | 20 +++ .../sync/TestClientRequestReExecution.java | 118 ++++++++++++++++++ .../hc/client5/http/config/RequestConfig.java | 6 + .../impl/DefaultHttpRequestRetryStrategy.java | 14 ++- .../impl/async/AsyncHttpRequestRetryExec.java | 15 ++- .../impl/classic/HttpRequestRetryExec.java | 86 +++++-------- .../TestDefaultHttpRequestRetryStrategy.java | 28 ++++- .../classic/TestHttpRequestRetryExec.java | 33 ----- 9 files changed, 307 insertions(+), 93 deletions(-) create mode 100644 httpclient5-testing/src/main/java/org/apache/hc/client5/testing/classic/ServiceUnavailableDecorator.java create mode 100644 httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestReExecution.java diff --git a/httpclient5-testing/src/main/java/org/apache/hc/client5/testing/classic/ServiceUnavailableDecorator.java b/httpclient5-testing/src/main/java/org/apache/hc/client5/testing/classic/ServiceUnavailableDecorator.java new file mode 100644 index 0000000000..3a00cbfd56 --- /dev/null +++ b/httpclient5-testing/src/main/java/org/apache/hc/client5/testing/classic/ServiceUnavailableDecorator.java @@ -0,0 +1,80 @@ +/* + * ==================================================================== + * 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 + * . + * + */ + +package org.apache.hc.client5.testing.classic; + +import java.io.IOException; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.apache.hc.core5.function.Resolver; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.HttpVersion; +import org.apache.hc.core5.http.ProtocolVersion; +import org.apache.hc.core5.http.io.HttpServerRequestHandler; +import org.apache.hc.core5.http.message.BasicClassicHttpResponse; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.util.Args; +import org.apache.hc.core5.util.TimeValue; + +public class ServiceUnavailableDecorator implements HttpServerRequestHandler { + + private final HttpServerRequestHandler requestHandler; + private final Resolver serviceAvailabilityResolver; + private final AtomicBoolean serviceUnavailable; + + public ServiceUnavailableDecorator(final HttpServerRequestHandler requestHandler, + final Resolver serviceAvailabilityResolver) { + this.requestHandler = Args.notNull(requestHandler, "Request handler"); + this.serviceAvailabilityResolver = Args.notNull(serviceAvailabilityResolver, "Service availability resolver"); + this.serviceUnavailable = new AtomicBoolean(); + } + + @Override + public void handle(final ClassicHttpRequest request, + final ResponseTrigger responseTrigger, + final HttpContext context) throws HttpException, IOException { + final TimeValue retryAfter = serviceAvailabilityResolver.resolve(request); + serviceUnavailable.set(TimeValue.isPositive(retryAfter)); + if (serviceUnavailable.get()) { + final ClassicHttpResponse response = new BasicClassicHttpResponse(HttpStatus.SC_SERVICE_UNAVAILABLE); + response.addHeader(HttpHeaders.RETRY_AFTER, Long.toString(retryAfter.toSeconds())); + final ProtocolVersion version = request.getVersion(); + if (version != null && version.compareToVersion(HttpVersion.HTTP_2) < 0) { + response.addHeader(HttpHeaders.CONNECTION, "Close"); + } + responseTrigger.submitResponse(response); + } else { + requestHandler.handle(request, responseTrigger, context); + } + } + +} diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/HttpIntegrationTests.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/HttpIntegrationTests.java index f162795f67..9b0b191f77 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/HttpIntegrationTests.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/HttpIntegrationTests.java @@ -112,4 +112,24 @@ public RedirectsTls() { } + @Nested + @DisplayName("Request re-execution (HTTP/1.1)") + class RequestReExecution extends TestClientRequestReExecution { + + public RequestReExecution() { + super(URIScheme.HTTP); + } + + } + + @Nested + @DisplayName("Request re-execution (HTTP/1.1)") + class RequestReExecutionTls extends TestClientRequestReExecution { + + public RequestReExecutionTls() { + super(URIScheme.HTTPS); + } + + } + } \ No newline at end of file diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestReExecution.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestReExecution.java new file mode 100644 index 0000000000..30b36b82d4 --- /dev/null +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestReExecution.java @@ -0,0 +1,118 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.client5.testing.sync; + +import static org.hamcrest.MatcherAssert.assertThat; + +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.client5.testing.classic.RandomHandler; +import org.apache.hc.client5.testing.classic.ServiceUnavailableDecorator; +import org.apache.hc.client5.testing.extension.sync.ClientProtocolLevel; +import org.apache.hc.client5.testing.extension.sync.TestClient; +import org.apache.hc.core5.function.Resolver; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.HttpHost; +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.URIScheme; +import org.apache.hc.core5.http.io.entity.EntityUtils; +import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; +import org.apache.hc.core5.util.TimeValue; +import org.hamcrest.CoreMatchers; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +abstract class TestClientRequestReExecution extends AbstractIntegrationTestBase { + + public TestClientRequestReExecution(final URIScheme scheme) { + super(scheme, ClientProtocolLevel.STANDARD); + } + + @BeforeEach + void setup() { + final Resolver serviceAvailabilityResolver = new Resolver() { + + private final AtomicInteger count = new AtomicInteger(0); + + @Override + public TimeValue resolve(final HttpRequest request) { + final int n = count.incrementAndGet(); + return n <= 3 ? TimeValue.ofSeconds(1) : null; + } + + }; + + configureServer(bootstrap -> bootstrap.setExchangeHandlerDecorator(handler + -> new ServiceUnavailableDecorator(handler, serviceAvailabilityResolver))); + } + + @Test + void testGiveUpAfterOneRetry() throws Exception { + configureServer(bootstrap -> bootstrap.register("/random/*", new RandomHandler())); + final HttpHost target = startServer(); + + configureClient(builder -> builder + .setRetryStrategy(new DefaultHttpRequestRetryStrategy(1, TimeValue.ofSeconds(1)))); + final TestClient client = client(); + + final HttpClientContext context = HttpClientContext.create(); + final ClassicHttpRequest request = ClassicRequestBuilder.get() + .setHttpHost(target) + .setPath("/random/2048") + .build(); + final HttpResponse response = client.execute(target, request, context, r -> { + EntityUtils.consume(r.getEntity()); + return r; + }); + assertThat(response.getCode(), CoreMatchers.equalTo(HttpStatus.SC_SERVICE_UNAVAILABLE)); + } + + @Test + void testDoNotGiveUpEasily() throws Exception { + configureServer(bootstrap -> bootstrap.register("/random/*", new RandomHandler())); + final HttpHost target = startServer(); + + configureClient(builder -> builder + .setRetryStrategy(new DefaultHttpRequestRetryStrategy(5, TimeValue.ofSeconds(1)))); + final TestClient client = client(); + + final HttpClientContext context = HttpClientContext.create(); + final ClassicHttpRequest request = ClassicRequestBuilder.get() + .setHttpHost(target) + .setPath("/random/2048") + .build(); + final HttpResponse response = client.execute(target, request, context, r -> { + EntityUtils.consume(r.getEntity()); + return r; + }); + assertThat(response.getCode(), CoreMatchers.equalTo(HttpStatus.SC_OK)); + } + +} diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java b/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java index d0d70fb482..186182c4f9 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java @@ -517,6 +517,12 @@ public Builder setConnectTimeout(final long connectTimeout, final TimeUnit timeU * HTTP transports with message multiplexing. *

*

+ * Please note that response timeout is not a deadline. Its absolute value + * can be exceeded, for example, in case of automatic request re-execution. + * Please make sure the automatic request re-execution policy has been + * configured appropriately. + *

+ *

* Default: {@code null} *

* diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultHttpRequestRetryStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultHttpRequestRetryStrategy.java index b8dfcb535b..47c9bab567 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultHttpRequestRetryStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultHttpRequestRetryStrategy.java @@ -41,6 +41,8 @@ import javax.net.ssl.SSLException; import org.apache.hc.client5.http.HttpRequestRetryStrategy; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; @@ -55,6 +57,7 @@ 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; /** * Default implementation of the {@link HttpRequestRetryStrategy} interface. @@ -95,7 +98,8 @@ protected DefaultHttpRequestRetryStrategy( final Collection> clazzes, final Collection codes) { Args.notNegative(maxRetries, "maxRetries"); - Args.notNegative(defaultRetryInterval.getDuration(), "defaultRetryInterval"); + Args.notNull(defaultRetryInterval, "defaultRetryInterval"); + Args.check(TimeValue.isNonNegative(defaultRetryInterval), "Default retry interval is negative"); this.maxRetries = maxRetries; this.defaultRetryInterval = defaultRetryInterval; this.nonRetriableIOExceptionClasses = new HashSet<>(clazzes); @@ -199,6 +203,14 @@ public boolean retryRequest( final HttpContext context) { Args.notNull(response, "response"); + if (context != null) { + final HttpClientContext clientContext = HttpClientContext.cast(context); + final RequestConfig requestConfig = clientContext.getRequestConfigOrDefault(); + final Timeout responseTimeout = requestConfig.getResponseTimeout(); + if (responseTimeout != null && defaultRetryInterval.compareTo(responseTimeout) > 0) { + return false; + } + } return execCount <= this.maxRetries && retriableCodes.contains(response.getCode()); } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java index 292b6ed070..90beadff76 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java @@ -126,9 +126,6 @@ public AsyncDataConsumer handleResponse( state.retrying = retryStrategy.retryRequest(response, scope.execCount.get(), clientContext); if (state.retrying) { state.delay = retryStrategy.getRetryInterval(response, scope.execCount.get(), clientContext); - if (LOG.isDebugEnabled()) { - LOG.debug("{} retrying request in {}", exchangeId, state.delay); - } return new DiscardingEntityConsumer<>(); } return asyncExecCallback.handleResponse(response, entityDetails); @@ -146,13 +143,17 @@ public void completed() { if (entityProducer != null) { entityProducer.releaseResources(); } + final TimeValue delay = TimeValue.isPositive(state.delay) ? state.delay : TimeValue.ZERO_MILLISECONDS; + if (LOG.isDebugEnabled()) { + LOG.debug("{} wait for {}", exchangeId, delay); + } scope.scheduler.scheduleExecution( request, entityProducer, scope, (r, e, s, c) -> execute(r, e, s, chain, c), asyncExecCallback, - state.delay); + delay); } else { asyncExecCallback.completed(); } @@ -182,13 +183,17 @@ public void failed(final Exception cause) { state.retrying = true; final int execCount = scope.execCount.incrementAndGet(); state.delay = retryStrategy.getRetryInterval(request, (IOException) cause, execCount - 1, clientContext); + final TimeValue delay = TimeValue.isPositive(state.delay) ? state.delay : TimeValue.ZERO_MILLISECONDS; + if (LOG.isDebugEnabled()) { + LOG.debug("{} wait for {}", exchangeId, delay); + } scope.scheduler.scheduleExecution( request, entityProducer, scope, (r, e, s, c) -> execute(r, e, s, chain, c), asyncExecCallback, - state.delay); + delay); return; } } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java index 2ab32b6644..2e0021085e 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java @@ -34,7 +34,6 @@ import org.apache.hc.client5.http.classic.ExecChain; import org.apache.hc.client5.http.classic.ExecChain.Scope; import org.apache.hc.client5.http.classic.ExecChainHandler; -import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.ChainElement; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.core5.annotation.Contract; @@ -48,7 +47,6 @@ import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; 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; @@ -108,9 +106,28 @@ public ClassicHttpResponse execute( ClassicHttpRequest currentRequest = request; for (int execCount = 1;; execCount++) { - final ClassicHttpResponse response; try { - response = chain.proceed(currentRequest, scope); + final ClassicHttpResponse response = chain.proceed(currentRequest, scope); + try { + final HttpEntity entity = request.getEntity(); + if (entity != null && !entity.isRepeatable()) { + if (LOG.isDebugEnabled()) { + LOG.debug("{} cannot retry non-repeatable request", exchangeId); + } + return response; + } + if (retryStrategy.retryRequest(response, execCount, context)) { + response.close(); + final TimeValue delay = retryStrategy.getRetryInterval(response, execCount, context); + pause(exchangeId, delay); + currentRequest = ClassicRequestBuilder.copy(scope.originalRequest).build(); + } else { + return response; + } + } catch (final RuntimeException ex) { + response.close(); + throw ex; + } } catch (final IOException ex) { if (scope.execRuntime.isExecutionAborted()) { throw new RequestFailedException("Request aborted"); @@ -130,18 +147,8 @@ public ClassicHttpResponse execute( LOG.info("Recoverable I/O exception ({}) caught when processing request to {}", ex.getClass().getName(), route); } - final TimeValue nextInterval = retryStrategy.getRetryInterval(request, ex, execCount, context); - if (TimeValue.isPositive(nextInterval)) { - try { - if (LOG.isDebugEnabled()) { - LOG.debug("{} wait for {}", exchangeId, nextInterval); - } - nextInterval.sleep(); - } catch (final InterruptedException e) { - Thread.currentThread().interrupt(); - throw new InterruptedIOException(); - } - } + final TimeValue delay = retryStrategy.getRetryInterval(request, ex, execCount, context); + pause(exchangeId, delay); currentRequest = ClassicRequestBuilder.copy(scope.originalRequest).build(); continue; } @@ -153,44 +160,19 @@ public ClassicHttpResponse execute( } throw ex; } + } + } + private static void pause(final String exchangeId, final TimeValue delay) throws InterruptedIOException { + if (LOG.isDebugEnabled()) { + LOG.debug("{} wait for {}", exchangeId, delay); + } + if (TimeValue.isPositive(delay)) { try { - final HttpEntity entity = request.getEntity(); - if (entity != null && !entity.isRepeatable()) { - if (LOG.isDebugEnabled()) { - LOG.debug("{} cannot retry non-repeatable request", exchangeId); - } - return response; - } - if (retryStrategy.retryRequest(response, execCount, context)) { - final TimeValue nextInterval = retryStrategy.getRetryInterval(response, execCount, context); - // Make sure the retry interval does not exceed the response timeout - if (TimeValue.isPositive(nextInterval)) { - final RequestConfig requestConfig = context.getRequestConfigOrDefault(); - final Timeout responseTimeout = requestConfig.getResponseTimeout(); - if (responseTimeout != null && nextInterval.compareTo(responseTimeout) > 0) { - return response; - } - } - response.close(); - if (TimeValue.isPositive(nextInterval)) { - try { - if (LOG.isDebugEnabled()) { - LOG.debug("{} wait for {}", exchangeId, nextInterval); - } - nextInterval.sleep(); - } catch (final InterruptedException e) { - Thread.currentThread().interrupt(); - throw new InterruptedIOException(); - } - } - currentRequest = ClassicRequestBuilder.copy(scope.originalRequest).build(); - } else { - return response; - } - } catch (final RuntimeException ex) { - response.close(); - throw ex; + delay.sleep(); + } catch (final InterruptedException e) { + Thread.currentThread().interrupt(); + throw new InterruptedIOException(); } } } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryStrategy.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryStrategy.java index 2440f5c989..4c2649b524 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryStrategy.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryStrategy.java @@ -33,16 +33,17 @@ import java.net.UnknownHostException; import java.time.Instant; import java.time.temporal.ChronoUnit; - import javax.net.ssl.SSLException; - import org.apache.hc.client5.http.classic.methods.HttpGet; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.client5.http.utils.DateUtils; import org.apache.hc.core5.http.ConnectionClosedException; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.message.BasicHttpResponse; import org.apache.hc.core5.util.TimeValue; +import org.apache.hc.core5.util.Timeout; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -74,6 +75,29 @@ void testBasics() { Assertions.assertEquals(TimeValue.ofMilliseconds(1234L), this.retryStrategy.getRetryInterval(response1, 1, null)); } + @Test + void testRetryRequestWithResponseTimeout() { + final HttpResponse response = new BasicHttpResponse(503, "Oopsie"); + + final HttpClientContext context = HttpClientContext.create(); + context.setRequestConfig(RequestConfig.custom() + .build()); + + Assertions.assertTrue(retryStrategy.retryRequest(response, 1, context)); + + context.setRequestConfig(RequestConfig.custom() + .setResponseTimeout(Timeout.ofMilliseconds(1234L)) + .build()); + + Assertions.assertTrue(retryStrategy.retryRequest(response, 1, context)); + + context.setRequestConfig(RequestConfig.custom() + .setResponseTimeout(Timeout.ofMilliseconds(1233L)) + .build()); + + Assertions.assertFalse(retryStrategy.retryRequest(response, 1, context)); + } + @Test void testRetryAfterHeaderAsLong() { final HttpResponse response = new BasicHttpResponse(503, "Oopsie"); diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpRequestRetryExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpRequestRetryExec.java index 5672587c50..006ef9a506 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpRequestRetryExec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpRequestRetryExec.java @@ -45,7 +45,6 @@ import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; import org.apache.hc.core5.util.TimeValue; -import org.apache.hc.core5.util.Timeout; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -138,38 +137,6 @@ void testRetrySleepOnIOException() throws Exception { Mockito.verify(nextInterval, Mockito.times(1)).sleep(); } - @Test - void testRetryIntervalGreaterResponseTimeout() throws Exception { - final HttpRoute route = new HttpRoute(target); - final HttpGet request = new HttpGet("/test"); - final HttpClientContext context = HttpClientContext.create(); - context.setRequestConfig(RequestConfig.custom() - .setResponseTimeout(Timeout.ofSeconds(3)) - .build()); - - final ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class); - - Mockito.when(chain.proceed( - Mockito.same(request), - Mockito.any())).thenReturn(response); - Mockito.when(retryStrategy.retryRequest( - Mockito.any(), - Mockito.anyInt(), - Mockito.any())).thenReturn(Boolean.TRUE, Boolean.FALSE); - Mockito.when(retryStrategy.getRetryInterval( - Mockito.any(), - Mockito.anyInt(), - Mockito.any())).thenReturn(TimeValue.ofSeconds(5)); - - final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context); - retryExec.execute(request, scope, chain); - - Mockito.verify(chain, Mockito.times(1)).proceed( - Mockito.any(), - Mockito.same(scope)); - Mockito.verify(response, Mockito.times(0)).close(); - } - @Test void testRetryIntervalResponseTimeoutNull() throws Exception { final HttpRoute route = new HttpRoute(target); From 90bd664dff82ffe3bf59bc8109ee1d59cec251c2 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 13 Jul 2025 10:35:57 +0200 Subject: [PATCH 10/29] HTTPCLIENT-2371: Logging of request re-execution at INFO priority --- .../impl/async/AsyncHttpRequestRetryExec.java | 25 +++++++++++-------- .../impl/classic/HttpRequestRetryExec.java | 25 +++++++++++-------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java index 90beadff76..325567cd6e 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java @@ -29,7 +29,6 @@ import java.io.IOException; import org.apache.hc.client5.http.HttpRequestRetryStrategy; -import org.apache.hc.client5.http.HttpRoute; import org.apache.hc.client5.http.async.AsyncExecCallback; import org.apache.hc.client5.http.async.AsyncExecChain; import org.apache.hc.client5.http.async.AsyncExecChainHandler; @@ -40,6 +39,7 @@ import org.apache.hc.core5.annotation.ThreadingBehavior; import org.apache.hc.core5.http.EntityDetails; import org.apache.hc.core5.http.HttpException; +import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpResponse; import org.apache.hc.core5.http.nio.AsyncDataConsumer; @@ -96,6 +96,7 @@ public AsyncHttpRequestRetryExec(final HttpRequestRetryStrategy retryStrategy) { private static class State { volatile boolean retrying; + volatile int status; volatile TimeValue delay; } @@ -125,6 +126,7 @@ public AsyncDataConsumer handleResponse( } state.retrying = retryStrategy.retryRequest(response, scope.execCount.get(), clientContext); if (state.retrying) { + state.status = response.getCode(); state.delay = retryStrategy.getRetryInterval(response, scope.execCount.get(), clientContext); return new DiscardingEntityConsumer<>(); } @@ -139,13 +141,16 @@ public void handleInformationResponse(final HttpResponse response) throws HttpEx @Override public void completed() { if (state.retrying) { - scope.execCount.incrementAndGet(); + final int execCount = scope.execCount.incrementAndGet(); if (entityProducer != null) { entityProducer.releaseResources(); } + final HttpHost target = scope.route.getTargetHost(); final TimeValue delay = TimeValue.isPositive(state.delay) ? state.delay : TimeValue.ZERO_MILLISECONDS; - if (LOG.isDebugEnabled()) { - LOG.debug("{} wait for {}", exchangeId, delay); + if (LOG.isInfoEnabled()) { + LOG.info("{} {} responded with status {}; " + + "request will be automatically re-executed in {} (exec count {})", + exchangeId, target, state.status, delay, execCount); } scope.scheduler.scheduleExecution( request, @@ -162,7 +167,7 @@ public void completed() { @Override public void failed(final Exception cause) { if (cause instanceof IOException) { - final HttpRoute route = scope.route; + final HttpHost target = scope.route.getTargetHost(); final HttpClientContext clientContext = scope.clientContext; if (entityProducer != null && !entityProducer.isRepeatable()) { if (LOG.isDebugEnabled()) { @@ -172,10 +177,6 @@ public void failed(final Exception cause) { if (LOG.isDebugEnabled()) { LOG.debug("{} {}", exchangeId, cause.getMessage(), cause); } - if (LOG.isInfoEnabled()) { - LOG.info("Recoverable I/O exception ({}) caught when processing request to {}", - cause.getClass().getName(), route); - } scope.execRuntime.discardEndpoint(); if (entityProducer != null) { entityProducer.releaseResources(); @@ -184,8 +185,10 @@ public void failed(final Exception cause) { final int execCount = scope.execCount.incrementAndGet(); state.delay = retryStrategy.getRetryInterval(request, (IOException) cause, execCount - 1, clientContext); final TimeValue delay = TimeValue.isPositive(state.delay) ? state.delay : TimeValue.ZERO_MILLISECONDS; - if (LOG.isDebugEnabled()) { - LOG.debug("{} wait for {}", exchangeId, delay); + if (LOG.isInfoEnabled()) { + LOG.info("{} recoverable I/O exception ({}) caught when sending request to {};" + + "request will be automatically re-executed in {} (exec count {})", + exchangeId, cause.getClass().getName(), target, delay, execCount); } scope.scheduler.scheduleExecution( request, diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java index 2e0021085e..3dc801dd36 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java @@ -43,6 +43,7 @@ import org.apache.hc.core5.http.ClassicHttpResponse; 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.NoHttpResponseException; import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; import org.apache.hc.core5.util.Args; @@ -102,6 +103,7 @@ public ClassicHttpResponse execute( Args.notNull(scope, "scope"); final String exchangeId = scope.exchangeId; final HttpRoute route = scope.route; + final HttpHost target = route.getTargetHost(); final HttpClientContext context = scope.clientContext; ClassicHttpRequest currentRequest = request; @@ -119,7 +121,12 @@ public ClassicHttpResponse execute( if (retryStrategy.retryRequest(response, execCount, context)) { response.close(); final TimeValue delay = retryStrategy.getRetryInterval(response, execCount, context); - pause(exchangeId, delay); + if (LOG.isInfoEnabled()) { + LOG.info("{} {} responded with status {}; " + + "request will be automatically re-executed in {} (exec count {})", + exchangeId, target, response.getCode(), delay, execCount + 1); + } + pause(delay); currentRequest = ClassicRequestBuilder.copy(scope.originalRequest).build(); } else { return response; @@ -143,18 +150,19 @@ public ClassicHttpResponse execute( if (LOG.isDebugEnabled()) { LOG.debug("{} {}", exchangeId, ex.getMessage(), ex); } + final TimeValue delay = retryStrategy.getRetryInterval(request, ex, execCount, context); if (LOG.isInfoEnabled()) { - LOG.info("Recoverable I/O exception ({}) caught when processing request to {}", - ex.getClass().getName(), route); + LOG.info("{} recoverable I/O exception ({}) caught when sending request to {};" + + "request will be automatically re-executed in {} (exec count {})", + exchangeId, ex.getClass().getName(), target, delay, execCount + 1); } - final TimeValue delay = retryStrategy.getRetryInterval(request, ex, execCount, context); - pause(exchangeId, delay); + pause(delay); currentRequest = ClassicRequestBuilder.copy(scope.originalRequest).build(); continue; } if (ex instanceof NoHttpResponseException) { final NoHttpResponseException updatedex = new NoHttpResponseException( - route.getTargetHost().toHostString() + " failed to respond"); + target.toHostString() + " failed to respond"); updatedex.setStackTrace(ex.getStackTrace()); throw updatedex; } @@ -163,10 +171,7 @@ public ClassicHttpResponse execute( } } - private static void pause(final String exchangeId, final TimeValue delay) throws InterruptedIOException { - if (LOG.isDebugEnabled()) { - LOG.debug("{} wait for {}", exchangeId, delay); - } + private static void pause(final TimeValue delay) throws InterruptedIOException { if (TimeValue.isPositive(delay)) { try { delay.sleep(); From a93d27d908a1c184869940df741b123c86c4c76f Mon Sep 17 00:00:00 2001 From: "Gerlach, Winfried" Date: Mon, 28 Jul 2025 18:42:13 +0200 Subject: [PATCH 11/29] fix JavaDoc for PoolingHttpClientConnectionManager --- .../http/impl/io/PoolingHttpClientConnectionManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 8d509209a5..bf674f96c0 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 @@ -84,15 +84,15 @@ import org.slf4j.LoggerFactory; /** - * {@code ClientConnectionPoolManager} maintains a pool of + * {@code PoolingHttpClientConnectionManager} maintains a pool of * {@link ManagedHttpClientConnection}s and is able to service connection requests * from multiple execution threads. Connections are pooled on a per route * basis. A request for a route which already the manager has persistent * connections for available in the pool will be serviced by leasing * a connection from the pool rather than creating a new connection. *

- * {@code ClientConnectionPoolManager} maintains a maximum limit of connection - * on a per route basis and in total. Connection limits, however, can be adjusted + * {@code PoolingHttpClientConnectionManager} maintains a maximum limit of connections + * per route and in total. Connection limits, however, can be adjusted * using {@link ConnPoolControl} methods. *

* Total time to live (TTL) set at construction time defines maximum life span From da4d9a43212f6ff273e0e05c102d7da8b366becc Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 3 Aug 2025 13:49:17 +0200 Subject: [PATCH 12/29] HTTPCLIENT-2384: Socket options related to TcpKeepAlive are ignored --- .../impl/io/DefaultHttpClientConnectionOperator.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 f81bd797db..134d97b4cd 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 @@ -57,6 +57,7 @@ import org.apache.hc.core5.http.io.SocketConfig; import org.apache.hc.core5.http.protocol.HttpContext; import org.apache.hc.core5.io.Closer; +import org.apache.hc.core5.io.SocketSupport; import org.apache.hc.core5.net.NamedEndpoint; import org.apache.hc.core5.util.Args; import org.apache.hc.core5.util.TimeValue; @@ -197,7 +198,15 @@ public void connect( if (socketConfig.getSndBufSize() > 0) { socket.setSendBufferSize(socketConfig.getSndBufSize()); } - + if (socketConfig.getTcpKeepIdle() > 0) { + SocketSupport.setOption(socket, SocketSupport.TCP_KEEPIDLE, socketConfig.getTcpKeepIdle()); + } + if (socketConfig.getTcpKeepInterval() > 0) { + SocketSupport.setOption(socket, SocketSupport.TCP_KEEPINTERVAL, socketConfig.getTcpKeepInterval()); + } + if (socketConfig.getTcpKeepCount() > 0) { + SocketSupport.setOption(socket, SocketSupport.TCP_KEEPCOUNT, socketConfig.getTcpKeepCount()); + } final int linger = socketConfig.getSoLinger().toMillisecondsIntBound(); if (linger >= 0) { socket.setSoLinger(true, linger); From 8094bac428ad61622530d216ee763724985f612f Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Tue, 5 Aug 2025 11:37:41 +0200 Subject: [PATCH 13/29] HTTPCLIENT-2386: Classic transport to use connect timeout as a default if TLS timeout has not been explicitly set --- .../io/DefaultHttpClientConnectionOperator.java | 15 +++++++++++---- .../nio/DefaultAsyncClientConnectionOperator.java | 4 ++-- .../http/ssl/AbstractClientTlsStrategy.java | 8 -------- 3 files changed, 13 insertions(+), 14 deletions(-) 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 134d97b4cd..cebfcfd4d6 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 @@ -41,6 +41,7 @@ import org.apache.hc.client5.http.SchemePortResolver; import org.apache.hc.client5.http.SystemDefaultDnsResolver; import org.apache.hc.client5.http.UnsupportedSchemeException; +import org.apache.hc.client5.http.config.TlsConfig; import org.apache.hc.client5.http.impl.ConnPoolSupport; import org.apache.hc.client5.http.impl.DefaultSchemePortResolver; import org.apache.hc.client5.http.io.DetachedSocketFactory; @@ -160,7 +161,6 @@ public void connect( Args.notNull(socketConfig, "Socket config"); Args.notNull(context, "Context"); - final Timeout soTimeout = socketConfig.getSoTimeout(); final SocketAddress socksProxyAddress = socketConfig.getSocksProxyAddress(); final Proxy socksProxy = socksProxyAddress != null ? new Proxy(Proxy.Type.SOCKS, socksProxyAddress) : null; @@ -186,8 +186,9 @@ public void connect( socket.bind(localAddress); } conn.bind(socket); - if (soTimeout != null) { - socket.setSoTimeout(soTimeout.toMillisecondsIntBound()); + final Timeout socketTimeout = socketConfig.getSoTimeout(); + if (socketTimeout != null) { + socket.setSoTimeout(socketTimeout.toMillisecondsIntBound()); } socket.setReuseAddress(socketConfig.isSoReuseAddress()); socket.setTcpNoDelay(socketConfig.isTcpNoDelay()); @@ -217,7 +218,6 @@ public void connect( if (LOG.isDebugEnabled()) { LOG.debug("{} {} connected {}->{}", ConnPoolSupport.getId(conn), endpointHost, conn.getLocalAddress(), conn.getRemoteAddress()); } - conn.setSocketTimeout(soTimeout); final TlsSocketStrategy tlsSocketStrategy = tlsSocketStrategyLookup != null ? tlsSocketStrategyLookup.lookup(endpointHost.getSchemeName()) : null; if (tlsSocketStrategy != null) { final NamedEndpoint tlsName = endpointName != null ? endpointName : endpointHost; @@ -225,8 +225,15 @@ public void connect( if (LOG.isDebugEnabled()) { LOG.debug("{} {} upgrading to TLS", ConnPoolSupport.getId(conn), tlsName); } + final TlsConfig tlsConfig = attachment instanceof TlsConfig ? (TlsConfig) attachment : TlsConfig.DEFAULT; + final int soTimeout = socket.getSoTimeout(); + final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout() != null ? tlsConfig.getHandshakeTimeout() : connectTimeout; + if (handshakeTimeout != null) { + socket.setSoTimeout(handshakeTimeout.toMillisecondsIntBound()); + } final SSLSocket sslSocket = tlsSocketStrategy.upgrade(socket, tlsName.getHostName(), tlsName.getPort(), attachment, context); conn.bind(sslSocket, socket); + socket.setSoTimeout(soTimeout); onAfterTlsHandshake(context, endpointHost); if (LOG.isDebugEnabled()) { LOG.debug("{} {} upgraded to TLS", ConnPoolSupport.getId(conn), tlsName); 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 4f04025236..4a90e45a40 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 @@ -141,7 +141,7 @@ public void completed(final IOSession session) { if (tlsStrategy != null) { try { final Timeout socketTimeout = connection.getSocketTimeout(); - final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout(); + final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout() != null ? tlsConfig.getHandshakeTimeout() : connectTimeout; final NamedEndpoint tlsName = endpointName != null ? endpointName : endpointHost; onBeforeTlsHandshake(context, endpointHost); if (LOG.isDebugEnabled()) { @@ -151,7 +151,7 @@ public void completed(final IOSession session) { connection, tlsName, attachment, - handshakeTimeout != null ? handshakeTimeout : connectTimeout, + handshakeTimeout, new FutureContribution(future) { @Override 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 e4f6481f6e..35373c8650 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 @@ -220,8 +220,6 @@ private void executeHandshake( final SSLSocket upgradedSocket, final String target, final Object attachment) throws IOException { - final TlsConfig tlsConfig = attachment instanceof TlsConfig ? (TlsConfig) attachment : TlsConfig.DEFAULT; - final SSLParameters sslParameters = upgradedSocket.getSSLParameters(); if (supportedProtocols != null) { sslParameters.setProtocols(supportedProtocols); @@ -238,17 +236,11 @@ private void executeHandshake( } upgradedSocket.setSSLParameters(sslParameters); - final Timeout handshakeTimeout = tlsConfig.getHandshakeTimeout(); - if (handshakeTimeout != null) { - upgradedSocket.setSoTimeout(handshakeTimeout.toMillisecondsIntBound()); - } - initializeSocket(upgradedSocket); if (LOG.isDebugEnabled()) { LOG.debug("Enabled protocols: {}", (Object) upgradedSocket.getEnabledProtocols()); LOG.debug("Enabled cipher suites: {}", (Object) upgradedSocket.getEnabledCipherSuites()); - LOG.debug("Starting handshake ({})", handshakeTimeout); } upgradedSocket.startHandshake(); verifySession(target, upgradedSocket.getSession()); From ab40ec96cc755bbe6e189213521e56f9cbfc744d Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Fri, 29 Aug 2025 19:37:28 +0200 Subject: [PATCH 14/29] Upgraded HttpCore to version 5.3.5 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 03abba507e..844fe25dbc 100644 --- a/pom.xml +++ b/pom.xml @@ -62,7 +62,7 @@ 1.8 1.8 - 5.3.4 + 5.3.5 2.24.3 0.1.2 2.5.2 From da1a8e0fed0d83a14c4e39895ac190fdcfe18c4c Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 14 Sep 2025 14:29:48 +0200 Subject: [PATCH 15/29] Imporved TestAsyncClient wiring; added internal methods to get the underlying connection from the connection endpoint; coverage for async connection management operations --- .../async/TestAsyncConnectionManagement.java | 315 ++++++++++++++++++ .../async/MinimalTestClientBuilder.java | 27 +- .../async/StandardTestClientBuilder.java | 17 +- .../extension/async/TestAsyncClient.java | 13 +- .../async/TestAsyncClientBuilder.java | 9 +- .../extension/sync/TestClientBuilder.java | 4 +- .../sync/TestConnectionManagement.java | 4 +- .../client5/http/impl/ConnectionHolder.java | 41 +++ .../async/AbstractHttpAsyncClientBase.java | 19 +- .../io/BasicHttpClientConnectionManager.java | 9 +- .../PoolingHttpClientConnectionManager.java | 10 +- .../PoolingAsyncClientConnectionManager.java | 10 +- 12 files changed, 447 insertions(+), 31 deletions(-) create mode 100644 httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionManagement.java create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/impl/ConnectionHolder.java 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 new file mode 100644 index 0000000000..fa6f35ab55 --- /dev/null +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionManagement.java @@ -0,0 +1,315 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.client5.testing.async; + +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import org.apache.hc.client5.http.HttpRoute; +import org.apache.hc.client5.http.async.methods.SimpleHttpRequest; +import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; +import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder; +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.nio.PoolingAsyncClientConnectionManager; +import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder; +import org.apache.hc.client5.http.nio.AsyncConnectionEndpoint; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.client5.testing.extension.async.ClientProtocolLevel; +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.HttpHeaders; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.URIScheme; +import org.apache.hc.core5.http.protocol.HttpCoreContext; +import org.apache.hc.core5.pool.PoolConcurrencyPolicy; +import org.apache.hc.core5.pool.PoolReusePolicy; +import org.apache.hc.core5.util.TimeValue; +import org.apache.hc.core5.util.Timeout; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class TestAsyncConnectionManagement extends AbstractIntegrationTestBase { + + static final Timeout TIMEOUT = Timeout.ofSeconds(30); + static final Timeout LEASE_TIMEOUT = Timeout.ofSeconds(5); + + public TestAsyncConnectionManagement() { + super(URIScheme.HTTP, ClientProtocolLevel.STANDARD, ServerProtocolLevel.STANDARD); + } + + @BeforeEach + void setup() { + configureServer(bootstrap -> bootstrap.register("*", () -> new AbstractSimpleServerExchangeHandler() { + + @Override + protected SimpleHttpResponse handle( + final SimpleHttpRequest request, + final HttpCoreContext context) { + final SimpleHttpResponse response = new SimpleHttpResponse(HttpStatus.SC_OK); + response.setBody("Whatever", ContentType.TEXT_PLAIN); + return response; + } + })); + } + + /** + * Tests releasing and re-using a connection after a response is read. + */ + @Test + void testReleaseConnection() throws Exception { + final HttpHost target = startServer(); + + final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create() + .build(); + configureClient(builder -> builder.setConnectionManager(connManager)); + final TestAsyncClient client = startClient(); + + connManager.setMaxTotal(1); + + final HttpRoute route = new HttpRoute(target, null, false); + 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()); + + final SimpleHttpRequest request = SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/") + .addHeader(HttpHeaders.HOST, target.toHostString()) + .build(); + + 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()); + + // this should fail quickly, connection has not been released + final Future endpointFuture2 = connManager.lease("id2", route, null, TIMEOUT, null); + Assertions.assertThrows(TimeoutException.class, () -> endpointFuture2.get(10, TimeUnit.MILLISECONDS)); + endpointFuture2.cancel(true); + + // close and release the connection + // expect the next connection obtained to be closed + openEndpoint1.close(); + connManager.release(openEndpoint1, null, TimeValue.NEG_ONE_MILLISECOND); + + final Future endpointFuture3 = connManager.lease("id3", route, null, TIMEOUT, null); + final AsyncConnectionEndpoint endpoint2 = endpointFuture3.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit()); + Assertions.assertFalse(endpoint2.isConnected()); + + final Future connectFuture2 = connManager.connect(endpoint2, client.getImplementation(), TIMEOUT, null, context, null); + final AsyncConnectionEndpoint openEndpoint2 = connectFuture2.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + + final Future responseFuture2 = openEndpoint2.execute("ex-2", SimpleRequestProducer.create(request), SimpleResponseConsumer.create(), null); + final SimpleHttpResponse response2 = responseFuture2.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + Assertions.assertEquals(HttpStatus.SC_OK, response2.getCode()); + + // release connection keeping it open + // expect the next connection obtained to be open + connManager.release(openEndpoint2, null, TimeValue.NEG_ONE_MILLISECOND); + + final Future endpointFuture4 = connManager.lease("id4", route, null, TIMEOUT, null); + final AsyncConnectionEndpoint openEndpoint3 = endpointFuture4.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit()); + Assertions.assertTrue(openEndpoint3.isConnected()); + + final Future responseFuture3 = openEndpoint3.execute("ex-3", SimpleRequestProducer.create(request), SimpleResponseConsumer.create(), null); + final SimpleHttpResponse response3 = responseFuture3.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + Assertions.assertEquals(HttpStatus.SC_OK, response3.getCode()); + + connManager.release(openEndpoint3, null, TimeValue.NEG_ONE_MILLISECOND); + connManager.close(); + } + + /** + * Tests releasing with time limits. + */ + @Test + void testReleaseConnectionWithTimeLimits() throws Exception { + final HttpHost target = startServer(); + + final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create() + .build(); + configureClient(builder -> builder.setConnectionManager(connManager)); + final TestAsyncClient client = startClient(); + + connManager.setMaxTotal(1); + + final HttpRoute route = new HttpRoute(target, null, false); + 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()); + + final SimpleHttpRequest request = SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/") + .addHeader(HttpHeaders.HOST, target.toHostString()) + .build(); + + 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()); + + final Future endpointFuture2 = connManager.lease("id2", route, null, TIMEOUT, null); + Assertions.assertThrows(TimeoutException.class, () -> endpointFuture2.get(10, TimeUnit.MILLISECONDS)); + endpointFuture2.cancel(true); + + openEndpoint1.close(); + connManager.release(openEndpoint1, null, TimeValue.NEG_ONE_MILLISECOND); + + final Future endpointFuture3 = connManager.lease("id3", route, null, TIMEOUT, null); + final AsyncConnectionEndpoint endpoint2 = endpointFuture3.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit()); + Assertions.assertFalse(endpoint2.isConnected()); + + final Future connectFuture2 = connManager.connect(endpoint2, client.getImplementation(), TIMEOUT, null, context, null); + final AsyncConnectionEndpoint openEndpoint2 = connectFuture2.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + + final Future responseFuture2 = openEndpoint2.execute("ex-2", SimpleRequestProducer.create(request), SimpleResponseConsumer.create(), null); + final SimpleHttpResponse response2 = responseFuture2.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + Assertions.assertEquals(HttpStatus.SC_OK, response2.getCode()); + + connManager.release(openEndpoint2, null, TimeValue.ofMilliseconds(100)); + Thread.sleep(150); + + final Future endpointFuture4 = connManager.lease("id4", route, null, TIMEOUT, null); + final AsyncConnectionEndpoint openEndpoint3 = endpointFuture4.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit()); + Assertions.assertFalse(openEndpoint3.isConnected()); + connManager.release(openEndpoint3, null, TimeValue.ofMilliseconds(100)); + + connManager.close(); + } + + @Test + void testCloseExpiredIdleConnections() throws Exception { + final HttpHost target = startServer(); + + final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create() + .build(); + configureClient(builder -> builder.setConnectionManager(connManager)); + final TestAsyncClient client = startClient(); + + connManager.setMaxTotal(1); + + final HttpRoute route = new HttpRoute(target, null, false); + 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()); + + connManager.connect(endpoint1, client.getImplementation(), TIMEOUT, null, context, null) + .get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + + Assertions.assertEquals(1, connManager.getTotalStats().getLeased()); + Assertions.assertEquals(1, connManager.getStats(route).getLeased()); + + connManager.release(endpoint1, null, TimeValue.ofMilliseconds(100)); + + // Released, still active. + Assertions.assertEquals(1, connManager.getTotalStats().getAvailable()); + Assertions.assertEquals(1, connManager.getStats(route).getAvailable()); + + connManager.closeExpired(); + + // Time has not expired yet. + Assertions.assertEquals(1, connManager.getTotalStats().getAvailable()); + Assertions.assertEquals(1, connManager.getStats(route).getAvailable()); + + Thread.sleep(150); + + connManager.closeExpired(); + + // Time expired now, connections are destroyed. + Assertions.assertEquals(0, connManager.getTotalStats().getAvailable()); + Assertions.assertEquals(0, connManager.getStats(route).getAvailable()); + + connManager.close(); + } + + @Test + void testCloseExpiredTTLConnections() throws Exception { + final HttpHost target = startServer(); + + final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create() + .setPoolConcurrencyPolicy(PoolConcurrencyPolicy.STRICT) + .setConnPoolPolicy(PoolReusePolicy.LIFO) + .setDefaultConnectionConfig(ConnectionConfig.custom() + .setTimeToLive(TimeValue.ofMilliseconds(100)) + .build()) + .build(); + configureClient(builder -> builder.setConnectionManager(connManager)); + + final TestAsyncClient client = startClient(); + + connManager.setMaxTotal(1); + + final HttpRoute route = new HttpRoute(target, null, false); + 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()); + + connManager.connect(endpoint1, client.getImplementation(), TIMEOUT, null, context, null) + .get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + + Assertions.assertEquals(1, connManager.getTotalStats().getLeased()); + Assertions.assertEquals(1, connManager.getStats(route).getLeased()); + // Release, let remain idle for forever + connManager.release(endpoint1, null, TimeValue.NEG_ONE_MILLISECOND); + + // Released, still active. + Assertions.assertEquals(1, connManager.getTotalStats().getAvailable()); + Assertions.assertEquals(1, connManager.getStats(route).getAvailable()); + + connManager.closeExpired(); + + // Time has not expired yet. + Assertions.assertEquals(1, connManager.getTotalStats().getAvailable()); + Assertions.assertEquals(1, connManager.getStats(route).getAvailable()); + + Thread.sleep(150); + + connManager.closeExpired(); + + // TTL expired now, connections are destroyed. + Assertions.assertEquals(0, connManager.getTotalStats().getAvailable()); + Assertions.assertEquals(0, connManager.getStats(route).getAvailable()); + + connManager.close(); + } + +} diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/MinimalTestClientBuilder.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/MinimalTestClientBuilder.java index 5c8b1c0a9f..e99a78c679 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/MinimalTestClientBuilder.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/MinimalTestClientBuilder.java @@ -31,8 +31,8 @@ import org.apache.hc.client5.http.config.TlsConfig; import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; import org.apache.hc.client5.http.impl.async.HttpAsyncClients; -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.AsyncClientConnectionManager; import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.client5.testing.SSLTestContexts; import org.apache.hc.core5.http.config.Http1Config; @@ -45,6 +45,7 @@ final class MinimalTestClientBuilder implements TestAsyncClientBuilder { private final PoolingAsyncClientConnectionManagerBuilder connectionManagerBuilder; + private AsyncClientConnectionManager connectionManager; private Timeout timeout; private TlsStrategy tlsStrategy; private Http1Config http1Config; @@ -65,6 +66,12 @@ public TestAsyncClientBuilder setTimeout(final Timeout timeout) { return this; } + @Override + public TestAsyncClientBuilder setConnectionManager(final AsyncClientConnectionManager connectionManager) { + this.connectionManager = connectionManager; + return this; + } + @Override public TestAsyncClientBuilder setTlsStrategy(final TlsStrategy tlsStrategy) { this.tlsStrategy = tlsStrategy; @@ -97,21 +104,21 @@ public TestAsyncClientBuilder setH2Config(final H2Config h2Config) { @Override public TestAsyncClient build() throws Exception { - final PoolingAsyncClientConnectionManager connectionManager = connectionManagerBuilder + final AsyncClientConnectionManager connectionManagerCopy = connectionManager == null ? connectionManagerBuilder .setTlsStrategy(tlsStrategy != null ? tlsStrategy : new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext())) .setDefaultConnectionConfig(ConnectionConfig.custom() .setSocketTimeout(timeout) .setConnectTimeout(timeout) .build()) - .build(); + .build() : connectionManager; final CloseableHttpAsyncClient client = HttpAsyncClients.createMinimal( - h2Config, - http1Config, - IOReactorConfig.custom() - .setSoTimeout(timeout) - .build(), - connectionManager); - return new TestAsyncClient(client, connectionManager); + h2Config, + http1Config, + IOReactorConfig.custom() + .setSoTimeout(timeout) + .build(), + connectionManagerCopy); + return new TestAsyncClient(client, connectionManagerCopy); } } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/StandardTestClientBuilder.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/StandardTestClientBuilder.java index 1df164a8c6..11c291686a 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/StandardTestClientBuilder.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/StandardTestClientBuilder.java @@ -38,8 +38,8 @@ import org.apache.hc.client5.http.config.TlsConfig; import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder; -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.AsyncClientConnectionManager; import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy; import org.apache.hc.client5.testing.SSLTestContexts; import org.apache.hc.core5.http.Header; @@ -57,6 +57,7 @@ final class StandardTestClientBuilder implements TestAsyncClientBuilder { private final PoolingAsyncClientConnectionManagerBuilder connectionManagerBuilder; private final HttpAsyncClientBuilder clientBuilder; + private AsyncClientConnectionManager connectionManager; private Timeout timeout; private TlsStrategy tlsStrategy; @@ -76,6 +77,12 @@ public TestAsyncClientBuilder setTimeout(final Timeout timeout) { return this; } + @Override + public TestAsyncClientBuilder setConnectionManager(final AsyncClientConnectionManager connectionManager) { + this.connectionManager = connectionManager; + return this; + } + @Override public TestAsyncClientBuilder addResponseInterceptorFirst(final HttpResponseInterceptor interceptor) { this.clientBuilder.addResponseInterceptorFirst(interceptor); @@ -168,20 +175,20 @@ public TestAsyncClientBuilder setDefaultCredentialsProvider(final CredentialsPro @Override public TestAsyncClient build() throws Exception { - final PoolingAsyncClientConnectionManager connectionManager = connectionManagerBuilder + final AsyncClientConnectionManager connectionManagerCopy = connectionManager == null ? connectionManagerBuilder .setTlsStrategy(tlsStrategy != null ? tlsStrategy : new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext())) .setDefaultConnectionConfig(ConnectionConfig.custom() .setSocketTimeout(timeout) .setConnectTimeout(timeout) .build()) - .build(); + .build() : connectionManager; final CloseableHttpAsyncClient client = clientBuilder .setIOReactorConfig(IOReactorConfig.custom() .setSoTimeout(timeout) .build()) - .setConnectionManager(connectionManager) + .setConnectionManager(connectionManagerCopy) .build(); - return new TestAsyncClient(client, connectionManager); + return new TestAsyncClient(client, connectionManagerCopy); } } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/TestAsyncClient.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/TestAsyncClient.java index adefc45602..30563ebf2d 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/TestAsyncClient.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/TestAsyncClient.java @@ -31,7 +31,7 @@ import java.util.concurrent.Future; import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; -import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager; +import org.apache.hc.client5.http.nio.AsyncClientConnectionManager; import org.apache.hc.core5.concurrent.FutureCallback; import org.apache.hc.core5.function.Supplier; import org.apache.hc.core5.http.HttpHost; @@ -43,16 +43,15 @@ import org.apache.hc.core5.io.CloseMode; import org.apache.hc.core5.reactor.IOReactorStatus; import org.apache.hc.core5.util.Args; -import org.apache.hc.core5.util.Asserts; import org.apache.hc.core5.util.TimeValue; public class TestAsyncClient extends CloseableHttpAsyncClient { private final CloseableHttpAsyncClient client; - private final PoolingAsyncClientConnectionManager connectionManager; + private final AsyncClientConnectionManager connectionManager; public TestAsyncClient(final CloseableHttpAsyncClient client, - final PoolingAsyncClientConnectionManager connectionManager) { + final AsyncClientConnectionManager connectionManager) { this.client = Args.notNull(client, "Client"); this.connectionManager = connectionManager; } @@ -113,9 +112,9 @@ public T getImplementation() { return (T) client; } - public PoolingAsyncClientConnectionManager getConnectionManager() { - Asserts.check(connectionManager != null, "Connection manager is not available"); - return connectionManager; + @SuppressWarnings("unchecked") + public T getConnectionManager() { + return (T) connectionManager; } } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/TestAsyncClientBuilder.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/TestAsyncClientBuilder.java index 3f12b77cf0..cbb2878eff 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/TestAsyncClientBuilder.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/TestAsyncClientBuilder.java @@ -35,6 +35,7 @@ import org.apache.hc.client5.http.auth.AuthSchemeFactory; import org.apache.hc.client5.http.auth.CredentialsProvider; import org.apache.hc.client5.http.config.TlsConfig; +import org.apache.hc.client5.http.nio.AsyncClientConnectionManager; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpRequestInterceptor; import org.apache.hc.core5.http.HttpResponseInterceptor; @@ -50,8 +51,12 @@ public interface TestAsyncClientBuilder { TestAsyncClientBuilder setTimeout(Timeout soTimeout); - default TestAsyncClientBuilder addResponseInterceptorFirst(final HttpResponseInterceptor interceptor) { - return this; + default TestAsyncClientBuilder setConnectionManager(AsyncClientConnectionManager connManager) { + throw new UnsupportedOperationException("Operation not supported by " + getProtocolLevel()); + } + + default TestAsyncClientBuilder addResponseInterceptorFirst(HttpResponseInterceptor interceptor) { + throw new UnsupportedOperationException("Operation not supported by " + getProtocolLevel()); } default TestAsyncClientBuilder addResponseInterceptorLast(HttpResponseInterceptor interceptor) { diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java index 459124693f..57526ef2c1 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java @@ -51,8 +51,8 @@ public interface TestClientBuilder { TestClientBuilder setConnectionManager(HttpClientConnectionManager connManager); - default TestClientBuilder addResponseInterceptorFirst(final HttpResponseInterceptor interceptor) { - return this; + default TestClientBuilder addResponseInterceptorFirst(HttpResponseInterceptor interceptor) { + throw new UnsupportedOperationException("Operation not supported by " + getProtocolLevel()); } default TestClientBuilder addResponseInterceptorLast(HttpResponseInterceptor interceptor) { 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 8c215b2a88..c46f98e87c 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 @@ -129,6 +129,8 @@ void testReleaseConnection() throws Exception { final LeaseRequest leaseRequest2 = connManager.lease("id2", route, null); Assertions.assertThrows(TimeoutException.class, () -> leaseRequest2.get(Timeout.ofMilliseconds(10))); + // close and release the connection + // expect the next connection obtained to be closed endpoint1.close(); connManager.release(endpoint1, null, TimeValue.NEG_ONE_MILLISECOND); final LeaseRequest leaseRequest3 = connManager.lease("id2", route, null); @@ -141,7 +143,7 @@ void testReleaseConnection() throws Exception { Assertions.assertEquals(HttpStatus.SC_OK, response2.getCode()); } - // release connection after marking it for re-use + // release connection keeping it open // expect the next connection obtained to be open connManager.release(endpoint2, null, TimeValue.NEG_ONE_MILLISECOND); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ConnectionHolder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ConnectionHolder.java new file mode 100644 index 0000000000..33a1d3d04f --- /dev/null +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/ConnectionHolder.java @@ -0,0 +1,41 @@ +/* + * ==================================================================== + * 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 + * . + * + */ +package org.apache.hc.client5.http.impl; + +import org.apache.hc.core5.annotation.Internal; +import org.apache.hc.core5.http.HttpConnection; + +/** + * Internal interface to expose the underlying connection + * @since 5.5 + */ +@Internal +public interface ConnectionHolder { + + HttpConnection get(); + +} diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AbstractHttpAsyncClientBase.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AbstractHttpAsyncClientBase.java index 264faa2c70..e6b859c33d 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AbstractHttpAsyncClientBase.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AbstractHttpAsyncClientBase.java @@ -26,22 +26,28 @@ */ package org.apache.hc.client5.http.impl.async; +import java.net.SocketAddress; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicReference; +import org.apache.hc.core5.concurrent.FutureCallback; import org.apache.hc.core5.function.Supplier; import org.apache.hc.core5.http.nio.AsyncPushConsumer; import org.apache.hc.core5.io.CloseMode; +import org.apache.hc.core5.net.NamedEndpoint; import org.apache.hc.core5.reactor.ConnectionInitiator; import org.apache.hc.core5.reactor.DefaultConnectingIOReactor; import org.apache.hc.core5.reactor.IOReactorStatus; +import org.apache.hc.core5.reactor.IOSession; import org.apache.hc.core5.util.TimeValue; +import org.apache.hc.core5.util.Timeout; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -abstract class AbstractHttpAsyncClientBase extends CloseableHttpAsyncClient { +abstract class AbstractHttpAsyncClientBase extends CloseableHttpAsyncClient implements ConnectionInitiator { enum Status { READY, RUNNING, TERMINATED } @@ -84,6 +90,17 @@ boolean isRunning() { return status.get() == Status.RUNNING; } + @Override + public Future connect( + final NamedEndpoint remoteEndpoint, + final SocketAddress remoteAddress, + final SocketAddress localAddress, + final Timeout timeout, + final Object attachment, + final FutureCallback callback) { + return ioReactor.connect(remoteEndpoint, remoteAddress, localAddress, timeout, attachment, callback); + } + ConnectionInitiator getConnectionInitiator() { return ioReactor; } 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 ef99e78725..9ca40f9986 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 @@ -44,6 +44,7 @@ import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.config.TlsConfig; import org.apache.hc.client5.http.impl.ConnPoolSupport; +import org.apache.hc.client5.http.impl.ConnectionHolder; import org.apache.hc.client5.http.impl.ConnectionShutdownException; import org.apache.hc.client5.http.io.ConnectionEndpoint; import org.apache.hc.client5.http.io.HttpClientConnectionManager; @@ -56,6 +57,7 @@ import org.apache.hc.core5.annotation.ThreadingBehavior; 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.URIScheme; @@ -590,7 +592,7 @@ public void setValidateAfterInactivity(final TimeValue validateAfterInactivity) .build(); } - class InternalConnectionEndpoint extends ConnectionEndpoint implements Identifiable { + class InternalConnectionEndpoint extends ConnectionEndpoint implements ConnectionHolder, Identifiable { private final HttpRoute route; private final AtomicReference connRef; @@ -703,6 +705,11 @@ public EndpointInfo getInfo() { return null; } + @Override + public HttpConnection get() { + return this.connRef.get(); + } + } /** 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 bf674f96c0..e027f56a81 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 @@ -42,6 +42,7 @@ import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.config.TlsConfig; import org.apache.hc.client5.http.impl.ConnPoolSupport; +import org.apache.hc.client5.http.impl.ConnectionHolder; import org.apache.hc.client5.http.impl.ConnectionShutdownException; import org.apache.hc.client5.http.impl.PrefixedIncrementingId; import org.apache.hc.client5.http.io.ConnectionEndpoint; @@ -57,6 +58,7 @@ import org.apache.hc.core5.function.Resolver; 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.URIScheme; @@ -689,7 +691,7 @@ public void setValidateAfterInactivity(final TimeValue validateAfterInactivity) private static final PrefixedIncrementingId INCREMENTING_ID = new PrefixedIncrementingId("ep-"); - static class InternalConnectionEndpoint extends ConnectionEndpoint implements Identifiable { + static class InternalConnectionEndpoint extends ConnectionEndpoint implements ConnectionHolder, Identifiable { private final AtomicReference> poolEntryRef; private final String id; @@ -806,6 +808,12 @@ public EndpointInfo getInfo() { return null; } + @Override + public HttpConnection get() { + final PoolEntry poolEntry = poolEntryRef.get(); + return poolEntry != null ? poolEntry.getConnection() : 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 3fbf34d197..be7b5348e4 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 @@ -42,6 +42,7 @@ import org.apache.hc.client5.http.config.ConnectionConfig; import org.apache.hc.client5.http.config.TlsConfig; import org.apache.hc.client5.http.impl.ConnPoolSupport; +import org.apache.hc.client5.http.impl.ConnectionHolder; import org.apache.hc.client5.http.impl.ConnectionShutdownException; import org.apache.hc.client5.http.impl.PrefixedIncrementingId; import org.apache.hc.client5.http.nio.AsyncClientConnectionManager; @@ -57,6 +58,7 @@ import org.apache.hc.core5.concurrent.ComplexFuture; import org.apache.hc.core5.concurrent.FutureCallback; import org.apache.hc.core5.function.Resolver; +import org.apache.hc.core5.http.HttpConnection; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.HttpVersion; import org.apache.hc.core5.http.ProtocolVersion; @@ -676,7 +678,7 @@ public void setValidateAfterInactivity(final TimeValue validateAfterInactivity) private static final PrefixedIncrementingId INCREMENTING_ID = new PrefixedIncrementingId("ep-"); - static class InternalConnectionEndpoint extends AsyncConnectionEndpoint implements Identifiable { + static class InternalConnectionEndpoint extends AsyncConnectionEndpoint implements ConnectionHolder, Identifiable { private final AtomicReference> poolEntryRef; private final String id; @@ -773,6 +775,12 @@ public EndpointInfo getInfo() { return null; } + @Override + public HttpConnection get() { + final PoolEntry poolEntry = poolEntryRef.get(); + return poolEntry != null ? poolEntry.getConnection() : null; + } + } /** From 4466cca4a1021820586789e6bfeb885643451b58 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sun, 14 Sep 2025 18:37:09 +0200 Subject: [PATCH 16/29] HTTPCLIENT-2393 - remove rspauth from Authorization (#716) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 7616 compliance: rspauth is server-side (Authentication-Info ยง3.5) only. (cherry picked from commit 89da74220a11341b42ec455324b36094cc29f225) --- .../org/apache/hc/client5/http/impl/auth/DigestScheme.java | 1 - .../apache/hc/client5/http/impl/auth/TestDigestScheme.java | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java index 0771f9bd3b..7f57e2f78a 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/DigestScheme.java @@ -471,7 +471,6 @@ private String createDigestResponse(final HttpRequest request) throws Authentica params.add(new BasicNameValuePair("qop", qop == QualityOfProtection.AUTH_INT ? "auth-int" : "auth")); params.add(new BasicNameValuePair("nc", nc)); params.add(new BasicNameValuePair("cnonce", cnonce)); - params.add(new BasicNameValuePair("rspauth", hasha2)); } if (algorithm != null) { params.add(new BasicNameValuePair("algorithm", algorithm)); diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestDigestScheme.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestDigestScheme.java index 6b0fc5b59d..e44fe89558 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestDigestScheme.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/auth/TestDigestScheme.java @@ -903,7 +903,7 @@ void testDigestAuthenticationWithNonAsciiUsername() throws Exception { } @Test - void testRspAuthFieldAndQuoting() throws Exception { + void testRspAuthFieldNotPresentClient() throws Exception { final ClassicHttpRequest request = new BasicClassicHttpRequest("POST", "/"); final HttpHost host = new HttpHost("somehost", 80); final CredentialsProvider credentialsProvider = CredentialsProviderBuilder.create() @@ -921,7 +921,8 @@ void testRspAuthFieldAndQuoting() throws Exception { final Map table = parseAuthResponse(authResponse); - Assertions.assertNotNull(table.get("rspauth")); + Assertions.assertNotNull(table); + Assertions.assertNull(table.get("rspauth")); } @Test From cb2ccda8f2987a56eadfd500c94bc12d3be1e730 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 14 Sep 2025 21:07:23 +0200 Subject: [PATCH 17/29] 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)); From 6675520feeaefd7d063d2db234b19e980918a6a4 Mon Sep 17 00:00:00 2001 From: ChangYong Date: Sat, 23 Aug 2025 20:33:14 +0900 Subject: [PATCH 18/29] Clarify behavior of the protocol level responseTimeout and the connection management level socketTimeout and their interrelation --- .../hc/client5/http/config/ConnectionConfig.java | 10 +++++++++- .../apache/hc/client5/http/config/RequestConfig.java | 4 ++++ 2 files changed, 13 insertions(+), 1 deletion(-) 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 b372d461aa..4be79544a8 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 @@ -149,7 +149,15 @@ public Builder setSocketTimeout(final int soTimeout, final TimeUnit timeUnit) { } /** - * Determines the default socket timeout value for I/O operations. + * Determines the default socket timeout value for I/O operations on + * connections created by this configuration. + * A timeout value of zero is interpreted as an infinite timeout. + *

+ * This value acts as a baseline at the connection management layer. + * This parameter overrides the socket timeout setting applied at the I/O layer + * and in its tuen can overridden by settings applied at the protocol layer + * for the duration of a message exchange. + *

*

* Default: {@code null} (undefined) *

diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java b/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java index 186182c4f9..e5d791c56e 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/config/RequestConfig.java @@ -517,6 +517,10 @@ public Builder setConnectTimeout(final long connectTimeout, final TimeUnit timeU * HTTP transports with message multiplexing. *

*

+ * This parameter overrides the socket timeout setting applied at the connection + * management or I/O layers for the duration of a message exchange. + *

+ *

* Please note that response timeout is not a deadline. Its absolute value * can be exceeded, for example, in case of automatic request re-execution. * Please make sure the automatic request re-execution policy has been From 038b74f09e014a5a35deb316ff97d48141096c3e Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 15 Sep 2025 19:15:07 +0200 Subject: [PATCH 19/29] Bug fix: Use 1 second timeout when closing out connections inside a connection pool lock --- .../http/impl/io/PoolingHttpClientConnectionManager.java | 2 ++ .../http/impl/nio/PoolingAsyncClientConnectionManager.java | 2 ++ 2 files changed, 4 insertions(+) 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 6d8ccea485..c16eddce0c 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 @@ -70,6 +70,7 @@ import org.apache.hc.core5.http.protocol.HttpContext; import org.apache.hc.core5.io.CloseMode; import org.apache.hc.core5.pool.ConnPoolControl; +import org.apache.hc.core5.pool.DefaultDisposalCallback; import org.apache.hc.core5.pool.LaxConnPool; import org.apache.hc.core5.pool.ManagedConnPool; import org.apache.hc.core5.pool.PoolConcurrencyPolicy; @@ -223,6 +224,7 @@ public PoolingHttpClientConnectionManager( DEFAULT_MAX_TOTAL_CONNECTIONS, timeToLive, poolReusePolicy, + new DefaultDisposalCallback<>(), null) { @Override 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 512276181d..90d3df916c 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 @@ -77,6 +77,7 @@ import org.apache.hc.core5.http2.ssl.ApplicationProtocol; import org.apache.hc.core5.io.CloseMode; import org.apache.hc.core5.pool.ConnPoolControl; +import org.apache.hc.core5.pool.DefaultDisposalCallback; import org.apache.hc.core5.pool.LaxConnPool; import org.apache.hc.core5.pool.ManagedConnPool; import org.apache.hc.core5.pool.PoolConcurrencyPolicy; @@ -181,6 +182,7 @@ public PoolingAsyncClientConnectionManager( DEFAULT_MAX_TOTAL_CONNECTIONS, timeToLive, poolReusePolicy, + new DefaultDisposalCallback<>(), null) { @Override From 694394ca8f5bb05eb36f9ec70ab8101a3859ed49 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sat, 13 Sep 2025 22:14:51 +0200 Subject: [PATCH 20/29] HTTPCLIENT-2391: improved GRACEGUL shutdown of ExecutorService used internally by async clients --- .../impl/async/AbstractHttpAsyncClientBase.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AbstractHttpAsyncClientBase.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AbstractHttpAsyncClientBase.java index e6b859c33d..275b82b1c9 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AbstractHttpAsyncClientBase.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AbstractHttpAsyncClientBase.java @@ -31,6 +31,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import org.apache.hc.core5.concurrent.FutureCallback; @@ -133,7 +134,18 @@ public final void close(final CloseMode closeMode) { } ioReactor.initiateShutdown(); ioReactor.close(closeMode); - executorService.shutdownNow(); + if (closeMode == CloseMode.GRACEFUL) { + executorService.shutdown(); + try { + if (!executorService.awaitTermination(1, TimeUnit.SECONDS)) { + executorService.shutdownNow(); + } + } catch (final InterruptedException ignore) { + Thread.currentThread().interrupt(); + } + } else { + executorService.shutdownNow(); + } internalClose(closeMode); } From 61f21a5ee5bb853ba3985939a8f93e54fbbe0183 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sat, 20 Sep 2025 12:52:36 +0200 Subject: [PATCH 21/29] Upgraded HttpCore to version 5.3.6 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 844fe25dbc..4647c68bc0 100644 --- a/pom.xml +++ b/pom.xml @@ -62,7 +62,7 @@ 1.8 1.8 - 5.3.5 + 5.3.6 2.24.3 0.1.2 2.5.2 From 66dea80e40b48332f41722126a4ee7bef9a77d78 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sat, 20 Sep 2025 14:40:17 +0200 Subject: [PATCH 22/29] Updated release notes for HttpClient 5.5.1 release --- RELEASE_NOTES.txt | 53 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index ef49b3a9f0..557d642b9f 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,9 +1,62 @@ Release 5.5.1 ------------------ +This is a maintenance release that fixes several defects in the connection management +code and a regression in the DIGEST authentication reported since the previous release. +It also upgrades HttpCore to version 5.3.6. + + Change Log ------------------- +* HTTPCLIENT-2391: Improved GRACEGUL shutdown of ExecutorService used internally by async + clients. + Contributed by Oleg Kalnichevski + +* Bug fix: Use a 1 second timeout when closing out connections inside a connection pool lock. + Contributed by Oleg Kalnichevski + +* Clarified the behavior of the protocol-level responseTimeout and the connection management + level socketTimeout and their interrelation. + Contributed by ChangYong + +* Bug fix: Connection managers to ensure open connections have a socket timeout set based on + ConnectionConfig upon lease. + Contributed by Oleg Kalnichevski + +* HTTPCLIENT-2393: Remove `rspauth` attribute from `Authorization` DIGEST header (#716) + RFC 7616 compliance: rspauth is server-side (Authentication-Info 3.5) only. + Contributed by Arturo Bernal + +* HTTPCLIENT-2386: Classic transport to use the connect timeout as a default if the TLS timeout has + not been explicitly set. + Contributed by Oleg Kalnichevski + +* HTTPCLIENT-2384: Socket options related to TcpKeepAlive are ignored. + Contributed by Oleg Kalnichevski + +* HTTPCLIENT-2371: Logging of request re-execution at INFO priority. + Contributed by Oleg Kalnichevski + +* HTTPCLIENT-2379: Fixed a defect in H2SharingConnPool causing an IllegalStateException + when releasing the same connection from multiple threads. (#663) + Contributed by Arturo Bernal + +* Fixed the behavior of the `validateAfterInactivity` connection setting by the async + connection manager. + Contributed by Ryan Schmitt + +* HTTPCLIENT-2376: Fixed the problem with ContentCompressionExec not taking `acceptEncoding` + parameter into account. + Contributed by Oleg Kalnichevski + +* HTTPCLIENT-2372: Normalize HttpHost port comparison to treat implicit default ports as + equal (#643). + Contributed by Arturo Bernal + +* Maven wrapper + Contributed by Ryan Schmitt + * Bump org.junit:junit-bom from 5.12.2 to 5.13.0 #639. Contributed by Gary Gregory From 844f156a80ad914f5aec5a00db677b6f565ea0e6 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sat, 27 Sep 2025 18:17:50 +0200 Subject: [PATCH 23/29] Upgraded HttpClient version to 5.5.2-SNAPSHOT --- httpclient5-cache/pom.xml | 2 +- httpclient5-fluent/pom.xml | 2 +- httpclient5-testing/pom.xml | 2 +- httpclient5/pom.xml | 2 +- pom.xml | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/httpclient5-cache/pom.xml b/httpclient5-cache/pom.xml index 5d19f7ec41..79dd5bc095 100644 --- a/httpclient5-cache/pom.xml +++ b/httpclient5-cache/pom.xml @@ -28,7 +28,7 @@ org.apache.httpcomponents.client5 httpclient5-parent - 5.5.1-SNAPSHOT + 5.5.2-SNAPSHOT httpclient5-cache Apache HttpClient Cache diff --git a/httpclient5-fluent/pom.xml b/httpclient5-fluent/pom.xml index e98aecda40..6fdf8aad7c 100644 --- a/httpclient5-fluent/pom.xml +++ b/httpclient5-fluent/pom.xml @@ -28,7 +28,7 @@ org.apache.httpcomponents.client5 httpclient5-parent - 5.5.1-SNAPSHOT + 5.5.2-SNAPSHOT httpclient5-fluent Apache HttpClient Fluent diff --git a/httpclient5-testing/pom.xml b/httpclient5-testing/pom.xml index 125e16d896..42e20440da 100644 --- a/httpclient5-testing/pom.xml +++ b/httpclient5-testing/pom.xml @@ -28,7 +28,7 @@ org.apache.httpcomponents.client5 httpclient5-parent - 5.5.1-SNAPSHOT + 5.5.2-SNAPSHOT httpclient5-testing Apache HttpClient Integration Tests diff --git a/httpclient5/pom.xml b/httpclient5/pom.xml index 2cd51e18f2..4a0add9efb 100644 --- a/httpclient5/pom.xml +++ b/httpclient5/pom.xml @@ -28,7 +28,7 @@ org.apache.httpcomponents.client5 httpclient5-parent - 5.5.1-SNAPSHOT + 5.5.2-SNAPSHOT httpclient5 Apache HttpClient diff --git a/pom.xml b/pom.xml index 4647c68bc0..aacbe62fd9 100644 --- a/pom.xml +++ b/pom.xml @@ -33,7 +33,7 @@ org.apache.httpcomponents.client5 httpclient5-parent Apache HttpComponents Client Parent - 5.5.1-SNAPSHOT + 5.5.2-SNAPSHOT Apache HttpComponents Client is a library of components for building client side HTTP services https://hc.apache.org/httpcomponents-client-5.5.x/${project.version}/ 1999 @@ -48,7 +48,7 @@ scm:git:https://gitbox.apache.org/repos/asf/httpcomponents-client.git scm:git:https://gitbox.apache.org/repos/asf/httpcomponents-client.git https://github.com/apache/httpcomponents-client/tree/${project.scm.tag} - 5.5.1-SNAPSHOT + 5.5.2-SNAPSHOT From eb2a831ceb989930f984a9c22b3ce5ea792ebe5b Mon Sep 17 00:00:00 2001 From: Istvan Toth Date: Tue, 21 Oct 2025 17:19:01 +0200 Subject: [PATCH 24/29] HTTPCLIENT-2403: Mutual authentication check not performed for proxies (#745) --- .../hc/client5/http/impl/async/AsyncProtocolExec.java | 11 +++++------ .../client5/http/impl/auth/AuthenticationHandler.java | 4 ++-- .../hc/client5/http/impl/classic/ProtocolExec.java | 11 +++++------ 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java index 0d8c504627..1b35f8cb34 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java @@ -334,26 +334,25 @@ private boolean needAuthentication( } } + boolean targetNeedsAuth = false; + boolean proxyNeedsAuth = false; if (targetAuthRequested || targetMutualAuthRequired) { - final boolean updated = authenticator.handleResponse(target, ChallengeType.TARGET, response, + targetNeedsAuth = authenticator.handleResponse(target, ChallengeType.TARGET, response, targetAuthStrategy, targetAuthExchange, context); if (authCacheKeeper != null) { authCacheKeeper.updateOnResponse(target, pathPrefix, targetAuthExchange, context); } - - return updated; } if (proxyAuthRequested || proxyMutualAuthRequired) { - final boolean updated = authenticator.handleResponse(proxy, ChallengeType.PROXY, response, + proxyNeedsAuth = authenticator.handleResponse(proxy, ChallengeType.PROXY, response, proxyAuthStrategy, proxyAuthExchange, context); if (authCacheKeeper != null) { authCacheKeeper.updateOnResponse(proxy, null, proxyAuthExchange, context); } - - return updated; } + return targetNeedsAuth || proxyNeedsAuth; } return false; } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthenticationHandler.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthenticationHandler.java index 8d38ec8e75..fc7e4a22dd 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthenticationHandler.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/auth/AuthenticationHandler.java @@ -121,8 +121,8 @@ public boolean isChallenged( } /** - * Determines whether the given response represents an authentication challenge, without - * changing the {@link AuthExchange} state. + * Determines whether the given response represents an authentication challenge + * of challangeType, without changing the {@link AuthExchange} state. * * @param challengeType the challenge type (target or proxy). * @param response the response message head. diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java index fe3d35281f..b927dbc798 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java @@ -295,26 +295,25 @@ private boolean needAuthentication( } } + boolean targetNeedsAuth = false; + boolean proxyNeedsAuth = false; if (targetAuthRequested || targetMutualAuthRequired) { - final boolean updated = authenticator.handleResponse(target, ChallengeType.TARGET, response, + targetNeedsAuth = authenticator.handleResponse(target, ChallengeType.TARGET, response, targetAuthStrategy, targetAuthExchange, context); if (authCacheKeeper != null) { authCacheKeeper.updateOnResponse(target, pathPrefix, targetAuthExchange, context); } - - return updated; } if (proxyAuthRequested || proxyMutualAuthRequired) { - final boolean updated = authenticator.handleResponse(proxy, ChallengeType.PROXY, response, + proxyNeedsAuth = authenticator.handleResponse(proxy, ChallengeType.PROXY, response, proxyAuthStrategy, proxyAuthExchange, context); if (authCacheKeeper != null) { authCacheKeeper.updateOnResponse(proxy, null, proxyAuthExchange, context); } - - return updated; } + return targetNeedsAuth || proxyNeedsAuth; } return false; } From 9c83a8e43fb82467351a6d16b1ae78e0f6320873 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sun, 14 Dec 2025 23:08:24 +0100 Subject: [PATCH 25/29] Fixed incompatibility with HttpCore 5.4 --- .../DefaultHttpClientConnectionOperator.java | 29 +++++++++++++------ pom.xml | 2 +- 2 files changed, 21 insertions(+), 10 deletions(-) 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 cebfcfd4d6..a10c436b88 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 @@ -31,11 +31,14 @@ import java.net.Proxy; import java.net.Socket; import java.net.SocketAddress; +import java.util.Arrays; import java.util.Collections; import java.util.List; import javax.net.ssl.SSLSocket; +import jdk.net.ExtendedSocketOptions; +import jdk.net.Sockets; import org.apache.hc.client5.http.ConnectExceptionSupport; import org.apache.hc.client5.http.DnsResolver; import org.apache.hc.client5.http.SchemePortResolver; @@ -58,7 +61,6 @@ import org.apache.hc.core5.http.io.SocketConfig; import org.apache.hc.core5.http.protocol.HttpContext; import org.apache.hc.core5.io.Closer; -import org.apache.hc.core5.io.SocketSupport; import org.apache.hc.core5.net.NamedEndpoint; import org.apache.hc.core5.util.Args; import org.apache.hc.core5.util.TimeValue; @@ -79,6 +81,11 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection private static final Logger LOG = LoggerFactory.getLogger(DefaultHttpClientConnectionOperator.class); + @SuppressWarnings("Since15") + private static final boolean SUPPORTS_KEEPALIVE_OPTIONS = Sockets.supportedOptions(Socket.class) + .containsAll(Arrays.asList(ExtendedSocketOptions.TCP_KEEPIDLE, ExtendedSocketOptions.TCP_KEEPINTERVAL, + ExtendedSocketOptions.TCP_KEEPCOUNT)); + static final DetachedSocketFactory PLAIN_SOCKET_FACTORY = socksProxy -> socksProxy == null ? new Socket() : new Socket(socksProxy); private final DetachedSocketFactory detachedSocketFactory; @@ -145,6 +152,7 @@ public void connect( connect(conn, host, null, localAddress, timeout, socketConfig, null, context); } + @SuppressWarnings("Since15") @Override public void connect( final ManagedHttpClientConnection conn, @@ -199,14 +207,17 @@ public void connect( if (socketConfig.getSndBufSize() > 0) { socket.setSendBufferSize(socketConfig.getSndBufSize()); } - if (socketConfig.getTcpKeepIdle() > 0) { - SocketSupport.setOption(socket, SocketSupport.TCP_KEEPIDLE, socketConfig.getTcpKeepIdle()); - } - if (socketConfig.getTcpKeepInterval() > 0) { - SocketSupport.setOption(socket, SocketSupport.TCP_KEEPINTERVAL, socketConfig.getTcpKeepInterval()); - } - if (socketConfig.getTcpKeepCount() > 0) { - SocketSupport.setOption(socket, SocketSupport.TCP_KEEPCOUNT, socketConfig.getTcpKeepCount()); + if (SUPPORTS_KEEPALIVE_OPTIONS) { + if (socketConfig.getTcpKeepIdle() > 0) { + Sockets.setOption(socket, ExtendedSocketOptions.TCP_KEEPIDLE, socketConfig.getTcpKeepIdle()); + } + if (socketConfig.getTcpKeepInterval() > 0) { + Sockets.setOption(socket, ExtendedSocketOptions.TCP_KEEPINTERVAL, + socketConfig.getTcpKeepInterval()); + } + if (socketConfig.getTcpKeepCount() > 0) { + Sockets.setOption(socket, ExtendedSocketOptions.TCP_KEEPCOUNT, socketConfig.getTcpKeepCount()); + } } final int linger = socketConfig.getSoLinger().toMillisecondsIntBound(); if (linger >= 0) { diff --git a/pom.xml b/pom.xml index aacbe62fd9..407ab922ee 100644 --- a/pom.xml +++ b/pom.xml @@ -76,7 +76,7 @@ 2.2.21 1.21.1 5.3 - javax.net.ssl.SSLEngine,javax.net.ssl.SSLParameters,java.nio.ByteBuffer,java.nio.CharBuffer + javax.net.ssl.SSLEngine,javax.net.ssl.SSLParameters,java.nio.ByteBuffer,java.nio.CharBuffer,jdk.net.ExtendedSocketOptions,jdk.net.Sockets From 15f9d4f83f2fc3fc74aaf1c7342694b6fb9bfcbb Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 15 Dec 2025 10:53:44 +0100 Subject: [PATCH 26/29] Bump org.junit:junit-bom from 5.13.0 to 5.13.3 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 407ab922ee..8eed3d18e0 100644 --- a/pom.xml +++ b/pom.xml @@ -69,7 +69,7 @@ 3.10.8 2.12.3 1.7.36 - 5.13.0 + 5.13.3 3.0 4.11.0 1 From 87d86a1c99d73cd45f71cd89809ffb7cfcec6feb Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 15 Dec 2025 10:23:19 +0100 Subject: [PATCH 27/29] Limit the length of content codec list that can be processed automatically --- .../impl/classic/ContentCompressionExec.java | 26 +++++++++++++-- .../classic/TestContentCompressionExec.java | 32 +++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java index ac3cd5a559..0d88979479 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ContentCompressionExec.java @@ -54,6 +54,7 @@ import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.ProtocolException; import org.apache.hc.core5.http.config.Lookup; import org.apache.hc.core5.http.config.RegistryBuilder; import org.apache.hc.core5.http.message.BasicHeaderValueParser; @@ -77,14 +78,18 @@ @Internal public final class ContentCompressionExec implements ExecChainHandler { + public static final int MAX_CODEC_LIST_LEN = 5; + private final Header acceptEncoding; private final Lookup decoderRegistry; private final boolean ignoreUnknown; + private final int maxCodecListLen; public ContentCompressionExec( final List acceptEncoding, final Lookup decoderRegistry, - final boolean ignoreUnknown) { + final boolean ignoreUnknown, + final int maxCodecListLen) { final boolean brotliSupported = decoderRegistry == null && BrotliDecompressingEntity.isAvailable(); if (acceptEncoding != null) { @@ -112,10 +117,22 @@ public ContentCompressionExec( this.decoderRegistry = builder.build(); } this.ignoreUnknown = ignoreUnknown; + this.maxCodecListLen = maxCodecListLen; + } + + public ContentCompressionExec( + final List acceptEncoding, + final Lookup decoderRegistry, + final boolean ignoreUnknown) { + this(acceptEncoding, decoderRegistry, ignoreUnknown, MAX_CODEC_LIST_LEN); } public ContentCompressionExec(final boolean ignoreUnknown) { - this(null, null, ignoreUnknown); + this(null, null, ignoreUnknown, MAX_CODEC_LIST_LEN); + } + + public ContentCompressionExec(final int maxCodecListLen) { + this(null, null, true, maxCodecListLen); } /** @@ -128,7 +145,7 @@ public ContentCompressionExec(final boolean ignoreUnknown) { * */ public ContentCompressionExec() { - this(null, null, true); + this(null, null, true, MAX_CODEC_LIST_LEN); } @@ -158,6 +175,9 @@ public ClassicHttpResponse execute( if (contentEncoding != null) { final ParserCursor cursor = new ParserCursor(0, contentEncoding.length()); final HeaderElement[] codecs = BasicHeaderValueParser.INSTANCE.parseElements(contentEncoding, cursor); + if (maxCodecListLen > 0 && codecs.length > maxCodecListLen) { + throw new ProtocolException("Codec list exceeds maximum of " + maxCodecListLen + " elements"); + } for (final HeaderElement codec : codecs) { final String codecname = codec.getName().toLowerCase(Locale.ROOT); final InputStreamFactory decoderFactory = decoderRegistry.lookup(codecname); diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestContentCompressionExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestContentCompressionExec.java index 60eaa27f6f..13df4e32c8 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestContentCompressionExec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestContentCompressionExec.java @@ -40,6 +40,7 @@ import org.apache.hc.core5.http.HttpException; import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.Method; +import org.apache.hc.core5.http.ProtocolException; import org.apache.hc.core5.http.io.entity.StringEntity; import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.BasicClassicHttpResponse; @@ -236,4 +237,35 @@ void testContentEncodingRequestParameter() throws Exception { Assertions.assertFalse(entity instanceof GzipDecompressingEntity); } + @Test + void testContentEncodingExceedsCodecListLenMax() throws Exception { + impl = new ContentCompressionExec(5); + + final ClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, host, "/"); + final ClassicHttpResponse response1 = new BasicClassicHttpResponse(200, "OK"); + final HttpEntity original1 = EntityBuilder.create() + .setText("encoded stuff") + .setContentEncoding("gzip,gzip,gzip,gzip,gzip") + .build(); + response1.setEntity(original1); + + Mockito.when(execChain.proceed(request, scope)).thenReturn(response1); + + final HttpEntity entity = response1.getEntity(); + Assertions.assertNotNull(entity); + Assertions.assertFalse(entity instanceof GzipDecompressingEntity); + + final ClassicHttpResponse response2 = new BasicClassicHttpResponse(200, "OK"); + final HttpEntity original2 = EntityBuilder.create() + .setText("encoded stuff") + .setContentEncoding("gzip,gzip,gzip,gzip,gzip,gzip") + .build(); + response2.setEntity(original2); + + Mockito.when(execChain.proceed(request, scope)).thenReturn(response2); + + final ProtocolException exception = Assertions.assertThrows(ProtocolException.class, () -> impl.execute(request, scope, execChain)); + Assertions.assertEquals("Codec list exceeds maximum of 5 elements", exception.getMessage()); + } + } From 67a265eed4c6488fb7f1b99d65a66880ed869ed1 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Mon, 15 Dec 2025 21:59:36 +0100 Subject: [PATCH 28/29] Updated release notes for HttpClient 5.5.2 release --- RELEASE_NOTES.txt | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 557d642b9f..9c592ad278 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,3 +1,24 @@ +Release 5.5.2 +------------------ + +This is a maintenance release that fixes incompatibility with the 5.4 branch +of HttpCore. + + +Change Log +------------------- + +* Limit the length of the content codec list that can be processed automatically. + Contributed by Oleg Kalnichevski + +* Fixed incompatibility with HttpCore 5.4 + Contributed by Oleg Kalnichevski + +* HTTPCLIENT-2403: Mutual authentication check not performed for proxies (#745). + Contributed by Istvan Toth + + + Release 5.5.1 ------------------ @@ -13,7 +34,7 @@ Change Log clients. Contributed by Oleg Kalnichevski -* Bug fix: Use a 1 second timeout when closing out connections inside a connection pool lock. +* Bug fix: Use a 1-second timeout when closing out connections inside a connection pool lock. Contributed by Oleg Kalnichevski * Clarified the behavior of the protocol-level responseTimeout and the connection management @@ -146,7 +167,7 @@ Release 5.5 ALPHA1 ------------------ This is the first ALPHA release in the 5.5 release series. It adds several experimental -features and improvements such as request multiplexing over a shared HTTP/2 connection +features and improvements, such as request multiplexing over a shared HTTP/2 connection and the Classic API facade acting as a compatibility bridge between classic I/O client services and the asynchronous message transport used internally. From 7dfc2d3fc27e63c93f946de508a17c4b15ac6f49 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Fri, 19 Dec 2025 21:34:09 +0100 Subject: [PATCH 29/29] Upgraded HttpClient version to 5.5.3-SNAPSHOT --- httpclient5-cache/pom.xml | 2 +- httpclient5-fluent/pom.xml | 2 +- httpclient5-testing/pom.xml | 2 +- httpclient5/pom.xml | 2 +- pom.xml | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/httpclient5-cache/pom.xml b/httpclient5-cache/pom.xml index 79dd5bc095..3ab32dc44b 100644 --- a/httpclient5-cache/pom.xml +++ b/httpclient5-cache/pom.xml @@ -28,7 +28,7 @@ org.apache.httpcomponents.client5 httpclient5-parent - 5.5.2-SNAPSHOT + 5.5.3-SNAPSHOT httpclient5-cache Apache HttpClient Cache diff --git a/httpclient5-fluent/pom.xml b/httpclient5-fluent/pom.xml index 6fdf8aad7c..01eb1a1212 100644 --- a/httpclient5-fluent/pom.xml +++ b/httpclient5-fluent/pom.xml @@ -28,7 +28,7 @@ org.apache.httpcomponents.client5 httpclient5-parent - 5.5.2-SNAPSHOT + 5.5.3-SNAPSHOT httpclient5-fluent Apache HttpClient Fluent diff --git a/httpclient5-testing/pom.xml b/httpclient5-testing/pom.xml index 42e20440da..7be2f43cbc 100644 --- a/httpclient5-testing/pom.xml +++ b/httpclient5-testing/pom.xml @@ -28,7 +28,7 @@ org.apache.httpcomponents.client5 httpclient5-parent - 5.5.2-SNAPSHOT + 5.5.3-SNAPSHOT httpclient5-testing Apache HttpClient Integration Tests diff --git a/httpclient5/pom.xml b/httpclient5/pom.xml index 4a0add9efb..c175c761b6 100644 --- a/httpclient5/pom.xml +++ b/httpclient5/pom.xml @@ -28,7 +28,7 @@ org.apache.httpcomponents.client5 httpclient5-parent - 5.5.2-SNAPSHOT + 5.5.3-SNAPSHOT httpclient5 Apache HttpClient diff --git a/pom.xml b/pom.xml index 8eed3d18e0..eba25a72e8 100644 --- a/pom.xml +++ b/pom.xml @@ -33,7 +33,7 @@ org.apache.httpcomponents.client5 httpclient5-parent Apache HttpComponents Client Parent - 5.5.2-SNAPSHOT + 5.5.3-SNAPSHOT Apache HttpComponents Client is a library of components for building client side HTTP services https://hc.apache.org/httpcomponents-client-5.5.x/${project.version}/ 1999 @@ -48,7 +48,7 @@ scm:git:https://gitbox.apache.org/repos/asf/httpcomponents-client.git scm:git:https://gitbox.apache.org/repos/asf/httpcomponents-client.git https://github.com/apache/httpcomponents-client/tree/${project.scm.tag} - 5.5.2-SNAPSHOT + 5.5.3-SNAPSHOT