Skip to content

Commit

Permalink
Fixes #12348 - HttpClientTransportDynamic does not initialize low-lev…
Browse files Browse the repository at this point in the history
…el 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 <[email protected]>
  • Loading branch information
sbordet authored Oct 10, 2024
1 parent 7b22104 commit f2c70fb
Show file tree
Hide file tree
Showing 13 changed files with 167 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ protected void doStart() throws Exception
if (cookieStore == null)
cookieStore = new HttpCookieStore.Default();

transport.setHttpClient(this);
getContainedBeans(Aware.class).forEach(bean -> bean.setHttpClient(this));

super.doStart();

Expand Down Expand Up @@ -1147,4 +1147,13 @@ public void close() throws Exception
{
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
* 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";
Expand All @@ -45,6 +45,7 @@ public interface HttpClientTransport extends ClientConnectionFactory
*
* @param client the {@link HttpClient} that uses this transport.
*/
@Override
public void setHttpClient(HttpClient client);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -114,8 +114,8 @@ public HttpClientTransportDynamic(ClientConnectionFactory.Info... infos)
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)
);
Expand All @@ -141,7 +141,7 @@ public Origin newOrigin(Request request)
{
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);
Expand All @@ -164,7 +164,7 @@ public Origin newOrigin(Request request)
}
else
{
Info preferredInfo = infos.get(0);
Info preferredInfo = clientConnectionFactoryInfos.get(0);
if (secure)
{
if (preferredInfo.getProtocols(true).contains("h3"))
Expand All @@ -178,7 +178,7 @@ public Origin newOrigin(Request request)
// 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;
Expand All @@ -200,7 +200,7 @@ else if (matches == 1)
else
{
// Pick the first that allows non-secure.
for (Info info : infos)
for (Info info : clientConnectionFactoryInfos)
{
if (info.getProtocols(false).contains("h3"))
continue;
Expand Down Expand Up @@ -249,7 +249,7 @@ public org.eclipse.jetty.io.Connection newConnection(EndPoint endPoint, Map<Stri
if (protocol == null)
{
// Use the default ClientConnectionFactory.
factory = infos.get(0).getClientConnectionFactory();
factory = clientConnectionFactoryInfos.get(0).getClientConnectionFactory();
}
else
{
Expand Down Expand Up @@ -295,7 +295,7 @@ protected Connection newNegotiatedConnection(EndPoint endPoint, Map<String, Obje
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);
}
Expand All @@ -310,7 +310,7 @@ protected Connection newNegotiatedConnection(EndPoint endPoint, Map<String, Obje

private Optional<Info> findClientConnectionFactoryInfo(List<String> protocols, boolean secure)
{
return infos.stream()
return clientConnectionFactoryInfos.stream()
.filter(info -> info.matches(protocols, secure))
.findFirst();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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;
Expand All @@ -35,7 +36,7 @@
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;
Expand All @@ -46,6 +47,12 @@ public ClientConnectionFactoryOverHTTP2(HTTP2Client http2Client)
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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,25 @@ public void setUseALPN(boolean useALPN)
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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,7 @@ public void onClose(Session session, GoAwayFrame frame, Callback callback)
@Test
public void testServerSendsLargeHeader() throws Exception
{
int maxResponseHeadersSize = 8 * 1024;
CompletableFuture<Throwable> serverFailureFuture = new CompletableFuture<>();
CompletableFuture<String> serverCloseReasonFuture = new CompletableFuture<>();
start(new ServerSessionListener()
Expand All @@ -1076,9 +1077,9 @@ public void testServerSendsLargeHeader() throws Exception
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));
Expand All @@ -1100,6 +1101,7 @@ public void onClose(Session session, GoAwayFrame frame, Callback callback)
}
});

http2Client.setMaxResponseHeadersSize(maxResponseHeadersSize);
CompletableFuture<Throwable> clientFailureFuture = new CompletableFuture<>();
CompletableFuture<String> clientCloseReasonFuture = new CompletableFuture<>();
Session.Listener listener = new Session.Listener()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@
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;
Expand All @@ -60,6 +62,7 @@
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;
Expand Down Expand Up @@ -105,10 +108,24 @@
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);
Expand All @@ -127,6 +144,7 @@ public void testPropertiesAreForwarded() throws Exception
assertEquals(httpClient.getIdleTimeout(), http2Client.getIdleTimeout());
assertEquals(httpClient.isUseInputDirectByteBuffers(), http2Client.isUseInputDirectByteBuffers());
assertEquals(httpClient.isUseOutputDirectByteBuffers(), http2Client.isUseOutputDirectByteBuffers());
assertEquals(httpClient.getMaxResponseHeadersSize(), http2Client.getMaxResponseHeadersSize());
}
assertTrue(http2Client.isStopped());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
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;
Expand All @@ -29,15 +30,23 @@
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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,24 +66,27 @@ public HTTP3Client getHTTP3Client()
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)
{
Expand Down
Loading

0 comments on commit f2c70fb

Please sign in to comment.