Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
lorban marked this conversation as resolved.
Show resolved Hide resolved
{
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
Loading