Skip to content

Commit fb110cc

Browse files
sbordetlorban
authored andcommitted
Fixes #13962 - HTTP/2 Client connection timeout does not work.
The problem was that ALPNClientConnection, which is set up when using a HttpClientTransportDynamic with h1 and h2, does not properly override onIdleExpired(). Fixed by overriding onIdleExpired() and failing the connection promise, so that the error is reported to the application. Signed-off-by: Simone Bordet <[email protected]>
1 parent 20968a9 commit fb110cc

File tree

3 files changed

+57
-1
lines changed

3 files changed

+57
-1
lines changed

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ProxyProtocolClientConnectionFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,7 @@ public void succeeded()
496496
@Override
497497
public void failed(Throwable x)
498498
{
499-
close();
499+
getEndPoint().close(x);
500500
Promise<?> promise = (Promise<?>)context.get(org.eclipse.jetty.client.Connection.PROMISE_CONTEXT_KEY);
501501
promise.failed(x);
502502
}

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/NegotiatingClientConnection.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@
1616
import java.io.IOException;
1717
import java.util.Map;
1818
import java.util.concurrent.Executor;
19+
import java.util.concurrent.TimeoutException;
1920
import javax.net.ssl.SSLEngine;
2021

2122
import org.eclipse.jetty.util.BufferUtil;
2223
import org.eclipse.jetty.util.ExceptionUtil;
24+
import org.eclipse.jetty.util.Promise;
2325
import org.slf4j.Logger;
2426
import org.slf4j.LoggerFactory;
2527

@@ -129,6 +131,18 @@ private void replaceConnection()
129131
}
130132
}
131133

134+
@Override
135+
public boolean onIdleExpired(TimeoutException timeout)
136+
{
137+
getEndPoint().close(timeout);
138+
139+
@SuppressWarnings("unchecked")
140+
Promise<Connection> promise = (Promise<Connection>)context.get(ClientConnector.CONNECTION_PROMISE_CONTEXT_KEY);
141+
promise.failed(timeout);
142+
143+
return false;
144+
}
145+
132146
@Override
133147
public void close()
134148
{

jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientTransportDynamicTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,16 @@
1313

1414
package org.eclipse.jetty.test.client.transport;
1515

16+
import java.net.InetSocketAddress;
17+
import java.nio.channels.ServerSocketChannel;
18+
import java.util.ArrayList;
1619
import java.util.List;
1720
import java.util.Random;
1821
import java.util.concurrent.CountDownLatch;
1922
import java.util.concurrent.ExecutionException;
2023
import java.util.concurrent.ThreadLocalRandom;
2124
import java.util.concurrent.TimeUnit;
25+
import java.util.concurrent.TimeoutException;
2226
import java.util.function.Function;
2327

2428
import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory;
@@ -62,6 +66,7 @@
6266
import org.junit.jupiter.api.AfterEach;
6367
import org.junit.jupiter.api.Test;
6468
import org.junit.jupiter.params.ParameterizedTest;
69+
import org.junit.jupiter.params.provider.CsvSource;
6570
import org.junit.jupiter.params.provider.ValueSource;
6671

6772
import static org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory.V1;
@@ -780,4 +785,41 @@ public void testClientOnlySpeaksHTTP1WithExplicitHTTP2Request() throws Exception
780785

781786
assertThat(failure.getCause(), instanceOf(HttpRequestException.class));
782787
}
788+
789+
@ParameterizedTest
790+
@CsvSource(textBlock = """
791+
http, h1
792+
http, h2
793+
http, h1;h2
794+
https, h1
795+
https, h2
796+
https, h1;h2
797+
""")
798+
public void testServerDoesNotAcceptConnections(String scheme, String protocols) throws Exception
799+
{
800+
try (ServerSocketChannel server = ServerSocketChannel.open())
801+
{
802+
// Bind but do not call accept().
803+
server.bind(new InetSocketAddress("0.0.0.0", 0));
804+
int port = ((InetSocketAddress)server.getLocalAddress()).getPort();
805+
806+
ClientConnector clientConnector = new ClientConnector();
807+
List<ClientConnectionFactory.Info> infos = new ArrayList<>();
808+
for (String protocol : protocols.split(";"))
809+
{
810+
if ("h1".equals(protocol))
811+
infos.add(HttpClientConnectionFactory.HTTP11);
812+
if ("h2".equals(protocol))
813+
{
814+
HTTP2Client http2Client = new HTTP2Client(clientConnector);
815+
ClientConnectionFactory.Info http2 = new ClientConnectionFactoryOverHTTP2.HTTP2(http2Client);
816+
infos.add(http2);
817+
}
818+
}
819+
startClient(clientConnector, infos.toArray(ClientConnectionFactory.Info[]::new));
820+
client.setIdleTimeout(1000);
821+
822+
assertThrows(TimeoutException.class, () -> client.GET(scheme + "://localhost:" + port));
823+
}
824+
}
783825
}

0 commit comments

Comments
 (0)