From baab1a15b98e5ba62eb8d6c8c1a387031e2d21ed Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 27 Oct 2021 20:55:11 +0200 Subject: [PATCH] Issue #6728 - QUIC and HTTP/3 - More fixes and improvement to HTTP client transport tests. Signed-off-by: Simone Bordet --- .../client/LeakTrackingConnectionPool.java | 6 +-- .../server/AbstractHttpClientServerTest.java | 3 +- .../quic/server/ServerQuicConnection.java | 4 +- .../http/client/HttpClientDemandTest.java | 7 +-- .../jetty/http/client/HttpClientLoadTest.java | 53 +++++++++---------- .../http/client/HttpClientStreamTest.java | 7 +++ 6 files changed, 41 insertions(+), 39 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/LeakTrackingConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/LeakTrackingConnectionPool.java index 20f98834699..18f39546086 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/LeakTrackingConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/LeakTrackingConnectionPool.java @@ -24,7 +24,7 @@ public class LeakTrackingConnectionPool extends DuplexConnectionPool { private static final Logger LOG = LoggerFactory.getLogger(LeakTrackingConnectionPool.class); - private final LeakDetector leakDetector = new LeakDetector() + private final LeakDetector leakDetector = new LeakDetector<>() { @Override protected void leaked(LeakInfo leakInfo) @@ -35,7 +35,7 @@ public class LeakTrackingConnectionPool extends DuplexConnectionPool public LeakTrackingConnectionPool(HttpDestination destination, int maxConnections, Callback requester) { - super((HttpDestination)destination, maxConnections, requester); + super(destination, maxConnections, requester); addBean(leakDetector); } @@ -60,7 +60,7 @@ public class LeakTrackingConnectionPool extends DuplexConnectionPool LOG.info("Connection {}@{} released but not acquired", connection, leakDetector.id(connection)); } - protected void leaked(LeakDetector.LeakInfo leakInfo) + protected void leaked(LeakDetector.LeakInfo leakInfo) { LOG.info("Connection {} leaked at:", leakInfo.getResourceDescription(), leakInfo.getStackFrames()); } diff --git a/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java b/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java index 2c18f80746d..ee10b68d2c6 100644 --- a/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java +++ b/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java @@ -18,6 +18,7 @@ import java.util.concurrent.atomic.AtomicLong; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.HttpClientTransport; import org.eclipse.jetty.client.LeakTrackingConnectionPool; +import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.fcgi.client.http.HttpClientTransportOverFCGI; import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.io.ByteBufferPool; @@ -71,7 +72,7 @@ public abstract class AbstractHttpClientServerTest transport.setConnectionPoolFactory(destination -> new LeakTrackingConnectionPool(destination, client.getMaxConnectionsPerDestination(), destination) { @Override - protected void leaked(LeakDetector.LeakInfo leakInfo) + protected void leaked(LeakDetector.LeakInfo leakInfo) { connectionLeaks.incrementAndGet(); } diff --git a/jetty-quic/quic-server/src/main/java/org/eclipse/jetty/quic/server/ServerQuicConnection.java b/jetty-quic/quic-server/src/main/java/org/eclipse/jetty/quic/server/ServerQuicConnection.java index d68f161f0be..8bbb2438cab 100644 --- a/jetty-quic/quic-server/src/main/java/org/eclipse/jetty/quic/server/ServerQuicConnection.java +++ b/jetty-quic/quic-server/src/main/java/org/eclipse/jetty/quic/server/ServerQuicConnection.java @@ -26,7 +26,6 @@ import org.eclipse.jetty.quic.common.QuicConnection; import org.eclipse.jetty.quic.common.QuicSession; import org.eclipse.jetty.quic.quiche.QuicheConfig; import org.eclipse.jetty.quic.quiche.QuicheConnection; -import org.eclipse.jetty.quic.quiche.ffi.LibQuiche; import org.eclipse.jetty.quic.server.internal.SimpleTokenMinter; import org.eclipse.jetty.quic.server.internal.SimpleTokenValidator; import org.eclipse.jetty.server.Connector; @@ -70,8 +69,7 @@ public class ServerQuicConnection extends QuicConnection QuicheConnection quicheConnection = QuicheConnection.tryAccept(quicheConfig, new SimpleTokenValidator((InetSocketAddress)remoteAddress), cipherBuffer, remoteAddress); if (quicheConnection == null) { - // TODO make the buffer size configurable - ByteBuffer negotiationBuffer = byteBufferPool.acquire(LibQuiche.QUICHE_MIN_CLIENT_INITIAL_LEN, true); + ByteBuffer negotiationBuffer = byteBufferPool.acquire(getOutputBufferSize(), true); int pos = BufferUtil.flipToFill(negotiationBuffer); // TODO make the token minter configurable if (!QuicheConnection.negotiate(new SimpleTokenMinter((InetSocketAddress)remoteAddress), cipherBuffer, negotiationBuffer)) diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientDemandTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientDemandTest.java index 96e2667c63f..3e608f113a3 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientDemandTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientDemandTest.java @@ -126,7 +126,9 @@ public class HttpClientDemandTest extends AbstractTest { init(transport); - int bufferSize = 512; + // A small buffer size so the response content is + // read in multiple buffers, but big enough for HTTP/3. + int bufferSize = 1536; byte[] content = new byte[10 * bufferSize]; new Random().nextBytes(content); scenario.startServer(new EmptyServerHandler() @@ -140,7 +142,6 @@ public class HttpClientDemandTest extends AbstractTest }); scenario.startClient(client -> { - // A small buffer size so the response content is read in multiple buffers. client.setByteBufferPool(new MappedByteBufferPool(bufferSize)); client.setResponseBufferSize(bufferSize); }); @@ -287,7 +288,7 @@ public class HttpClientDemandTest extends AbstractTest { init(transport); - int bufferSize = 512; + int bufferSize = 1536; byte[] content = new byte[10 * bufferSize]; new Random().nextBytes(content); scenario.startServer(new EmptyServerHandler() diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java index ef6465af05a..95d604dd8f3 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java @@ -31,18 +31,17 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.HttpClientTransport; import org.eclipse.jetty.client.LeakTrackingConnectionPool; +import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.api.Result; -import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; import org.eclipse.jetty.client.util.BytesRequestContent; -import org.eclipse.jetty.fcgi.client.http.HttpClientTransportOverFCGI; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.http3.server.HTTP3ServerConnector; import org.eclipse.jetty.io.ArrayByteBufferPool; import org.eclipse.jetty.io.ByteBufferPool; -import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.io.LeakTrackingByteBufferPool; import org.eclipse.jetty.io.MappedByteBufferPool; import org.eclipse.jetty.server.Connector; @@ -368,58 +367,54 @@ public class HttpClientLoadTest extends AbstractTest new LeakTrackingConnectionPool(destination, client.getMaxConnectionsPerDestination(), destination) { @Override - protected void leaked(LeakDetector.LeakInfo leakInfo) + protected void leaked(LeakDetector.LeakInfo leakInfo) { super.leaked(leakInfo); connectionLeaks.incrementAndGet(); } }); - return clientTransport; - } - case FCGI: - { - HttpClientTransport clientTransport = new HttpClientTransportOverFCGI(1, ""); - clientTransport.setConnectionPoolFactory(destination -> new LeakTrackingConnectionPool(destination, client.getMaxConnectionsPerDestination(), destination) - { - @Override - protected void leaked(LeakDetector.LeakInfo leakInfo) - { - super.leaked(leakInfo); - connectionLeaks.incrementAndGet(); - } - }); - return clientTransport; + break; } default: { - return super.provideClientTransport(transport, sslContextFactory); + break; } } + return clientTransport; } } } diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java index af66f5c1780..8740a8c39c6 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientStreamTest.java @@ -58,6 +58,7 @@ import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; @@ -890,6 +891,9 @@ public class HttpClientStreamTest extends AbstractTest @ArgumentsSource(TransportProvider.class) public void testUploadWithOutputStreamFailureToConnect(Transport transport) throws Exception { + // Failure to connect is based on InetSocket address failure, which Unix-Domain does not use. + Assumptions.assumeTrue(transport != Transport.UNIX_DOMAIN); + init(transport); long connectTimeout = 1000; @@ -972,6 +976,9 @@ public class HttpClientStreamTest extends AbstractTest @ArgumentsSource(TransportProvider.class) public void testUploadWithConnectFailureClosesStream(Transport transport) throws Exception { + // Failure to connect is based on InetSocket address failure, which Unix-Domain does not use. + Assumptions.assumeTrue(transport != Transport.UNIX_DOMAIN); + init(transport); long connectTimeout = 1000;