Skip to content

Commit 4b8cccc

Browse files
committed
Improve HTTP client alt-svc alternative validation.
Motivation: HTTP client alternative validation test should ensure that the failure of an alternative not presenting a valid certificate for the origin host is an hostname verification check. Changes: Add an exception handler on HTTP client to capture pool connection errors. Use HTTP client exception handler to catch the connection error and verify the failure is due to a client invalid hostname failure.
1 parent 0771c22 commit 4b8cccc

File tree

5 files changed

+41
-18
lines changed

5 files changed

+41
-18
lines changed

vertx-core/src/main/java/io/vertx/core/http/impl/CleanableHttpClient.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import io.vertx.core.Completable;
1414
import io.vertx.core.Future;
15+
import io.vertx.core.Handler;
1516
import io.vertx.core.http.*;
1617
import io.vertx.core.http.HttpClientConnection;
1718
import io.vertx.core.internal.VertxInternal;
@@ -108,6 +109,11 @@ public HttpClientTransport quicTransport() {
108109
return delegate.quicTransport();
109110
}
110111

112+
@Override
113+
public HttpClientInternal exceptionHandler(Handler<Throwable> handler) {
114+
return delegate.exceptionHandler(handler);
115+
}
116+
111117
@Override
112118
public HttpClientOptions options() {
113119
return delegate.options();

vertx-core/src/main/java/io/vertx/core/http/impl/HttpClientImpl.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public class HttpClientImpl extends HttpClientBase implements HttpClientInternal
7070
private final int maxRedirects;
7171
private final List<HttpVersion> versions;
7272
private final Handler<HttpConnection> connectHandler;
73+
private volatile Handler<Throwable> exceptionHandler;
7374
private volatile ClientSSLOptions sslOptions;
7475

7576
HttpClientImpl(VertxInternal vertx,
@@ -233,6 +234,7 @@ private Function<EndpointKey, SharedHttpClientConnectionGroup> httpEndpointProvi
233234
});
234235
}
235236
},
237+
exceptionHandler,
236238
p,
237239
poolMetrics,
238240
key.authority,
@@ -250,6 +252,12 @@ public HttpClientTransport quicTransport() {
250252
return quicTransport;
251253
}
252254

255+
@Override
256+
public HttpClientInternal exceptionHandler(Handler<Throwable> handler) {
257+
this.exceptionHandler = handler;
258+
return this;
259+
}
260+
253261
protected void setDefaultSslOptions(ClientSSLOptions options) {
254262
configureSSLOptions(verifyHost, options);
255263
this.sslOptions = options;

vertx-core/src/main/java/io/vertx/core/http/impl/SharedHttpClientConnectionGroup.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,15 @@ class SharedHttpClientConnectionGroup extends ManagedResource {
6666
private final HttpClientMetrics<?, ?> httpMetrics;
6767
private final ClientMetrics<?, ?, ?> clientMetrics;
6868
private final Handler<HttpConnection> connectHandler;
69+
private final Handler<Throwable> exceptionHandler;
6970
private final Pool pool;
7071
private final HostAndPort authority;
7172
private final SocketAddress server;
7273

7374
public SharedHttpClientConnectionGroup(ClientMetrics<?, ?, ?> clientMetrics,
7475
HttpClientMetrics<?, ?> httpMetrics,
7576
Handler<HttpConnection> connectHandler,
77+
Handler<Throwable> exceptionHandler,
7678
Function<SharedHttpClientConnectionGroup, Pool> poolProvider,
7779
PoolMetrics poolMetrics,
7880
HostAndPort authority,
@@ -84,6 +86,7 @@ public SharedHttpClientConnectionGroup(ClientMetrics<?, ?, ?> clientMetrics,
8486
this.pool = poolProvider.apply(this);
8587
this.server = server;
8688
this.connectHandler = connectHandler;
89+
this.exceptionHandler = exceptionHandler;
8790
}
8891

8992
public int size() {
@@ -233,7 +236,7 @@ static class Pool implements PoolConnector<HttpClientConnection> {
233236

234237
@Override
235238
public Future<ConnectResult<HttpClientConnection>> connect(ContextInternal context, Listener listener) {
236-
return connector
239+
Future<ConnectResult<HttpClientConnection>> fut = connector
237240
.connect(context, owner.server, owner.authority, connectParams, owner.clientMetrics)
238241
.map(connection -> {
239242
connection.evictionHandler(v -> {
@@ -243,9 +246,17 @@ public Future<ConnectResult<HttpClientConnection>> connect(ContextInternal conte
243246
connection.concurrencyChangeHandler(listener::onConcurrencyChange);
244247
owner.init(connection);
245248
long capacity = connection.concurrency();
246-
int idx = connection.protocolVersion() != HttpVersion.HTTP_2 ? 0: 1;
249+
int idx = connection.protocolVersion() != HttpVersion.HTTP_2 ? 0 : 1;
247250
return new ConnectResult<>(connection, capacity, idx);
248251
});
252+
if (owner.exceptionHandler != null) {
253+
fut = fut.andThen(ar -> {
254+
if (ar.failed()) {
255+
owner.exceptionHandler.handle(ar.cause());
256+
}
257+
});
258+
}
259+
return fut;
249260
}
250261

251262
@Override

vertx-core/src/main/java/io/vertx/core/internal/http/HttpClientInternal.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
import io.vertx.core.Closeable;
1515
import io.vertx.core.Future;
16+
import io.vertx.core.Handler;
1617
import io.vertx.core.http.*;
1718
import io.vertx.core.internal.VertxInternal;
1819
import io.vertx.core.internal.net.endpoint.EndpointResolverInternal;
@@ -36,6 +37,8 @@ public interface HttpClientInternal extends HttpClientAgent, MetricsProvider, Cl
3637

3738
HttpClientTransport quicTransport();
3839

40+
HttpClientInternal exceptionHandler(Handler<Throwable> handler);
41+
3942
// Should not be here but currently necessary for WebClient
4043
@Deprecated(forRemoval = true)
4144
default HttpClientOptions options() {

vertx-core/src/test/java/io/vertx/tests/http/HttpAlternativesTest.java

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,7 @@
3333
import org.junit.Test;
3434

3535
import javax.net.ssl.SSLHandshakeException;
36-
import java.util.IdentityHashMap;
37-
import java.util.List;
38-
import java.util.Map;
36+
import java.util.*;
3937
import java.util.concurrent.atomic.AtomicInteger;
4038
import java.util.concurrent.atomic.AtomicReference;
4139
import java.util.function.Supplier;
@@ -334,7 +332,7 @@ public void testIgnoreAlternativeWithoutSNI() {
334332
}
335333

336334
@Test
337-
public void testCertificateValidation() {
335+
public void testServerCertValidation() {
338336
startServer(4043, Cert.SNI_JKS, HttpVersion.HTTP_1_1)
339337
.handler(request -> {
340338
assertNull(request.getHeader(HttpHeaders.ALT_USED));
@@ -349,24 +347,21 @@ public void testCertificateValidation() {
349347
fail();
350348
});
351349
client = vertx.createHttpClient(new HttpClientConfig().setFollowAlternativeServices(true).setSsl(true), new ClientSSLOptions().setTrustAll(true));
350+
List<SSLHandshakeException> connectErrors = Collections.synchronizedList(new ArrayList<>());
351+
((HttpClientInternal)client).exceptionHandler(err -> {
352+
if (err instanceof SSLHandshakeException) {
353+
connectErrors.add((SSLHandshakeException)err);
354+
}
355+
});
352356
Buffer body = client.request(HttpMethod.GET, 4043, "host2.com", "/")
353357
.compose(request -> request
354358
.send()
355359
.compose(HttpClientResponse::body)
356360
).await();
357361
assertEquals("host2.com:4043", body.toString());
358-
try {
359-
client.request(new RequestOptions().setHost("host2.com").setProtocolVersion(HttpVersion.HTTP_2).setPort(4043).setURI("/"))
360-
.compose(request -> request
361-
.send()
362-
.expecting(response -> request.version() == HttpVersion.HTTP_2)
363-
.compose(HttpClientResponse::body)
364-
).await();
365-
fail();
366-
} catch (Exception e) {
367-
assertEquals(SSLHandshakeException.class, e.getClass());
368-
assertTrue(e.getCause().getMessage().contains("no_application_protocol"));
369-
}
362+
assertWaitUntil(() -> !connectErrors.isEmpty());
363+
SSLHandshakeException err = connectErrors.get(0);
364+
assertTrue(err.getCause().getMessage().contains("No name matching host2.com found"));
370365
}
371366

372367
@Test

0 commit comments

Comments
 (0)