From ee0a102104d03a8d1cfe18e571d179c41242182c Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Sat, 22 Jun 2024 17:13:27 +0200 Subject: [PATCH] HTTPCLIENT-2328: Blocking i/o connections to check if the opposite TLS endpoint has been closed by the opposite endpoint while writing out request body --- .../io/DefaultHttpClientConnectionOperator.java | 4 ++-- .../io/DefaultManagedHttpClientConnection.java | 8 ++++++++ .../http/impl/io/LoggingSocketHolder.java | 7 +++++++ .../http/io/ManagedHttpClientConnection.java | 16 ++++++++++++++++ .../http/ssl/AbstractClientTlsStrategy.java | 12 +++++++++--- .../io/TestHttpClientConnectionOperator.java | 2 +- 6 files changed, 43 insertions(+), 6 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 8212c83f8..a90847cb8 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 @@ -235,8 +235,8 @@ public class DefaultHttpClientConnectionOperator implements HttpClientConnection if (LOG.isDebugEnabled()) { LOG.debug("{} {} upgrading to TLS", ConnPoolSupport.getId(conn), tlsName); } - final Socket upgradedSocket = tlsSocketStrategy.upgrade(socket, tlsName.getHostName(), tlsName.getPort(), attachment, context); - conn.bind(upgradedSocket); + final SSLSocket sslSocket = tlsSocketStrategy.upgrade(socket, tlsName.getHostName(), tlsName.getPort(), attachment, context); + conn.bind(sslSocket, socket); 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/io/DefaultManagedHttpClientConnection.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultManagedHttpClientConnection.java index 03d697dcd..720d614e3 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultManagedHttpClientConnection.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/DefaultManagedHttpClientConnection.java @@ -184,6 +184,14 @@ final class DefaultManagedHttpClientConnection socketTimeout = Timeout.ofMilliseconds(socket.getSoTimeout()); } + @Override + public void bind(final SSLSocket sslSocket, final Socket socket) throws IOException { + super.bind(WIRE_LOG.isDebugEnabled() ? + new LoggingSocketHolder(sslSocket, socket, this.id, WIRE_LOG) : + new SocketHolder(sslSocket, socket)); + socketTimeout = Timeout.ofMilliseconds(sslSocket.getSoTimeout()); + } + @Override protected void onResponseReceived(final ClassicHttpResponse response) { if (response != null && HEADER_LOG.isDebugEnabled()) { diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/LoggingSocketHolder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/LoggingSocketHolder.java index 5107b3316..a042849e9 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/LoggingSocketHolder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/LoggingSocketHolder.java @@ -32,6 +32,8 @@ import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; +import javax.net.ssl.SSLSocket; + import org.apache.hc.client5.http.impl.Wire; import org.apache.hc.core5.http.impl.io.SocketHolder; import org.slf4j.Logger; @@ -45,6 +47,11 @@ class LoggingSocketHolder extends SocketHolder { this.wire = new Wire(log, id); } + LoggingSocketHolder(final SSLSocket sslSocket, final Socket baseSocket, final String id, final Logger log) { + super(sslSocket, baseSocket); + this.wire = new Wire(log, id); + } + @Override protected InputStream getInputStream(final Socket socket) throws IOException { return new LoggingInputStream(super.getInputStream(socket), wire); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/io/ManagedHttpClientConnection.java b/httpclient5/src/main/java/org/apache/hc/client5/http/io/ManagedHttpClientConnection.java index 48546b963..0877c92df 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/io/ManagedHttpClientConnection.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/io/ManagedHttpClientConnection.java @@ -31,6 +31,7 @@ import java.io.IOException; import java.net.Socket; import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; import org.apache.hc.core5.annotation.Internal; import org.apache.hc.core5.http.io.HttpClientConnection; @@ -55,6 +56,21 @@ public interface ManagedHttpClientConnection extends HttpClientConnection { */ void bind(Socket socket) throws IOException; + /** + * Binds this connection to the SSL given socket and the underlying network + * socket. The connection is considered open if it is bound, the underlying + * network socket is connection to a remote host and the SSL socket is + * fully initialized (TLS handshake has been successfully executed). + * + * @param sslSocket the SSL socket to bind the connection to. + * @param socket the underlying network socket of the SSL socket. + * + * @since 5.4 + */ + default void bind(SSLSocket sslSocket, Socket socket) throws IOException { + bind(sslSocket); + } + /** * Returns the underlying socket. */ 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 ff236f21f..c79d8394d 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 @@ -62,6 +62,7 @@ import org.apache.hc.core5.http.ssl.TlsCiphers; import org.apache.hc.core5.http2.HttpVersionPolicy; import org.apache.hc.core5.http2.ssl.ApplicationProtocol; import org.apache.hc.core5.http2.ssl.H2TlsSupport; +import org.apache.hc.core5.io.Closer; import org.apache.hc.core5.net.NamedEndpoint; import org.apache.hc.core5.reactor.ssl.SSLBufferMode; import org.apache.hc.core5.reactor.ssl.TlsDetails; @@ -204,9 +205,14 @@ abstract class AbstractClientTlsStrategy implements TlsStrategy, TlsSocketStrate socket, target, port, - true); - executeHandshake(upgradedSocket, target, attachment); - return upgradedSocket; + false); + try { + executeHandshake(upgradedSocket, target, attachment); + return upgradedSocket; + } catch (IOException | RuntimeException ex) { + Closer.closeQuietly(upgradedSocket); + throw ex; + } } private void executeHandshake( diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java index 64aff2612..e369f495a 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/io/TestHttpClientConnectionOperator.java @@ -147,7 +147,7 @@ public class TestHttpClientConnectionOperator { Mockito.verify(socket).connect(new InetSocketAddress(ip1, 443), 123); Mockito.verify(conn, Mockito.times(2)).bind(socket); Mockito.verify(tlsSocketStrategy).upgrade(socket, "somehost", -1, tlsConfig, context); - Mockito.verify(conn, Mockito.times(1)).bind(upgradedSocket); + Mockito.verify(conn, Mockito.times(1)).bind(upgradedSocket, socket); } @Test