From 56a2482d125acda29ff18af0155b8d968657010e Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Thu, 31 Jul 2025 19:36:42 +0200 Subject: [PATCH] HTTPCLIENT-2386: Fix TLS handshake timeout handling - Use socketTimeout (not connectTimeout) as default TLS handshake timeout - Throw TlsHandshakeTimeoutException for handshake timeouts --- .../testing/TlsHandshakeTimeoutTest.java | 304 ++++++++++++++++++ .../hc/core5/reactor/InternalDataChannel.java | 8 + .../hc/core5/reactor/ssl/SSLIOSession.java | 12 +- .../ssl/TlsHandshakeTimeoutException.java | 53 +++ 4 files changed, 375 insertions(+), 2 deletions(-) create mode 100644 httpcore5-testing/src/test/java/org/apache/hc/core5/testing/TlsHandshakeTimeoutTest.java create mode 100644 httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/TlsHandshakeTimeoutException.java diff --git a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/TlsHandshakeTimeoutTest.java b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/TlsHandshakeTimeoutTest.java new file mode 100644 index 000000000..913648f16 --- /dev/null +++ b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/TlsHandshakeTimeoutTest.java @@ -0,0 +1,304 @@ +/* + * ==================================================================== + * 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.core5.testing; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.net.SocketAddress; +import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + +import javax.net.ssl.SSLContext; + +import org.apache.hc.core5.concurrent.FutureCallback; +import org.apache.hc.core5.net.Host; +import org.apache.hc.core5.reactor.IOEventHandler; +import org.apache.hc.core5.reactor.IOSession; +import org.apache.hc.core5.reactor.ssl.SSLBufferMode; +import org.apache.hc.core5.reactor.ssl.SSLIOSession; +import org.apache.hc.core5.reactor.ssl.SSLMode; +import org.apache.hc.core5.reactor.ssl.TlsHandshakeTimeoutException; +import org.apache.hc.core5.util.Timeout; +import org.junit.jupiter.api.Test; + +/** + * Unit test for TLS handshake timeout handling in SSLIOSession. + */ +public class TlsHandshakeTimeoutTest { + + @Test + void testHandshakeTimeoutTriggersTlsHandshakeTimeoutException() throws Exception { + // Use concrete Host since NamedEndpoint is an interface + final Host endpoint = new Host("localhost", 443); + // Create a client SSL context + final SSLContext sslContext = SSLTestContexts.createClientSSLContext(); + + // Capture exception from handshake + final AtomicReference failure = new AtomicReference<>(); + final FutureCallback callback = new FutureCallback() { + @Override + public void completed(final javax.net.ssl.SSLSession result) { + // Should not complete successfully + failure.set(new RuntimeException("Handshake should not complete")); + } + + @Override + public void failed(final Exception ex) { + failure.set(ex); + } + + @Override + public void cancelled() { + failure.set(new RuntimeException("Handshake cancelled")); + } + }; + + // Stub IOSession with small socket timeout + final TestIOSession mockSession = new TestIOSession(Timeout.ofSeconds(1)); + + // Create SSLIOSession with a tiny handshake timeout + final SSLIOSession sslioSession = new SSLIOSession( + endpoint, + mockSession, + SSLMode.CLIENT, + sslContext, + SSLBufferMode.STATIC, + null, + null, + Timeout.ofMilliseconds(10), + null, + null, + callback + ); + + // Start the handshake process + sslioSession.beginHandshake(mockSession); + + // Simulate a timeout event after handshakeTimeout + sslioSession.getHandler().timeout(mockSession, Timeout.ofMilliseconds(10)); + + // Assert that our callback received a TlsHandshakeTimeoutException + final Exception ex = failure.get(); + assertNotNull(ex, "Expected handshake failure"); + Throwable cause = ex; + while (cause != null && !(cause instanceof TlsHandshakeTimeoutException)) { + cause = cause.getCause(); + } + assertTrue(cause instanceof TlsHandshakeTimeoutException, + "Expected TlsHandshakeTimeoutException but got: " + ex); + } + + /** + * Minimal IOSession stub for testing SSLIOSession handshake timeout logic. + */ + static class TestIOSession implements IOSession { + private Timeout socketTimeout; + private IOEventHandler handler; + private final Lock lock = new ReentrantLock(); + + TestIOSession(final Timeout socketTimeout) { + this.socketTimeout = socketTimeout; + // default no-op handler + this.handler = new IOEventHandler() { + @Override + public void connected(final IOSession session) { + } + + @Override + public void inputReady(final IOSession session, final ByteBuffer src) { + } + + @Override + public void outputReady(final IOSession session) { + } + + @Override + public void timeout(final IOSession session, final Timeout timeout) { + } + + @Override + public void exception(final IOSession session, final Exception ex) { + } + + @Override + public void disconnected(final IOSession session) { + } + }; + } + + @Override + public IOEventHandler getHandler() { + return handler; + } + + @Override + public void upgrade(final IOEventHandler handler) { + this.handler = handler; + } + + @Override + public Lock getLock() { + return lock; + } + + @Override + public String getId() { + return "test-session"; + } + + @Override + public boolean isOpen() { + return true; + } + + @Override + public void close() { + } + + @Override + public void close(final org.apache.hc.core5.io.CloseMode closeMode) { + } + + @Override + public Status getStatus() { + return Status.ACTIVE; + } + + @Override + public java.nio.channels.ByteChannel channel() { + return new java.nio.channels.ByteChannel() { + @Override + public int read(final ByteBuffer dst) throws IOException { + return 0; + } + + @Override + public int write(final ByteBuffer src) throws IOException { + return 0; + } + + @Override + public boolean isOpen() { + return true; + } + + @Override + public void close() throws IOException { + } + }; + } + + @Override + public SocketAddress getRemoteAddress() { + return null; + } + + @Override + public SocketAddress getLocalAddress() { + return null; + } + + @Override + public int getEventMask() { + return 0; + } + + @Override + public void setEventMask(final int ops) { + } + + @Override + public void setEvent(final int op) { + } + + @Override + public void clearEvent(final int op) { + } + + @Override + public Timeout getSocketTimeout() { + return socketTimeout; + } + + @Override + public void setSocketTimeout(final Timeout timeout) { + this.socketTimeout = timeout; + } + + @Override + public void updateReadTime() { + } + + @Override + public void updateWriteTime() { + } + + @Override + public long getLastReadTime() { + return 0; + } + + @Override + public long getLastWriteTime() { + return 0; + } + + @Override + public long getLastEventTime() { + return 0; + } + + @Override + public void enqueue(final org.apache.hc.core5.reactor.Command command, final org.apache.hc.core5.reactor.Command.Priority priority) { + } + + @Override + public boolean hasCommands() { + return false; + } + + @Override + public org.apache.hc.core5.reactor.Command poll() { + return null; + } + + @Override + public int read(final ByteBuffer byteBuffer) throws IOException { + return 0; + } + + @Override + public int write(final ByteBuffer byteBuffer) throws IOException { + return 0; + } + } +} diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java index d290939b4..e6d981d0a 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java @@ -54,6 +54,7 @@ import org.apache.hc.core5.reactor.ssl.SSLSessionInitializer; import org.apache.hc.core5.reactor.ssl.SSLSessionVerifier; import org.apache.hc.core5.reactor.ssl.TlsDetails; +import org.apache.hc.core5.reactor.ssl.TlsHandshakeTimeoutException; import org.apache.hc.core5.reactor.ssl.TransportSecurityLayer; import org.apache.hc.core5.util.Args; import org.apache.hc.core5.util.Asserts; @@ -166,10 +167,17 @@ void onTimeout(final Timeout timeout) throws IOException { if (sessionListener != null) { sessionListener.timeout(currentSession); } + + final SSLIOSession tlsSession = tlsSessionRef.get(); + if (tlsSession != null && !tlsSession.isHandshakeComplete()) { + throw new TlsHandshakeTimeoutException("TLS handshake timed out after " + timeout); + } + final IOEventHandler handler = ensureHandler(currentSession); handler.timeout(currentSession, timeout); } + @Override void onException(final Exception cause) { final IOSession currentSession = currentSessionRef.get(); diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java index 02298eb5d..95e982b14 100644 --- a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java +++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java @@ -52,7 +52,6 @@ import org.apache.hc.core5.concurrent.FutureCallback; import org.apache.hc.core5.function.Callback; import org.apache.hc.core5.io.CloseMode; -import org.apache.hc.core5.io.SocketTimeoutExceptionFactory; import org.apache.hc.core5.net.NamedEndpoint; import org.apache.hc.core5.reactor.Command; import org.apache.hc.core5.reactor.EventMask; @@ -220,7 +219,7 @@ public void timeout(final IOSession protocolSession, final Timeout timeout) thro close(CloseMode.IMMEDIATE); } if (handshakeStateRef.get() != TLSHandShakeState.COMPLETE) { - exception(protocolSession, SocketTimeoutExceptionFactory.create(handshakeTimeout)); + exception(protocolSession, new TlsHandshakeTimeoutException("TLS handshake timed out after " + handshakeTimeout)); } else { ensureHandler().timeout(protocolSession, timeout); } @@ -928,4 +927,13 @@ public String toString() { } } + /** + * Indicates whether the TLS handshake has successfully completed. + * + * @return {@code true} if the handshake has completed; {@code false} otherwise. + */ + public boolean isHandshakeComplete() { + return handshakeStateRef.get() == TLSHandShakeState.COMPLETE; + } + } diff --git a/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/TlsHandshakeTimeoutException.java b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/TlsHandshakeTimeoutException.java new file mode 100644 index 000000000..c0ac9df4c --- /dev/null +++ b/httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/TlsHandshakeTimeoutException.java @@ -0,0 +1,53 @@ +/* + * ==================================================================== + * 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.core5.reactor.ssl; + + +import java.net.SocketTimeoutException; + +/** + * Signals that a timeout has occurred specifically during the TLS handshake phase. + *

+ * This exception clearly differentiates a TLS handshake timeout from general socket timeouts, + * enabling easier debugging and handling of TLS-related issues. + * + * @since 5.4 + */ +public class TlsHandshakeTimeoutException extends SocketTimeoutException { + + private static final long serialVersionUID = 1L; + + /** + * Constructs a new TlsHandshakeTimeoutException with the specified detail message. + * + * @param message the detail message explaining the timeout cause. + */ + public TlsHandshakeTimeoutException(final String message) { + super(message); + } + +} \ No newline at end of file