Skip to content

Commit 56a2482

Browse files
arturobernalgarturobernalg
committed
HTTPCLIENT-2386: Fix TLS handshake timeout handling
- Use socketTimeout (not connectTimeout) as default TLS handshake timeout - Throw TlsHandshakeTimeoutException for handshake timeouts
1 parent c301f22 commit 56a2482

4 files changed

Lines changed: 375 additions & 2 deletions

File tree

‎httpcore5-testing/src/test/java/org/apache/hc/core5/testing/TlsHandshakeTimeoutTest.java‎

Lines changed: 304 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,304 @@
1+
/*
2+
* ====================================================================
3+
* Licensed to the Apache Software Foundation (ASF) under one
4+
* or more contributor license agreements. See the NOTICE file
5+
* distributed with this work for additional information
6+
* regarding copyright ownership. The ASF licenses this file
7+
* to you under the Apache License, Version 2.0 (the
8+
* "License"); you may not use this file except in compliance
9+
* with the License. You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing,
14+
* software distributed under the License is distributed on an
15+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
* KIND, either express or implied. See the License for the
17+
* specific language governing permissions and limitations
18+
* under the License.
19+
* ====================================================================
20+
*
21+
* This software consists of voluntary contributions made by many
22+
* individuals on behalf of the Apache Software Foundation. For more
23+
* information on the Apache Software Foundation, please see
24+
* <http://www.apache.org/>.
25+
*
26+
*/
27+
28+
package org.apache.hc.core5.testing;
29+
30+
import static org.junit.jupiter.api.Assertions.assertNotNull;
31+
import static org.junit.jupiter.api.Assertions.assertTrue;
32+
33+
import java.io.IOException;
34+
import java.net.SocketAddress;
35+
import java.nio.ByteBuffer;
36+
import java.util.concurrent.atomic.AtomicReference;
37+
import java.util.concurrent.locks.Lock;
38+
import java.util.concurrent.locks.ReentrantLock;
39+
40+
import javax.net.ssl.SSLContext;
41+
42+
import org.apache.hc.core5.concurrent.FutureCallback;
43+
import org.apache.hc.core5.net.Host;
44+
import org.apache.hc.core5.reactor.IOEventHandler;
45+
import org.apache.hc.core5.reactor.IOSession;
46+
import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
47+
import org.apache.hc.core5.reactor.ssl.SSLIOSession;
48+
import org.apache.hc.core5.reactor.ssl.SSLMode;
49+
import org.apache.hc.core5.reactor.ssl.TlsHandshakeTimeoutException;
50+
import org.apache.hc.core5.util.Timeout;
51+
import org.junit.jupiter.api.Test;
52+
53+
/**
54+
* Unit test for TLS handshake timeout handling in SSLIOSession.
55+
*/
56+
public class TlsHandshakeTimeoutTest {
57+
58+
@Test
59+
void testHandshakeTimeoutTriggersTlsHandshakeTimeoutException() throws Exception {
60+
// Use concrete Host since NamedEndpoint is an interface
61+
final Host endpoint = new Host("localhost", 443);
62+
// Create a client SSL context
63+
final SSLContext sslContext = SSLTestContexts.createClientSSLContext();
64+
65+
// Capture exception from handshake
66+
final AtomicReference<Exception> failure = new AtomicReference<>();
67+
final FutureCallback<javax.net.ssl.SSLSession> callback = new FutureCallback<javax.net.ssl.SSLSession>() {
68+
@Override
69+
public void completed(final javax.net.ssl.SSLSession result) {
70+
// Should not complete successfully
71+
failure.set(new RuntimeException("Handshake should not complete"));
72+
}
73+
74+
@Override
75+
public void failed(final Exception ex) {
76+
failure.set(ex);
77+
}
78+
79+
@Override
80+
public void cancelled() {
81+
failure.set(new RuntimeException("Handshake cancelled"));
82+
}
83+
};
84+
85+
// Stub IOSession with small socket timeout
86+
final TestIOSession mockSession = new TestIOSession(Timeout.ofSeconds(1));
87+
88+
// Create SSLIOSession with a tiny handshake timeout
89+
final SSLIOSession sslioSession = new SSLIOSession(
90+
endpoint,
91+
mockSession,
92+
SSLMode.CLIENT,
93+
sslContext,
94+
SSLBufferMode.STATIC,
95+
null,
96+
null,
97+
Timeout.ofMilliseconds(10),
98+
null,
99+
null,
100+
callback
101+
);
102+
103+
// Start the handshake process
104+
sslioSession.beginHandshake(mockSession);
105+
106+
// Simulate a timeout event after handshakeTimeout
107+
sslioSession.getHandler().timeout(mockSession, Timeout.ofMilliseconds(10));
108+
109+
// Assert that our callback received a TlsHandshakeTimeoutException
110+
final Exception ex = failure.get();
111+
assertNotNull(ex, "Expected handshake failure");
112+
Throwable cause = ex;
113+
while (cause != null && !(cause instanceof TlsHandshakeTimeoutException)) {
114+
cause = cause.getCause();
115+
}
116+
assertTrue(cause instanceof TlsHandshakeTimeoutException,
117+
"Expected TlsHandshakeTimeoutException but got: " + ex);
118+
}
119+
120+
/**
121+
* Minimal IOSession stub for testing SSLIOSession handshake timeout logic.
122+
*/
123+
static class TestIOSession implements IOSession {
124+
private Timeout socketTimeout;
125+
private IOEventHandler handler;
126+
private final Lock lock = new ReentrantLock();
127+
128+
TestIOSession(final Timeout socketTimeout) {
129+
this.socketTimeout = socketTimeout;
130+
// default no-op handler
131+
this.handler = new IOEventHandler() {
132+
@Override
133+
public void connected(final IOSession session) {
134+
}
135+
136+
@Override
137+
public void inputReady(final IOSession session, final ByteBuffer src) {
138+
}
139+
140+
@Override
141+
public void outputReady(final IOSession session) {
142+
}
143+
144+
@Override
145+
public void timeout(final IOSession session, final Timeout timeout) {
146+
}
147+
148+
@Override
149+
public void exception(final IOSession session, final Exception ex) {
150+
}
151+
152+
@Override
153+
public void disconnected(final IOSession session) {
154+
}
155+
};
156+
}
157+
158+
@Override
159+
public IOEventHandler getHandler() {
160+
return handler;
161+
}
162+
163+
@Override
164+
public void upgrade(final IOEventHandler handler) {
165+
this.handler = handler;
166+
}
167+
168+
@Override
169+
public Lock getLock() {
170+
return lock;
171+
}
172+
173+
@Override
174+
public String getId() {
175+
return "test-session";
176+
}
177+
178+
@Override
179+
public boolean isOpen() {
180+
return true;
181+
}
182+
183+
@Override
184+
public void close() {
185+
}
186+
187+
@Override
188+
public void close(final org.apache.hc.core5.io.CloseMode closeMode) {
189+
}
190+
191+
@Override
192+
public Status getStatus() {
193+
return Status.ACTIVE;
194+
}
195+
196+
@Override
197+
public java.nio.channels.ByteChannel channel() {
198+
return new java.nio.channels.ByteChannel() {
199+
@Override
200+
public int read(final ByteBuffer dst) throws IOException {
201+
return 0;
202+
}
203+
204+
@Override
205+
public int write(final ByteBuffer src) throws IOException {
206+
return 0;
207+
}
208+
209+
@Override
210+
public boolean isOpen() {
211+
return true;
212+
}
213+
214+
@Override
215+
public void close() throws IOException {
216+
}
217+
};
218+
}
219+
220+
@Override
221+
public SocketAddress getRemoteAddress() {
222+
return null;
223+
}
224+
225+
@Override
226+
public SocketAddress getLocalAddress() {
227+
return null;
228+
}
229+
230+
@Override
231+
public int getEventMask() {
232+
return 0;
233+
}
234+
235+
@Override
236+
public void setEventMask(final int ops) {
237+
}
238+
239+
@Override
240+
public void setEvent(final int op) {
241+
}
242+
243+
@Override
244+
public void clearEvent(final int op) {
245+
}
246+
247+
@Override
248+
public Timeout getSocketTimeout() {
249+
return socketTimeout;
250+
}
251+
252+
@Override
253+
public void setSocketTimeout(final Timeout timeout) {
254+
this.socketTimeout = timeout;
255+
}
256+
257+
@Override
258+
public void updateReadTime() {
259+
}
260+
261+
@Override
262+
public void updateWriteTime() {
263+
}
264+
265+
@Override
266+
public long getLastReadTime() {
267+
return 0;
268+
}
269+
270+
@Override
271+
public long getLastWriteTime() {
272+
return 0;
273+
}
274+
275+
@Override
276+
public long getLastEventTime() {
277+
return 0;
278+
}
279+
280+
@Override
281+
public void enqueue(final org.apache.hc.core5.reactor.Command command, final org.apache.hc.core5.reactor.Command.Priority priority) {
282+
}
283+
284+
@Override
285+
public boolean hasCommands() {
286+
return false;
287+
}
288+
289+
@Override
290+
public org.apache.hc.core5.reactor.Command poll() {
291+
return null;
292+
}
293+
294+
@Override
295+
public int read(final ByteBuffer byteBuffer) throws IOException {
296+
return 0;
297+
}
298+
299+
@Override
300+
public int write(final ByteBuffer byteBuffer) throws IOException {
301+
return 0;
302+
}
303+
}
304+
}

‎httpcore5/src/main/java/org/apache/hc/core5/reactor/InternalDataChannel.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.apache.hc.core5.reactor.ssl.SSLSessionInitializer;
5555
import org.apache.hc.core5.reactor.ssl.SSLSessionVerifier;
5656
import org.apache.hc.core5.reactor.ssl.TlsDetails;
57+
import org.apache.hc.core5.reactor.ssl.TlsHandshakeTimeoutException;
5758
import org.apache.hc.core5.reactor.ssl.TransportSecurityLayer;
5859
import org.apache.hc.core5.util.Args;
5960
import org.apache.hc.core5.util.Asserts;
@@ -166,10 +167,17 @@ void onTimeout(final Timeout timeout) throws IOException {
166167
if (sessionListener != null) {
167168
sessionListener.timeout(currentSession);
168169
}
170+
171+
final SSLIOSession tlsSession = tlsSessionRef.get();
172+
if (tlsSession != null && !tlsSession.isHandshakeComplete()) {
173+
throw new TlsHandshakeTimeoutException("TLS handshake timed out after " + timeout);
174+
}
175+
169176
final IOEventHandler handler = ensureHandler(currentSession);
170177
handler.timeout(currentSession, timeout);
171178
}
172179

180+
173181
@Override
174182
void onException(final Exception cause) {
175183
final IOSession currentSession = currentSessionRef.get();

‎httpcore5/src/main/java/org/apache/hc/core5/reactor/ssl/SSLIOSession.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
import org.apache.hc.core5.concurrent.FutureCallback;
5353
import org.apache.hc.core5.function.Callback;
5454
import org.apache.hc.core5.io.CloseMode;
55-
import org.apache.hc.core5.io.SocketTimeoutExceptionFactory;
5655
import org.apache.hc.core5.net.NamedEndpoint;
5756
import org.apache.hc.core5.reactor.Command;
5857
import org.apache.hc.core5.reactor.EventMask;
@@ -220,7 +219,7 @@ public void timeout(final IOSession protocolSession, final Timeout timeout) thro
220219
close(CloseMode.IMMEDIATE);
221220
}
222221
if (handshakeStateRef.get() != TLSHandShakeState.COMPLETE) {
223-
exception(protocolSession, SocketTimeoutExceptionFactory.create(handshakeTimeout));
222+
exception(protocolSession, new TlsHandshakeTimeoutException("TLS handshake timed out after " + handshakeTimeout));
224223
} else {
225224
ensureHandler().timeout(protocolSession, timeout);
226225
}
@@ -928,4 +927,13 @@ public String toString() {
928927
}
929928
}
930929

930+
/**
931+
* Indicates whether the TLS handshake has successfully completed.
932+
*
933+
* @return {@code true} if the handshake has completed; {@code false} otherwise.
934+
*/
935+
public boolean isHandshakeComplete() {
936+
return handshakeStateRef.get() == TLSHandShakeState.COMPLETE;
937+
}
938+
931939
}

0 commit comments

Comments
 (0)