Fixes #12348 - HttpClientTransportDynamic does not initialize low-level clients. (#12349)

Fixed initialization for HTTP/2 and HTTP/3.
Fixed test that was relying on default configuration, rather than explicit.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Simone Bordet 2024-10-10 18:03:39 +03:00 committed by GitHub
parent 7b22104cab
commit f2c70fb8f3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 167 additions and 54 deletions

View File

@ -218,7 +218,7 @@ public class HttpClient extends ContainerLifeCycle implements AutoCloseable
if (cookieStore == null)
cookieStore = new HttpCookieStore.Default();
transport.setHttpClient(this);
getContainedBeans(Aware.class).forEach(bean -> bean.setHttpClient(this));
super.doStart();
@ -1147,4 +1147,13 @@ public class HttpClient extends ContainerLifeCycle implements AutoCloseable
{
stop();
}
/**
* <p>Descendant beans of {@code HttpClient} that implement this interface
* are made aware of the {@code HttpClient} instance while it is starting.</p>
*/
public interface Aware
{
void setHttpClient(HttpClient httpClient);
}
}

View File

@ -31,7 +31,7 @@ import org.eclipse.jetty.io.ClientConnectionFactory;
* but the HTTP exchange may also be carried using the FCGI protocol, the HTTP/2 protocol or,
* in future, other protocols.
*/
public interface HttpClientTransport extends ClientConnectionFactory
public interface HttpClientTransport extends ClientConnectionFactory, HttpClient.Aware
{
public static final String HTTP_DESTINATION_CONTEXT_KEY = "org.eclipse.jetty.client.destination";
public static final String HTTP_CONNECTION_PROMISE_CONTEXT_KEY = "org.eclipse.jetty.client.connection.promise";
@ -45,6 +45,7 @@ public interface HttpClientTransport extends ClientConnectionFactory
*
* @param client the {@link HttpClient} that uses this transport.
*/
@Override
public void setHttpClient(HttpClient client);
/**

View File

@ -81,7 +81,7 @@ public class HttpClientTransportDynamic extends AbstractConnectorHttpClientTrans
{
private static final Logger LOG = LoggerFactory.getLogger(HttpClientTransportDynamic.class);
private final List<ClientConnectionFactory.Info> infos;
private final List<ClientConnectionFactory.Info> clientConnectionFactoryInfos;
/**
* Creates a dynamic transport that speaks only HTTP/1.1.
@ -114,8 +114,8 @@ public class HttpClientTransportDynamic extends AbstractConnectorHttpClientTrans
public HttpClientTransportDynamic(ClientConnector connector, ClientConnectionFactory.Info... infos)
{
super(connector);
this.infos = infos.length == 0 ? List.of(HttpClientConnectionFactory.HTTP11) : List.of(infos);
this.infos.forEach(this::installBean);
this.clientConnectionFactoryInfos = infos.length == 0 ? List.of(HttpClientConnectionFactory.HTTP11) : List.of(infos);
this.clientConnectionFactoryInfos.forEach(this::installBean);
setConnectionPoolFactory(destination ->
new MultiplexConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), 1)
);
@ -141,7 +141,7 @@ public class HttpClientTransportDynamic extends AbstractConnectorHttpClientTrans
{
HttpVersion version = request.getVersion();
List<String> wanted = toProtocols(version);
for (Info info : infos)
for (Info info : clientConnectionFactoryInfos)
{
// Find the first protocol that matches the version.
List<String> protocols = info.getProtocols(secure);
@ -164,7 +164,7 @@ public class HttpClientTransportDynamic extends AbstractConnectorHttpClientTrans
}
else
{
Info preferredInfo = infos.get(0);
Info preferredInfo = clientConnectionFactoryInfos.get(0);
if (secure)
{
if (preferredInfo.getProtocols(true).contains("h3"))
@ -178,7 +178,7 @@ public class HttpClientTransportDynamic extends AbstractConnectorHttpClientTrans
// If the preferred protocol is not HTTP/3, then
// must be excluded since it won't be compatible
// with the other HTTP versions due to UDP vs TCP.
for (Info info : infos)
for (Info info : clientConnectionFactoryInfos)
{
if (info.getProtocols(true).contains("h3"))
continue;
@ -200,7 +200,7 @@ public class HttpClientTransportDynamic extends AbstractConnectorHttpClientTrans
else
{
// Pick the first that allows non-secure.
for (Info info : infos)
for (Info info : clientConnectionFactoryInfos)
{
if (info.getProtocols(false).contains("h3"))
continue;
@ -249,7 +249,7 @@ public class HttpClientTransportDynamic extends AbstractConnectorHttpClientTrans
if (protocol == null)
{
// Use the default ClientConnectionFactory.
factory = infos.get(0).getClientConnectionFactory();
factory = clientConnectionFactoryInfos.get(0).getClientConnectionFactory();
}
else
{
@ -295,7 +295,7 @@ public class HttpClientTransportDynamic extends AbstractConnectorHttpClientTrans
else
{
// Server does not support ALPN, let's try the first protocol.
factoryInfo = infos.get(0);
factoryInfo = clientConnectionFactoryInfos.get(0);
if (LOG.isDebugEnabled())
LOG.debug("No ALPN protocol, using {}", factoryInfo);
}
@ -310,7 +310,7 @@ public class HttpClientTransportDynamic extends AbstractConnectorHttpClientTrans
private Optional<Info> findClientConnectionFactoryInfo(List<String> protocols, boolean secure)
{
return infos.stream()
return clientConnectionFactoryInfos.stream()
.filter(info -> info.matches(protocols, secure))
.findFirst();
}

View File

@ -19,6 +19,7 @@ import java.util.List;
import java.util.Map;
import org.eclipse.jetty.client.Connection;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpClientTransport;
import org.eclipse.jetty.client.transport.HttpClientConnectionFactory;
import org.eclipse.jetty.client.transport.HttpClientTransportDynamic;
@ -35,7 +36,7 @@ import org.eclipse.jetty.io.ssl.SslConnection;
import org.eclipse.jetty.util.Promise;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
public class ClientConnectionFactoryOverHTTP2 extends ContainerLifeCycle implements ClientConnectionFactory
public class ClientConnectionFactoryOverHTTP2 extends ContainerLifeCycle implements ClientConnectionFactory, HttpClient.Aware
{
private final ClientConnectionFactory factory = new HTTP2ClientConnectionFactory();
private final HTTP2Client http2Client;
@ -46,6 +47,12 @@ public class ClientConnectionFactoryOverHTTP2 extends ContainerLifeCycle impleme
installBean(http2Client);
}
@Override
public void setHttpClient(HttpClient httpClient)
{
HttpClientTransportOverHTTP2.configure(httpClient, http2Client);
}
@Override
public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map<String, Object> context) throws IOException
{

View File

@ -87,22 +87,25 @@ public class HttpClientTransportOverHTTP2 extends AbstractHttpClientTransport
protected void doStart() throws Exception
{
if (!http2Client.isStarted())
{
HttpClient httpClient = getHttpClient();
http2Client.setExecutor(httpClient.getExecutor());
http2Client.setScheduler(httpClient.getScheduler());
http2Client.setByteBufferPool(httpClient.getByteBufferPool());
http2Client.setConnectTimeout(httpClient.getConnectTimeout());
http2Client.setIdleTimeout(httpClient.getIdleTimeout());
http2Client.setInputBufferSize(httpClient.getResponseBufferSize());
http2Client.setUseInputDirectByteBuffers(httpClient.isUseInputDirectByteBuffers());
http2Client.setUseOutputDirectByteBuffers(httpClient.isUseOutputDirectByteBuffers());
http2Client.setConnectBlocking(httpClient.isConnectBlocking());
http2Client.setBindAddress(httpClient.getBindAddress());
}
configure(getHttpClient(), getHTTP2Client());
super.doStart();
}
static void configure(HttpClient httpClient, HTTP2Client http2Client)
{
http2Client.setExecutor(httpClient.getExecutor());
http2Client.setScheduler(httpClient.getScheduler());
http2Client.setByteBufferPool(httpClient.getByteBufferPool());
http2Client.setConnectTimeout(httpClient.getConnectTimeout());
http2Client.setIdleTimeout(httpClient.getIdleTimeout());
http2Client.setInputBufferSize(httpClient.getResponseBufferSize());
http2Client.setUseInputDirectByteBuffers(httpClient.isUseInputDirectByteBuffers());
http2Client.setUseOutputDirectByteBuffers(httpClient.isUseOutputDirectByteBuffers());
http2Client.setConnectBlocking(httpClient.isConnectBlocking());
http2Client.setBindAddress(httpClient.getBindAddress());
http2Client.setMaxResponseHeadersSize(httpClient.getMaxResponseHeadersSize());
}
@Override
public Origin newOrigin(Request request)
{

View File

@ -1068,6 +1068,7 @@ public class HTTP2Test extends AbstractTest
@Test
public void testServerSendsLargeHeader() throws Exception
{
int maxResponseHeadersSize = 8 * 1024;
CompletableFuture<Throwable> serverFailureFuture = new CompletableFuture<>();
CompletableFuture<String> serverCloseReasonFuture = new CompletableFuture<>();
start(new ServerSessionListener()
@ -1076,9 +1077,9 @@ public class HTTP2Test extends AbstractTest
public Stream.Listener onNewStream(Stream stream, HeadersFrame frame)
{
HTTP2Session session = (HTTP2Session)stream.getSession();
session.getGenerator().getHpackEncoder().setMaxHeaderListSize(1024 * 1024);
session.getGenerator().getHpackEncoder().setMaxHeaderListSize(2 * maxResponseHeadersSize);
String value = "x".repeat(8 * 1024);
String value = "x".repeat(maxResponseHeadersSize);
HttpFields fields = HttpFields.build().put("custom", value);
MetaData.Response response = new MetaData.Response(HttpStatus.OK_200, null, HttpVersion.HTTP_2, fields);
stream.headers(new HeadersFrame(stream.getId(), response, null, true));
@ -1100,6 +1101,7 @@ public class HTTP2Test extends AbstractTest
}
});
http2Client.setMaxResponseHeadersSize(maxResponseHeadersSize);
CompletableFuture<Throwable> clientFailureFuture = new CompletableFuture<>();
CompletableFuture<String> clientCloseReasonFuture = new CompletableFuture<>();
Session.Listener listener = new Session.Listener()

View File

@ -40,11 +40,13 @@ import org.eclipse.jetty.client.Connection;
import org.eclipse.jetty.client.ContentResponse;
import org.eclipse.jetty.client.Destination;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpClientTransport;
import org.eclipse.jetty.client.HttpProxy;
import org.eclipse.jetty.client.InputStreamResponseListener;
import org.eclipse.jetty.client.Origin;
import org.eclipse.jetty.client.Response;
import org.eclipse.jetty.client.Result;
import org.eclipse.jetty.client.transport.HttpClientTransportDynamic;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
@ -60,6 +62,7 @@ import org.eclipse.jetty.http2.api.Session;
import org.eclipse.jetty.http2.api.Stream;
import org.eclipse.jetty.http2.api.server.ServerSessionListener;
import org.eclipse.jetty.http2.client.HTTP2Client;
import org.eclipse.jetty.http2.client.transport.ClientConnectionFactoryOverHTTP2;
import org.eclipse.jetty.http2.client.transport.HttpClientTransportOverHTTP2;
import org.eclipse.jetty.http2.client.transport.internal.HttpChannelOverHTTP2;
import org.eclipse.jetty.http2.client.transport.internal.HttpConnectionOverHTTP2;
@ -105,10 +108,24 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
public class HttpClientTransportOverHTTP2Test extends AbstractTest
{
@Test
public void testPropertiesAreForwarded() throws Exception
public void testPropertiesAreForwardedOverHTTP2() throws Exception
{
HTTP2Client http2Client = new HTTP2Client();
try (HttpClient httpClient = new HttpClient(new HttpClientTransportOverHTTP2(http2Client)))
ClientConnector clientConnector = new ClientConnector();
HTTP2Client http2Client = new HTTP2Client(clientConnector);
testPropertiesAreForwarded(http2Client, new HttpClientTransportOverHTTP2(http2Client));
}
@Test
public void testPropertiesAreForwardedDynamic() throws Exception
{
ClientConnector clientConnector = new ClientConnector();
HTTP2Client http2Client = new HTTP2Client(clientConnector);
testPropertiesAreForwarded(http2Client, new HttpClientTransportDynamic(clientConnector, new ClientConnectionFactoryOverHTTP2.HTTP2(http2Client)));
}
private void testPropertiesAreForwarded(HTTP2Client http2Client, HttpClientTransport httpClientTransport) throws Exception
{
try (HttpClient httpClient = new HttpClient(httpClientTransport))
{
Executor executor = new QueuedThreadPool();
httpClient.setExecutor(executor);
@ -127,6 +144,7 @@ public class HttpClientTransportOverHTTP2Test extends AbstractTest
assertEquals(httpClient.getIdleTimeout(), http2Client.getIdleTimeout());
assertEquals(httpClient.isUseInputDirectByteBuffers(), http2Client.isUseInputDirectByteBuffers());
assertEquals(httpClient.isUseOutputDirectByteBuffers(), http2Client.isUseOutputDirectByteBuffers());
assertEquals(httpClient.getMaxResponseHeadersSize(), http2Client.getMaxResponseHeadersSize());
}
assertTrue(http2Client.isStopped());
}

View File

@ -16,6 +16,7 @@ package org.eclipse.jetty.http3.client.transport;
import java.util.List;
import java.util.Map;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.transport.HttpClientConnectionFactory;
import org.eclipse.jetty.client.transport.HttpClientTransportDynamic;
import org.eclipse.jetty.http3.client.HTTP3Client;
@ -29,15 +30,23 @@ import org.eclipse.jetty.quic.common.ProtocolSession;
import org.eclipse.jetty.quic.common.QuicSession;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
public class ClientConnectionFactoryOverHTTP3 extends ContainerLifeCycle implements ClientConnectionFactory
public class ClientConnectionFactoryOverHTTP3 extends ContainerLifeCycle implements ClientConnectionFactory, HttpClient.Aware
{
private final HTTP3ClientConnectionFactory factory = new HTTP3ClientConnectionFactory();
private final HTTP3Client http3Client;
public ClientConnectionFactoryOverHTTP3(HTTP3Client http3Client)
{
this.http3Client = http3Client;
installBean(http3Client);
}
@Override
public void setHttpClient(HttpClient httpClient)
{
HttpClientTransportOverHTTP3.configure(httpClient, http3Client);
}
@Override
public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map<String, Object> context)
{

View File

@ -66,24 +66,27 @@ public class HttpClientTransportOverHTTP3 extends AbstractHttpClientTransport im
protected void doStart() throws Exception
{
if (!http3Client.isStarted())
{
HttpClient httpClient = getHttpClient();
ClientConnector clientConnector = this.http3Client.getClientConnector();
clientConnector.setExecutor(httpClient.getExecutor());
clientConnector.setScheduler(httpClient.getScheduler());
clientConnector.setByteBufferPool(httpClient.getByteBufferPool());
clientConnector.setConnectTimeout(Duration.ofMillis(httpClient.getConnectTimeout()));
clientConnector.setConnectBlocking(httpClient.isConnectBlocking());
clientConnector.setBindAddress(httpClient.getBindAddress());
clientConnector.setIdleTimeout(Duration.ofMillis(httpClient.getIdleTimeout()));
HTTP3Configuration configuration = http3Client.getHTTP3Configuration();
configuration.setInputBufferSize(httpClient.getResponseBufferSize());
configuration.setUseInputDirectByteBuffers(httpClient.isUseInputDirectByteBuffers());
configuration.setUseOutputDirectByteBuffers(httpClient.isUseOutputDirectByteBuffers());
}
configure(getHttpClient(), http3Client);
super.doStart();
}
static void configure(HttpClient httpClient, HTTP3Client http3Client)
{
ClientConnector clientConnector = http3Client.getClientConnector();
clientConnector.setExecutor(httpClient.getExecutor());
clientConnector.setScheduler(httpClient.getScheduler());
clientConnector.setByteBufferPool(httpClient.getByteBufferPool());
clientConnector.setConnectTimeout(Duration.ofMillis(httpClient.getConnectTimeout()));
clientConnector.setConnectBlocking(httpClient.isConnectBlocking());
clientConnector.setBindAddress(httpClient.getBindAddress());
clientConnector.setIdleTimeout(Duration.ofMillis(httpClient.getIdleTimeout()));
HTTP3Configuration configuration = http3Client.getHTTP3Configuration();
configuration.setInputBufferSize(httpClient.getResponseBufferSize());
configuration.setUseInputDirectByteBuffers(httpClient.isUseInputDirectByteBuffers());
configuration.setUseOutputDirectByteBuffers(httpClient.isUseOutputDirectByteBuffers());
configuration.setMaxResponseHeadersSize(httpClient.getMaxResponseHeadersSize());
}
@Override
public Origin newOrigin(Request request)
{

View File

@ -16,30 +16,87 @@ package org.eclipse.jetty.http3.tests;
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jetty.client.ContentResponse;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpClientTransport;
import org.eclipse.jetty.client.Response;
import org.eclipse.jetty.client.transport.HttpClientTransportDynamic;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http3.HTTP3Configuration;
import org.eclipse.jetty.http3.client.HTTP3Client;
import org.eclipse.jetty.http3.client.transport.ClientConnectionFactoryOverHTTP3;
import org.eclipse.jetty.http3.client.transport.HttpClientTransportOverHTTP3;
import org.eclipse.jetty.io.ClientConnector;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.quic.client.ClientQuicConfiguration;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class HttpClientTransportOverHTTP3Test extends AbstractClientServerTest
{
@Test
public void testPropertiesAreForwardedOverHTTP3() throws Exception
{
ClientConnector clientConnector = new ClientConnector();
HTTP3Client http3Client = new HTTP3Client(new ClientQuicConfiguration(new SslContextFactory.Client(), null), clientConnector);
testPropertiesAreForwarded(http3Client, new HttpClientTransportOverHTTP3(http3Client));
}
@Test
public void testPropertiesAreForwardedDynamic() throws Exception
{
ClientConnector clientConnector = new ClientConnector();
HTTP3Client http3Client = new HTTP3Client(new ClientQuicConfiguration(new SslContextFactory.Client(), null), clientConnector);
testPropertiesAreForwarded(http3Client, new HttpClientTransportDynamic(clientConnector, new ClientConnectionFactoryOverHTTP3.HTTP3(http3Client)));
}
private void testPropertiesAreForwarded(HTTP3Client http3Client, HttpClientTransport httpClientTransport) throws Exception
{
try (HttpClient httpClient = new HttpClient(httpClientTransport))
{
Executor executor = new QueuedThreadPool();
httpClient.setExecutor(executor);
httpClient.setConnectTimeout(13);
httpClient.setIdleTimeout(17);
httpClient.setUseInputDirectByteBuffers(false);
httpClient.setUseOutputDirectByteBuffers(false);
httpClient.start();
assertTrue(http3Client.isStarted());
ClientConnector clientConnector = http3Client.getClientConnector();
assertSame(httpClient.getExecutor(), clientConnector.getExecutor());
assertSame(httpClient.getScheduler(), clientConnector.getScheduler());
assertSame(httpClient.getByteBufferPool(), clientConnector.getByteBufferPool());
assertEquals(httpClient.getConnectTimeout(), clientConnector.getConnectTimeout().toMillis());
assertEquals(httpClient.getIdleTimeout(), clientConnector.getIdleTimeout().toMillis());
HTTP3Configuration http3Configuration = http3Client.getHTTP3Configuration();
assertEquals(httpClient.isUseInputDirectByteBuffers(), http3Configuration.isUseInputDirectByteBuffers());
assertEquals(httpClient.isUseOutputDirectByteBuffers(), http3Configuration.isUseOutputDirectByteBuffers());
assertEquals(httpClient.getMaxResponseHeadersSize(), http3Configuration.getMaxResponseHeadersSize());
}
assertTrue(http3Client.isStopped());
}
@Test
public void testRequestHasHTTP3Version() throws Exception
{

View File

@ -73,7 +73,7 @@ public interface ClientConnectionFactory
public Info(ClientConnectionFactory factory)
{
this.factory = factory;
addBean(factory);
installBean(factory);
}
/**

View File

@ -130,15 +130,14 @@ public interface Container
/**
* @param clazz the class of the beans
* @param <T> the Bean type
* @return the list of beans of the given class from the entire Container hierarchy.
* The order is by depth first and then the order beans were added.
* @param <T> the bean type
* @return the collection of beans of the given class from the {@code Container} hierarchy
*/
<T> Collection<T> getContainedBeans(Class<T> clazz);
/**
* Get the beans added to the container that are EventListeners.
* This is essentially equivalent to <code>getBeans(EventListener.class);</code>,
* This is essentially equivalent to {@code getBeans(EventListener.class)},
* except that: <ul>
* <li>The result may be precomputed, so it can be more efficient</li>
* <li>The result is ordered by the order added.</li>

View File

@ -19,7 +19,7 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.EventListener;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
@ -813,6 +813,11 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container,
return _managed == Managed.MANAGED;
}
/**
* @return {@code true} if this bean {@link #isManaged() is managed};
* {@code true} if this bean will be managed if it were to be started;
* {@code false} otherwise
*/
public boolean isManageable()
{
return switch (_managed)
@ -882,7 +887,7 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container,
@Override
public <T> Collection<T> getContainedBeans(Class<T> clazz)
{
Set<T> beans = new HashSet<>();
Set<T> beans = new LinkedHashSet<>();
getContainedBeans(clazz, beans);
return beans;
}