From 5f7c0fcd774efcb2e0036cd1d48ea0a64b94999c Mon Sep 17 00:00:00 2001 From: Aayush Atharva Date: Sat, 19 Sep 2026 18:36:15 +0000 Subject: [PATCH] Fail fast when the origin cannot carry HTTP/2 --- .../netty/request/NettyRequestSender.java | 16 ++- .../Http2ConnectionWaiterGateTest.java | 106 ++++++++++++++++++ 2 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 client/src/test/java/org/asynchttpclient/netty/request/Http2ConnectionWaiterGateTest.java diff --git a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java index 91a57cfdb..7c5fe6ff3 100755 --- a/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java +++ b/client/src/main/java/org/asynchttpclient/netty/request/NettyRequestSender.java @@ -1303,8 +1303,8 @@ private static void validateWebSocketRequest(Request request, AsyncHandler as * rather than waiting out the deadline for its own pinned IP. * * @return the (pending) future when the request was reused or deferred; {@code null} if it should be - * failed with {@code semaphoreException} (a WebSocket request, or on the event loop with no - * connection available) + * failed with {@code semaphoreException} (a WebSocket request, an origin that could never carry + * an HTTP/2 connection, or on the event loop with no connection available) */ private ListenableFuture reuseOrDeferHttp2Connection(Request request, ProxyServer proxy, NettyResponseFuture future, AsyncHandler asyncHandler, IOException semaphoreException) { @@ -1326,6 +1326,18 @@ private ListenableFuture reuseOrDeferHttp2Connection(Request request, Pro if (isOnEventLoop()) { return null; } + // Only wait for a connection that could actually turn up. HTTP/2 gets registered from ALPN on a + // secured origin, or on a cleartext one from an h2c prior-knowledge upgrade, and nothing else (see + // NettyConnectListener). With neither in play nobody will ever register for this key, so the waiter + // could only expire: we would hold the request for connectTimeout and then fail it with the permit + // exception we already have, ignoring the acquireFreeChannelTimeout the caller asked for. + // + // This rests on every hop we open for a cleartext origin being cleartext itself. We never negotiate + // h2 with a proxy, and an h2 connection to one would be registered under this cleartext key, so + // revisit here if that ever changes. + if (!request.getUri().isSecured() && !channelManager.isHttp2CleartextEnabled()) { + return null; + } new Http2ConnectionWaiter<>(request, proxy, future, asyncHandler, override, semaphoreException).arm(); return future; } diff --git a/client/src/test/java/org/asynchttpclient/netty/request/Http2ConnectionWaiterGateTest.java b/client/src/test/java/org/asynchttpclient/netty/request/Http2ConnectionWaiterGateTest.java new file mode 100644 index 000000000..abb56c63f --- /dev/null +++ b/client/src/test/java/org/asynchttpclient/netty/request/Http2ConnectionWaiterGateTest.java @@ -0,0 +1,106 @@ +/* + * Copyright (c) 2026 AsyncHttpClient Project. All rights reserved. + * + * Licensed 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. + */ +package org.asynchttpclient.netty.request; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.asynchttpclient.AbstractBasicTest; +import org.asynchttpclient.AsyncHttpClient; +import org.asynchttpclient.Response; +import org.asynchttpclient.exception.TooManyConnectionsException; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.handler.AbstractHandler; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.time.Duration; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import static org.asynchttpclient.Dsl.asyncHttpClient; +import static org.asynchttpclient.Dsl.config; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * A request that cannot take a connection permit only defers on an HTTP/2 connection that could exist. An + * HTTP/2 connection is registered from ALPN on a secured origin or from an h2c upgrade on a cleartext one, + * so for a cleartext origin with h2c disabled, which is the default, nothing ever registers and the waiter can + * only expire. Arming it there holds the request for the whole {@code connectTimeout} before failing it + * with the permit exception it already had, overriding the {@code acquireFreeChannelTimeout} the caller + * asked for (0 by default: fail fast). + */ +public class Http2ConnectionWaiterGateTest extends AbstractBasicTest { + + private final CountDownLatch release = new CountDownLatch(1); + + @Override + public AbstractHandler configureHandler() throws Exception { + return new BlockingHandler(); + } + + @Test + public void permitExhaustedOnCleartextOriginFailsWithoutWaitingOutConnectTimeout() throws Exception { + // connectTimeout bounds the HTTP/2 waiter, so make it far longer than the window asserted below: a + // regression then fails on the window rather than passing by being merely slow. + try (AsyncHttpClient client = asyncHttpClient(config() + .setMaxConnections(1) + .setConnectTimeout(Duration.ofSeconds(30)))) { + + // execute() takes the permit on this thread before it returns, and the handler holds the + // exchange open, so the second request below is guaranteed to find the semaphore exhausted. + Future holder = client.prepareGet(getTargetUrl()).execute(); + + try { + Future refused = client.prepareGet(getTargetUrl()).execute(); + + // The point of the fix: the refusal is synchronous, so the future is already done by the + // time execute() hands it back. Without this we would only be asserting on the clock, and a + // regression would still end in TooManyConnectionsException, just 30 seconds later. + assertTrue(refused.isDone(), "the permit refusal should fail the future before execute() returns"); + + ExecutionException failure = assertThrows(ExecutionException.class, + () -> refused.get(5, TimeUnit.SECONDS)); + assertInstanceOf(TooManyConnectionsException.class, failure.getCause()); + } finally { + // In a finally so a failed assertion does not leave the handler parked until the server + // stops, which turns one red test into a slow and confusing teardown. + release.countDown(); + } + holder.get(TIMEOUT, TimeUnit.SECONDS); + } + } + + private class BlockingHandler extends AbstractHandler { + + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + throws IOException, ServletException { + try { + release.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new ServletException(e); + } + response.setStatus(HttpServletResponse.SC_OK); + baseRequest.setHandled(true); + } + } +}