From cc3b6cb611ca7f1fa76b78ea84b6df8769dd3925 Mon Sep 17 00:00:00 2001 From: Ryan Schmitt Date: Tue, 13 Oct 2026 13:26:08 -0700 Subject: [PATCH] Fix spurious TCP resets when initiating TLS connection closure When an HTTP/1.1 response contains `Connection: close`, the async client begins a graceful TLS shutdown. This causes `SSLIOSession.isOpen()` to immediately start returning `false`, so at the completion of the HTTP exchange when `AsyncProtocolExec` calls `isEndpointConnected()`, the `InternalConnectionEndpoint.isConnected()` method sees that the pool entry's underlying connection is reporting `!isOpen()` and calls `poolEntry.discardConnection(CloseMode.IMMEDIATE)`, which sends a TCP reset to the server. This commit changes that `discardConnection` call site to use `CloseMode.GRACEFUL`, and also provides a reproducer and integration test demonstrating the problem. --- .../async/TestAsyncTlsConnectionClose.java | 238 ++++++++++++++++++ .../PoolingAsyncClientConnectionManager.java | 2 +- 2 files changed, 239 insertions(+), 1 deletion(-) create mode 100644 httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncTlsConnectionClose.java diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncTlsConnectionClose.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncTlsConnectionClose.java new file mode 100644 index 0000000000..50f6dd0509 --- /dev/null +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncTlsConnectionClose.java @@ -0,0 +1,238 @@ +/* + * ==================================================================== + * 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.io.Closeable; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.util.Locale; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; + +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocket; + +import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; +import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder; +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.ssl.DefaultClientTlsStrategy; +import org.apache.hc.client5.testing.SSLTestContexts; +import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.ssl.TLS; +import org.apache.hc.core5.http2.HttpVersionPolicy; +import org.apache.hc.core5.util.Timeout; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +class TestAsyncTlsConnectionClose { + + private static final Timeout TIMEOUT = Timeout.ofSeconds(5); + + enum CloseInitiator { + // The origin sends Connection: close; HttpClient initiates TLS shutdown. + CLIENT, + // The origin sends close_notify after a reusable HTTP response. + SERVER; + + @Override + public String toString() { + return name().toLowerCase(Locale.ROOT); + } + } + + static Stream tlsCloseScenarios() { + return Stream.of(TLS.V_1_2, TLS.V_1_3) + .flatMap(tlsVersion -> Stream.of(CloseInitiator.CLIENT, CloseInitiator.SERVER) + .map(closeInitiator -> Arguments.of(tlsVersion, closeInitiator))); + } + + @ParameterizedTest(name = "{0}, {1}-initiated TLS close") + @MethodSource("tlsCloseScenarios") + void testConnectionCloseCompletesTlsShutdownWithoutReset( + final TLS tlsVersion, + final CloseInitiator closeInitiator) throws Exception { + try (TlsConnectionCloseServer server = new TlsConnectionCloseServer(tlsVersion, closeInitiator); + PoolingAsyncClientConnectionManager connectionManager = PoolingAsyncClientConnectionManagerBuilder.create() + .setTlsStrategy(new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext())) + .setDefaultTlsConfig(TlsConfig.custom() + .setSupportedProtocols(tlsVersion) + .setVersionPolicy(HttpVersionPolicy.FORCE_HTTP_1) + .build()) + .build(); + CloseableHttpAsyncClient client = HttpAsyncClients.custom() + .setConnectionManager(connectionManager) + .build()) { + server.start(); + client.start(); + + final HttpHost target = new HttpHost( + "https", InetAddress.getByName("127.0.0.1"), "localhost", server.getPort()); + final Future responseFuture = client.execute( + SimpleRequestBuilder.get() + .setHttpHost(target) + .setPath("/") + .build(), + null); + + Assertions.assertEquals(200, responseFuture.get( + TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()).getCode()); + Assertions.assertTrue(server.awaitCleanClose(), + () -> "the client reset the TCP connection during the " + + tlsVersion.id + " " + closeInitiator + + "-initiated TLS close_notify exchange"); + } + } + + private static final class TlsConnectionCloseServer implements Closeable { + + private final TLS tlsVersion; + private final CloseInitiator closeInitiator; + private final ServerSocket serverSocket; + private final CompletableFuture cleanCloseFuture; + private Thread worker; + + TlsConnectionCloseServer(final TLS tlsVersion, final CloseInitiator closeInitiator) throws IOException { + this.tlsVersion = tlsVersion; + this.closeInitiator = closeInitiator; + this.serverSocket = new ServerSocket(); + this.serverSocket.bind(new InetSocketAddress("127.0.0.1", 0)); + this.cleanCloseFuture = new CompletableFuture<>(); + } + + void start() { + worker = new Thread(this::run, "tls-connection-close-server"); + worker.setDaemon(true); + worker.start(); + } + + int getPort() { + return serverSocket.getLocalPort(); + } + + boolean awaitCleanClose() throws Exception { + return cleanCloseFuture.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit()); + } + + private void run() { + try (Socket socket = serverSocket.accept()) { + socket.setSoTimeout(TIMEOUT.toMillisecondsIntBound()); + final SSLContext sslContext = SSLTestContexts.createServerSSLContext(); + try (SSLSocket sslSocket = (SSLSocket) sslContext.getSocketFactory().createSocket( + socket, "localhost", socket.getPort(), false)) { + sslSocket.setUseClientMode(false); + sslSocket.setEnabledProtocols(new String[] { tlsVersion.id }); + sslSocket.startHandshake(); + if (!tlsVersion.id.equals(sslSocket.getSession().getProtocol())) { + throw new IOException("Unexpected negotiated TLS protocol " + + sslSocket.getSession().getProtocol()); + } + + readRequestHead(sslSocket.getInputStream()); + final OutputStream outputStream = sslSocket.getOutputStream(); + outputStream.write(("HTTP/1.1 200 OK\r\n" + + "Content-Length: 0\r\n" + + (closeInitiator == CloseInitiator.CLIENT ? "Connection: close\r\n" : "") + + "\r\n").getBytes(StandardCharsets.US_ASCII)); + outputStream.flush(); + + if (closeInitiator == CloseInitiator.CLIENT) { + // Connection: close causes the client to initiate TLS + // shutdown. Wait for its close_notify before returning ours. + if (sslSocket.getInputStream().read() != -1) { + throw new IOException("Unexpected application data after the request"); + } + } + // Otherwise closing this SSLSocket sends the server's + // close_notify first. + } + + // The layered SSLSocket does not own the TCP socket. Check the + // transport after TLS shutdown to distinguish FIN from RST. + cleanCloseFuture.complete(socket.getInputStream().read() == -1); + } catch (final IOException ex) { + if (isConnectionReset(ex)) { + cleanCloseFuture.complete(false); + } else { + cleanCloseFuture.completeExceptionally(ex); + } + } catch (final Exception ex) { + cleanCloseFuture.completeExceptionally(ex); + } + } + + private static boolean isConnectionReset(final Throwable cause) { + for (Throwable current = cause; current != null; current = current.getCause()) { + if (current.getMessage() != null + && current.getMessage().toLowerCase(Locale.ROOT).contains("connection reset")) { + return true; + } + } + return false; + } + + private static void readRequestHead(final InputStream inputStream) throws IOException { + int matched = 0; + while (matched < 4) { + final int b = inputStream.read(); + if (b == -1) { + throw new IOException("Connection closed before the request head was complete"); + } + if (b == "\r\n\r\n".charAt(matched)) { + matched++; + } else { + matched = b == '\r' ? 1 : 0; + } + } + } + + @Override + public void close() throws IOException { + serverSocket.close(); + if (worker != null) { + try { + worker.join(TimeUnit.SECONDS.toMillis(1)); + } catch (final InterruptedException ex) { + Thread.currentThread().interrupt(); + } + } + } + } +} 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 712bdd8a13..c5215a4156 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 @@ -804,7 +804,7 @@ public boolean isConnected() { return false; } if (!connection.isOpen()) { - poolEntry.discardConnection(CloseMode.IMMEDIATE); + poolEntry.discardConnection(CloseMode.GRACEFUL); return false; } return true;