From f73d15a2d9a4d8b27d8b32909149261c0486e00a Mon Sep 17 00:00:00 2001 From: Alexis Le Dantec Date: Mon, 20 Oct 2025 15:49:43 +0200 Subject: [PATCH 1/3] Ensure connection is closed immediately upon socket timeout --- .../classic/ClassicIntegrationTest.java | 113 ++++++++++++++++-- .../http/impl/io/BHttpConnectionBase.java | 5 + 2 files changed, 109 insertions(+), 9 deletions(-) diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java index 22f27425cd..ab6aa4f562 100644 --- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java +++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java @@ -32,11 +32,16 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.net.SocketTimeoutException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.Random; +import java.util.concurrent.*; +import java.util.function.Consumer; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; @@ -51,7 +56,16 @@ import org.apache.hc.core5.http.Method; import org.apache.hc.core5.http.ProtocolException; import org.apache.hc.core5.http.URIScheme; +import org.apache.hc.core5.http.config.CharCodingConfig; import org.apache.hc.core5.http.config.Http1Config; +import org.apache.hc.core5.http.impl.DefaultAddressResolver; +import org.apache.hc.core5.http.impl.DefaultConnectionReuseStrategy; +import org.apache.hc.core5.http.impl.HttpProcessors; +import org.apache.hc.core5.http.impl.io.DefaultBHttpClientConnectionFactory; +import org.apache.hc.core5.http.impl.io.HttpRequestExecutor; +import org.apache.hc.core5.http.io.HttpClientConnection; +import org.apache.hc.core5.http.io.HttpConnectionFactory; +import org.apache.hc.core5.http.io.SocketConfig; import org.apache.hc.core5.http.io.entity.AbstractHttpEntity; import org.apache.hc.core5.http.io.entity.ByteArrayEntity; import org.apache.hc.core5.http.io.entity.EntityUtils; @@ -59,21 +73,21 @@ import org.apache.hc.core5.http.io.support.BasicHttpServerExpectationDecorator; import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.BasicClassicHttpResponse; -import org.apache.hc.core5.http.protocol.DefaultHttpProcessor; -import org.apache.hc.core5.http.protocol.HttpContext; -import org.apache.hc.core5.http.protocol.HttpCoreContext; -import org.apache.hc.core5.http.protocol.RequestConnControl; -import org.apache.hc.core5.http.protocol.RequestContent; -import org.apache.hc.core5.http.protocol.RequestExpectContinue; -import org.apache.hc.core5.http.protocol.RequestTE; -import org.apache.hc.core5.http.protocol.RequestTargetHost; -import org.apache.hc.core5.http.protocol.RequestUserAgent; +import org.apache.hc.core5.http.protocol.*; +import org.apache.hc.core5.io.CloseMode; +import org.apache.hc.core5.net.URIAuthority; +import org.apache.hc.core5.testing.SSLTestContexts; import org.apache.hc.core5.testing.extension.classic.ClassicTestResources; import org.apache.hc.core5.util.Timeout; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLSocketFactory; + abstract class ClassicIntegrationTest { private static final Timeout TIMEOUT = Timeout.ofMinutes(1); @@ -775,4 +789,85 @@ void testHeaderTooLargePost() throws Exception { } } + @Test + void testImmediateCloseUponSocketTimeout() throws Exception { + final int socketTimeoutMillis = 1000; + final int serverDelayMillis = 5 * socketTimeoutMillis; + + final ClassicTestServer server = testResources.server(); + + final CountDownLatch serverDelayStarted = new CountDownLatch(1); + + // Configure server to delay significantly before responding + server.register("*", (request, response, context) -> { + serverDelayStarted.countDown(); + try { + // Delay much longer than socket timeout + Thread.sleep(serverDelayMillis); + } catch (final InterruptedException ex) { + Thread.currentThread().interrupt(); + } + response.setCode(HttpStatus.SC_OK); + response.setEntity(new StringEntity("Delayed response", ContentType.TEXT_PLAIN)); + }); + + server.start(); + + final HttpHost host = new HttpHost(scheme.id, "localhost", server.getPort()); + final HttpCoreContext context = HttpCoreContext.create(); + final HttpConnectionFactory connFactory = + DefaultBHttpClientConnectionFactory.builder().build(); + + final HttpRequestExecutor requestExecutor = new HttpRequestExecutor(); + final HttpProcessor processor = HttpProcessors.client(); + + final BasicClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/"); + request.setAuthority(new URIAuthority(host)); + request.setScheme(host.getSchemeName()); + + runWithSocket(host, socketTimeoutMillis, socket -> { + try { + final HttpClientConnection connection = connFactory.createConnection(socket); + + requestExecutor.preProcess(request, processor, context); + + final long startTime = System.currentTimeMillis(); + try (final ClassicHttpResponse response = requestExecutor.execute( + request, connection, context)) { + Assertions.fail("Expected SocketTimeoutException not thrown"); + } catch (final SocketTimeoutException e) { + // Expected due to server delay exceeding socket timeout + } + + final long endTime = System.currentTimeMillis(); + final long durationMillis = endTime - startTime; + // Assert that the timeout occurred around the socket timeout duration + Assertions.assertTrue(durationMillis >= socketTimeoutMillis && durationMillis < socketTimeoutMillis * 2, + String.format("Socket timeout should occur around %dms (took %dms)", + socketTimeoutMillis, durationMillis)); + } catch (final IOException | HttpException e) { + Assertions.fail("IOException during request execution: " + e.getMessage()); + } + }); + } + + private static void runWithSocket(HttpHost host, int socketTimeoutMillis, Consumer socketConsumer) + throws IOException { + try (final Socket clientSocket = new Socket()) { + clientSocket.setSoTimeout(socketTimeoutMillis); + clientSocket.connect(new InetSocketAddress(host.getHostName(), host.getPort())); + if (host.getSchemeName().equalsIgnoreCase("http")) { + socketConsumer.accept(clientSocket); + return; + } + + try (SSLSocket sslSocket = (SSLSocket) SSLTestContexts.createClientSSLContext() + .getSocketFactory() + .createSocket(clientSocket, host.getHostName(), -1, true)) { + sslSocket.startHandshake(); + + socketConsumer.accept(sslSocket); + } + } + } } diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java index a300580379..f0aedb6299 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java @@ -238,6 +238,11 @@ public void close(final CloseMode closeMode) { try { // force abortive close (RST) baseSocket.setSoLinger(true, 0); + // Some implementations like SSLSocketImpl read from the input stream to do a graceful close + // We want an immediate close here, so we set a minimal timeout to prevent blocking + // This specific case could happen if the underlying socketHolder is initialized through the + // bind(Socket) method with an SSLSocket instance. + baseSocket.setSoTimeout(1); } catch (final IOException ignore) { } finally { Closer.closeQuietly(baseSocket); From 95087b9dcd800c5ec2f032b32e230f7010bb5be6 Mon Sep 17 00:00:00 2001 From: Alexis Le Dantec Date: Tue, 21 Oct 2025 11:23:35 +0200 Subject: [PATCH 2/3] Formatting --- .../classic/ClassicIntegrationTest.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java index ab6aa4f562..73ff66f551 100644 --- a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java +++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/ClassicIntegrationTest.java @@ -40,7 +40,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Random; -import java.util.concurrent.*; +import java.util.concurrent.CountDownLatch; import java.util.function.Consumer; import org.apache.hc.core5.http.ClassicHttpRequest; @@ -56,16 +56,12 @@ import org.apache.hc.core5.http.Method; import org.apache.hc.core5.http.ProtocolException; import org.apache.hc.core5.http.URIScheme; -import org.apache.hc.core5.http.config.CharCodingConfig; import org.apache.hc.core5.http.config.Http1Config; -import org.apache.hc.core5.http.impl.DefaultAddressResolver; -import org.apache.hc.core5.http.impl.DefaultConnectionReuseStrategy; import org.apache.hc.core5.http.impl.HttpProcessors; import org.apache.hc.core5.http.impl.io.DefaultBHttpClientConnectionFactory; import org.apache.hc.core5.http.impl.io.HttpRequestExecutor; import org.apache.hc.core5.http.io.HttpClientConnection; import org.apache.hc.core5.http.io.HttpConnectionFactory; -import org.apache.hc.core5.http.io.SocketConfig; import org.apache.hc.core5.http.io.entity.AbstractHttpEntity; import org.apache.hc.core5.http.io.entity.ByteArrayEntity; import org.apache.hc.core5.http.io.entity.EntityUtils; @@ -73,8 +69,16 @@ import org.apache.hc.core5.http.io.support.BasicHttpServerExpectationDecorator; import org.apache.hc.core5.http.message.BasicClassicHttpRequest; import org.apache.hc.core5.http.message.BasicClassicHttpResponse; -import org.apache.hc.core5.http.protocol.*; -import org.apache.hc.core5.io.CloseMode; +import org.apache.hc.core5.http.protocol.DefaultHttpProcessor; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.http.protocol.HttpCoreContext; +import org.apache.hc.core5.http.protocol.HttpProcessor; +import org.apache.hc.core5.http.protocol.RequestConnControl; +import org.apache.hc.core5.http.protocol.RequestContent; +import org.apache.hc.core5.http.protocol.RequestExpectContinue; +import org.apache.hc.core5.http.protocol.RequestTE; +import org.apache.hc.core5.http.protocol.RequestTargetHost; +import org.apache.hc.core5.http.protocol.RequestUserAgent; import org.apache.hc.core5.net.URIAuthority; import org.apache.hc.core5.testing.SSLTestContexts; import org.apache.hc.core5.testing.extension.classic.ClassicTestResources; @@ -83,10 +87,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocket; -import javax.net.ssl.SSLSocketFactory; abstract class ClassicIntegrationTest { @@ -851,7 +852,8 @@ void testImmediateCloseUponSocketTimeout() throws Exception { }); } - private static void runWithSocket(HttpHost host, int socketTimeoutMillis, Consumer socketConsumer) + private static void runWithSocket( + final HttpHost host, final int socketTimeoutMillis, final Consumer socketConsumer) throws IOException { try (final Socket clientSocket = new Socket()) { clientSocket.setSoTimeout(socketTimeoutMillis); @@ -861,7 +863,7 @@ private static void runWithSocket(HttpHost host, int socketTimeoutMillis, Consum return; } - try (SSLSocket sslSocket = (SSLSocket) SSLTestContexts.createClientSSLContext() + try (final SSLSocket sslSocket = (SSLSocket) SSLTestContexts.createClientSSLContext() .getSocketFactory() .createSocket(clientSocket, host.getHostName(), -1, true)) { sslSocket.startHandshake(); From 00c68e1104dbc1d09a9a67ce0dd6ea6d8433e394 Mon Sep 17 00:00:00 2001 From: Alexis Le Dantec Date: Tue, 21 Oct 2025 11:28:06 +0200 Subject: [PATCH 3/3] Only reduce socket timeout upon SocketTimeoutException --- .../apache/hc/core5/http/impl/io/BHttpConnectionBase.java | 5 ----- .../apache/hc/core5/http/impl/io/HttpRequestExecutor.java | 8 ++++++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java index f0aedb6299..a300580379 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/BHttpConnectionBase.java @@ -238,11 +238,6 @@ public void close(final CloseMode closeMode) { try { // force abortive close (RST) baseSocket.setSoLinger(true, 0); - // Some implementations like SSLSocketImpl read from the input stream to do a graceful close - // We want an immediate close here, so we set a minimal timeout to prevent blocking - // This specific case could happen if the underlying socketHolder is initialized through the - // bind(Socket) method with an SSLSocket instance. - baseSocket.setSoTimeout(1); } catch (final IOException ignore) { } finally { Closer.closeQuietly(baseSocket); diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpRequestExecutor.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpRequestExecutor.java index 9bff2058ef..5208c0fc9f 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpRequestExecutor.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpRequestExecutor.java @@ -28,6 +28,7 @@ package org.apache.hc.core5.http.impl.io; import java.io.IOException; +import java.net.SocketTimeoutException; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; @@ -217,6 +218,13 @@ public ClassicHttpResponse execute( } catch (final HttpException | SSLException ex) { Closer.closeQuietly(conn); throw ex; + } catch (final SocketTimeoutException ex) { + // If the server isn't responsive, we want to close the connection immediately + // We set the socket timeout to a minimal value in such a case, because in some cases, the connection + // might only have access to an SSLSocket that it will try to close gracefully. + conn.setSocketTimeout(Timeout.ONE_MILLISECOND); + Closer.close(conn, CloseMode.IMMEDIATE); + throw ex; } catch (final IOException | RuntimeException ex) { Closer.close(conn, CloseMode.IMMEDIATE); throw ex;