Skip to content

Commit b78c2ad

Browse files
rschmittrschmitt
committed
Improve and simplify TLS handshake timeout tests
- Convert to pass/fail style (drop timing assertions) - Reenable on all Java versions - Parameterize test cases - Add HTTP/2 and ALPN coverage - Check for uncaught IOReactor exceptions - Remove `slow` tag
1 parent f457c8b commit b78c2ad

File tree

3 files changed

+79
-83
lines changed

3 files changed

+79
-83
lines changed

‎httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncTlsHandshakeTimeout.java‎

Lines changed: 47 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
import org.apache.hc.client5.http.async.methods.SimpleHttpRequest;
3030
import org.apache.hc.client5.http.config.ConnectionConfig;
31+
import org.apache.hc.client5.http.config.ConnectionConfig.Builder;
3132
import org.apache.hc.client5.http.config.TlsConfig;
3233
import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient;
3334
import org.apache.hc.client5.http.impl.async.HttpAsyncClientBuilder;
@@ -36,54 +37,70 @@
3637
import org.apache.hc.client5.http.ssl.DefaultClientTlsStrategy;
3738
import org.apache.hc.client5.testing.SSLTestContexts;
3839
import org.apache.hc.client5.testing.tls.TlsHandshakeTimeoutServer;
40+
import org.apache.hc.core5.http2.HttpVersionPolicy;
3941
import org.apache.hc.core5.reactor.IOReactorConfig;
4042
import org.apache.hc.core5.util.TimeValue;
41-
import org.junit.jupiter.api.Tag;
4243
import org.junit.jupiter.api.Timeout;
4344
import org.junit.jupiter.params.ParameterizedTest;
44-
import org.junit.jupiter.params.provider.ValueSource;
45+
import org.junit.jupiter.params.provider.CsvSource;
4546

46-
import java.time.Duration;
47-
import java.time.temporal.ChronoUnit;
4847
import java.util.concurrent.ExecutionException;
48+
import java.util.concurrent.atomic.AtomicReference;
4949

50-
import static org.apache.hc.core5.util.ReflectionUtils.determineJRELevel;
51-
import static java.lang.String.format;
5250
import static java.util.concurrent.TimeUnit.MILLISECONDS;
5351
import static java.util.concurrent.TimeUnit.SECONDS;
52+
import static org.apache.hc.core5.http2.HttpVersionPolicy.FORCE_HTTP_2;
5453
import static org.junit.jupiter.api.Assertions.assertThrows;
5554
import static org.junit.jupiter.api.Assertions.assertTrue;
5655
import static org.junit.jupiter.api.Assumptions.assumeFalse;
5756

5857
public class TestAsyncTlsHandshakeTimeout {
59-
private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500);
60-
61-
@Tag("slow")
6258
@Timeout(5)
6359
@ParameterizedTest
64-
@ValueSource(strings = { "false", "true" })
65-
void testTimeout(final boolean sendServerHello) throws Exception {
66-
// There is a bug in Java 11: after the handshake times out, the SSLSocket implementation performs a blocking
67-
// read on the socket to wait for close_notify or alert. This operation blocks until the read times out,
68-
// which means that TLS handshakes take twice as long to time out on Java 11. Without a workaround, the only
69-
// option is to skip the timeout duration assertions on Java 11.
70-
assumeFalse(determineJRELevel() == 11, "TLS handshake timeouts are buggy on Java 11");
60+
@CsvSource({
61+
"10,0,false,",
62+
"10,0,true,",
63+
"0,10,false,",
64+
"0,10,true,",
65+
// handshakeTimeout overrides connectTimeout
66+
"30000,10,false,",
67+
"30000,10,true,",
68+
// ALPN and HTTP/2
69+
"0,10,false,FORCE_HTTP_2",
70+
"0,10,true,FORCE_HTTP_2",
71+
"0,10,false,NEGOTIATE",
72+
"0,10,true,NEGOTIATE",
73+
})
74+
void testTimeout(
75+
final int connTimeout,
76+
final int handshakeTimeout,
77+
final boolean sendServerHello,
78+
final HttpVersionPolicy httpVersionPolicy
79+
) throws Exception {
80+
final Builder connectionConfig = ConnectionConfig.custom().setSocketTimeout(300, SECONDS);
81+
final TlsConfig.Builder tlsConfig = TlsConfig.custom();
82+
if (connTimeout > 0) {
83+
connectionConfig.setConnectTimeout(connTimeout, MILLISECONDS);
84+
}
85+
if (handshakeTimeout > 0) {
86+
tlsConfig.setHandshakeTimeout(handshakeTimeout, MILLISECONDS);
87+
}
88+
if (httpVersionPolicy != null) {
89+
tlsConfig.setVersionPolicy(httpVersionPolicy);
90+
}
7191

92+
final AtomicReference<Exception> uncaughtException = new AtomicReference<>();
7293
final PoolingAsyncClientConnectionManager connMgr = PoolingAsyncClientConnectionManagerBuilder.create()
73-
.setDefaultConnectionConfig(ConnectionConfig.custom()
74-
.setConnectTimeout(5, SECONDS)
75-
.setSocketTimeout(5, SECONDS)
76-
.build())
94+
.setDefaultConnectionConfig(connectionConfig.build())
7795
.setTlsStrategy(new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext()))
78-
.setDefaultTlsConfig(TlsConfig.custom()
79-
.setHandshakeTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
80-
.build())
96+
.setDefaultTlsConfig(tlsConfig.build())
8197
.build();
8298
try (
8399
final TlsHandshakeTimeoutServer server = new TlsHandshakeTimeoutServer(sendServerHello);
84100
final CloseableHttpAsyncClient client = HttpAsyncClientBuilder.create()
101+
.setIoReactorExceptionCallback(uncaughtException::set)
85102
.setIOReactorConfig(IOReactorConfig.custom()
86-
.setSelectInterval(TimeValue.ofMilliseconds(50))
103+
.setSelectInterval(TimeValue.ofMilliseconds(10))
87104
.build())
88105
.setConnectionManager(connMgr)
89106
.build()
@@ -94,21 +111,16 @@ void testTimeout(final boolean sendServerHello) throws Exception {
94111
final SimpleHttpRequest request = SimpleHttpRequest.create("GET", "https://127.0.0.1:" + server.getPort());
95112
assertTimeout(request, client);
96113
}
114+
final Exception ex = uncaughtException.get();
115+
if (ex != null) {
116+
assumeFalse(httpVersionPolicy == FORCE_HTTP_2, "Known bug");
117+
throw ex;
118+
}
97119
}
98120

99121
private static void assertTimeout(final SimpleHttpRequest request, final CloseableHttpAsyncClient client) {
100-
final long startTime = System.nanoTime();
101122
final Throwable ex = assertThrows(ExecutionException.class,
102123
() -> client.execute(request, null).get()).getCause();
103-
final Duration actualTime = Duration.of(System.nanoTime() - startTime, ChronoUnit.NANOS);
104-
assertTrue(actualTime.toMillis() > EXPECTED_TIMEOUT.toMillis() / 2,
105-
format("Handshake attempt timed out too soon (only %,d out of %,d ms)",
106-
actualTime.toMillis(),
107-
EXPECTED_TIMEOUT.toMillis()));
108-
assertTrue(actualTime.toMillis() < EXPECTED_TIMEOUT.toMillis() * 2,
109-
format("Handshake attempt timed out too late (%,d out of %,d ms)",
110-
actualTime.toMillis(),
111-
EXPECTED_TIMEOUT.toMillis()));
112-
assertTrue(ex.getMessage().contains(EXPECTED_TIMEOUT.toMillis() + " MILLISECONDS"), ex.getMessage());
124+
assertTrue(ex.getMessage().contains("10 MILLISECONDS"), ex.getMessage());
113125
}
114126
}

‎httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestTlsHandshakeTimeout.java‎

Lines changed: 28 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@
2626
*/
2727
package org.apache.hc.client5.testing.sync;
2828

29-
import org.apache.hc.client5.http.ConnectTimeoutException;
3029
import org.apache.hc.client5.http.classic.HttpClient;
3130
import org.apache.hc.client5.http.classic.methods.HttpGet;
3231
import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
3332
import org.apache.hc.client5.http.config.ConnectionConfig;
33+
import org.apache.hc.client5.http.config.ConnectionConfig.Builder;
3434
import org.apache.hc.client5.http.config.TlsConfig;
3535
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
3636
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
@@ -42,50 +42,50 @@
4242
import org.apache.hc.client5.testing.SSLTestContexts;
4343
import org.apache.hc.client5.testing.tls.TlsHandshakeTimeoutServer;
4444
import org.apache.hc.core5.http.ClassicHttpRequest;
45-
import org.apache.hc.core5.http.io.SocketConfig;
46-
import org.junit.jupiter.api.Tag;
4745
import org.junit.jupiter.api.Timeout;
4846
import org.junit.jupiter.params.ParameterizedTest;
49-
import org.junit.jupiter.params.provider.ValueSource;
47+
import org.junit.jupiter.params.provider.CsvSource;
5048

51-
import java.time.Duration;
52-
import java.time.temporal.ChronoUnit;
53-
54-
import static java.lang.String.format;
5549
import static java.util.concurrent.TimeUnit.MILLISECONDS;
5650
import static java.util.concurrent.TimeUnit.SECONDS;
57-
import static org.apache.hc.core5.util.ReflectionUtils.determineJRELevel;
5851
import static org.junit.jupiter.api.Assertions.assertThrows;
5952
import static org.junit.jupiter.api.Assertions.assertTrue;
60-
import static org.junit.jupiter.api.Assumptions.assumeFalse;
6153

6254
public class TestTlsHandshakeTimeout {
63-
private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500);
64-
65-
@Tag("slow")
6655
@Timeout(5)
6756
@ParameterizedTest
68-
@ValueSource(strings = { "false", "true" })
69-
void testTimeout(final boolean sendServerHello) throws Exception {
57+
@CsvSource({
58+
"10,0,false",
59+
"10,0,true",
60+
"0,10,false",
61+
"0,10,true",
62+
// handshakeTimeout overrides connectTimeout
63+
"30000,10,false",
64+
"30000,10,true",
65+
})
66+
void testTimeout(
67+
final int connTimeout,
68+
final int handshakeTimeout,
69+
final boolean sendServerHello
70+
) throws Exception {
71+
final Builder connectionConfig = ConnectionConfig.custom().setSocketTimeout(300, SECONDS);
72+
final TlsConfig.Builder tlsConfig = TlsConfig.custom();
73+
if (connTimeout > 0) {
74+
connectionConfig.setConnectTimeout(connTimeout, MILLISECONDS);
75+
}
76+
if (handshakeTimeout > 0) {
77+
tlsConfig.setHandshakeTimeout(handshakeTimeout, MILLISECONDS);
78+
}
7079
final PoolingHttpClientConnectionManager connMgr = PoolingHttpClientConnectionManagerBuilder.create()
71-
.setDefaultConnectionConfig(ConnectionConfig.custom()
72-
.setConnectTimeout(5, SECONDS)
73-
.setSocketTimeout(5, SECONDS)
74-
.build())
75-
.setDefaultSocketConfig(SocketConfig.custom()
76-
.setTcpKeepIdle(2)
77-
.setTcpKeepInterval(1)
78-
.setTcpKeepCount(5)
79-
.build())
80+
.setDefaultConnectionConfig(connectionConfig.build())
8081
.setTlsSocketStrategy(new DefaultClientTlsStrategy(SSLTestContexts.createClientSSLContext(), HostnameVerificationPolicy.CLIENT, NoopHostnameVerifier.INSTANCE))
81-
.setDefaultTlsConfig(TlsConfig.custom()
82-
.setHandshakeTimeout(EXPECTED_TIMEOUT.toMillis(), MILLISECONDS)
83-
.build())
82+
.setDefaultTlsConfig(tlsConfig.build())
8483
.build();
8584
try (
8685
final TlsHandshakeTimeoutServer server = new TlsHandshakeTimeoutServer(sendServerHello);
8786
final CloseableHttpClient client = HttpClientBuilder.create()
8887
.setConnectionManager(connMgr)
88+
.disableAutomaticRetries()
8989
.build()
9090
) {
9191
server.start();
@@ -97,27 +97,7 @@ void testTimeout(final boolean sendServerHello) throws Exception {
9797

9898
@SuppressWarnings("deprecation")
9999
private static void assertTimeout(final ClassicHttpRequest request, final HttpClient client) {
100-
// There is a bug in Java 11, and some releases of Java 8: after the
101-
// handshake times out, the SSLSocket implementation performs a
102-
// blocking read on the socket to wait for close_notify or alert. This
103-
// operation blocks until the read times out, which means that TLS
104-
// handshakes take twice as long to time out on Java 11. Without a
105-
// workaround, the only option is to skip the timeout assertions on
106-
// older versions of Java.
107-
assumeFalse(determineJRELevel() <= 11, "TLS handshake timeouts are buggy on Java 11 and earlier");
108-
109-
final long startTime = System.nanoTime();
110-
final ConnectTimeoutException ex = assertThrows(ConnectTimeoutException.class, () -> client.execute(request));
100+
final Exception ex = assertThrows(Exception.class, () -> client.execute(request));
111101
assertTrue(ex.getMessage().contains("Read timed out"), ex.getMessage());
112-
113-
final Duration actualTime = Duration.of(System.nanoTime() - startTime, ChronoUnit.NANOS);
114-
assertTrue(actualTime.toMillis() > EXPECTED_TIMEOUT.toMillis() / 2,
115-
format("Handshake attempt timed out too soon (only %,d out of %,d ms)",
116-
actualTime.toMillis(),
117-
EXPECTED_TIMEOUT.toMillis()));
118-
assertTrue(actualTime.toMillis() < EXPECTED_TIMEOUT.toMillis() * 2,
119-
format("Handshake attempt timed out too late (%,d out of %,d ms)",
120-
actualTime.toMillis(),
121-
EXPECTED_TIMEOUT.toMillis()));
122102
}
123103
}

‎httpclient5-testing/src/test/java/org/apache/hc/client5/testing/tls/TlsHandshakeTimeoutServer.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
package org.apache.hc.client5.testing.tls;
2828

2929
import org.apache.hc.client5.testing.SSLTestContexts;
30+
import org.apache.hc.core5.http2.HttpVersionPolicy;
3031

3132
import javax.net.ssl.SSLContext;
3233
import javax.net.ssl.SSLEngine;
@@ -40,6 +41,8 @@
4041
import java.nio.channels.ServerSocketChannel;
4142
import java.nio.channels.SocketChannel;
4243

44+
import static org.apache.hc.core5.http2.ssl.H2TlsSupport.selectApplicationProtocols;
45+
4346
/**
4447
* This test server accepts a single TLS connection request, which will then time out. The server can run in two modes.
4548
* If {@code sendServerHello} is false, the Client Hello message will be swallowed, and the client will time out while
@@ -90,6 +93,7 @@ private SSLEngine initHandshake() throws Exception {
9093
sslEngine.setEnabledProtocols(new String[]{ "TLSv1.2" });
9194
sslEngine.setUseClientMode(false);
9295
sslEngine.setNeedClientAuth(false);
96+
sslEngine.getSSLParameters().setApplicationProtocols(selectApplicationProtocols(HttpVersionPolicy.NEGOTIATE));
9397

9498
sslEngine.beginHandshake();
9599
return sslEngine;

0 commit comments

Comments
 (0)