From 8259eabbc70ae7fc2d525f1e95b43fbdfd2ad097 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 14 Aug 2024 01:55:17 +0300 Subject: [PATCH] Jetty 10: Improvements to HttpConnection when reading 0 bytes. (#12156) * Improvements to HttpConnection when reading 0 bytes. --------- Signed-off-by: Simone Bordet --- .../eclipse/jetty/server/HttpConnection.java | 19 ++++++-- .../http/client/HttpClientContinueTest.java | 45 +++++++++++++++++++ .../jetty/http/client/TransportScenario.java | 5 ++- 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java index 5b971525fe82..908d8a0a0a4f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java @@ -375,20 +375,31 @@ private int fillRequestBuffer() filled = getEndPoint().fill(requestBuffer); if (filled > 0) + { bytesIn.add(filled); - else if (filled < 0) - _parser.atEOF(); + } + else + { + if (filled < 0) + _parser.atEOF(); + releaseRequestBuffer(); + } if (LOG.isDebugEnabled()) LOG.debug("{} filled {} {}", this, filled, _retainableByteBuffer); return filled; } - catch (IOException e) + catch (Throwable x) { if (LOG.isDebugEnabled()) - LOG.debug("Unable to fill from endpoint {}", getEndPoint(), e); + LOG.debug("Unable to fill from endpoint {}", getEndPoint(), x); _parser.atEOF(); + if (_retainableByteBuffer != null) + { + _retainableByteBuffer.clear(); + releaseRequestBuffer(); + } return -1; } } diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java index 680535f6b5bf..00d935c7a40b 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java @@ -45,14 +45,21 @@ import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.io.ArrayRetainableByteBufferPool; +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.RetainableByteBufferPool; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.IO; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; import static org.awaitility.Awaitility.await; import static org.eclipse.jetty.http.client.Transport.FCGI; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -63,6 +70,22 @@ public class HttpClientContinueTest extends AbstractTest { + @AfterEach + public void dispose() + { + if (scenario == null || scenario.connector == null) + return; + ByteBufferPool bbp = scenario.connector.getByteBufferPool(); + if (bbp == null) + return; + RetainableByteBufferPool rbbp = bbp.asRetainableByteBufferPool(); + if (rbbp instanceof ArrayRetainableByteBufferPool.Tracking) + { + ArrayRetainableByteBufferPool.Tracking tracking = (ArrayRetainableByteBufferPool.Tracking)rbbp; + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server leaks: " + tracking.dumpLeaks(), tracking.getLeaks().size(), is(0))); + } + } + @Override public void init(Transport transport) throws IOException { @@ -852,6 +875,28 @@ public void testNoExpect100ContinueThenRedirectThen100ContinueThenResponse(Trans } } + @ParameterizedTest + @ArgumentsSource(TransportProvider.class) + public void testExpect100ContinueThen404(Transport transport) throws Exception + { + init(transport); + + // No Handler so every request is a 404. + scenario.start((Handler)null); + + for (int i = 0; i < 100; ++i) + { + CountDownLatch latch = new CountDownLatch(1); + AsyncRequestContent requestContent = new AsyncRequestContent("text-plain"); + scenario.client.newRequest(scenario.newURI()) + .headers(headers -> headers.put(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE)) + .body(requestContent) + .send(result -> latch.countDown()); + + assertTrue(latch.await(5, TimeUnit.SECONDS)); + } + } + private void readRequestHeaders(InputStream input) throws IOException { int crlfs = 0; diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java index 51a9eaf4d54b..941cfe568d9c 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java @@ -43,6 +43,8 @@ import org.eclipse.jetty.http3.server.AbstractHTTP3ServerConnectionFactory; import org.eclipse.jetty.http3.server.HTTP3ServerConnectionFactory; 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.jmx.MBeanContainer; import org.eclipse.jetty.quic.server.QuicServerConnector; @@ -122,7 +124,8 @@ public Connector newServerConnector(Server server) case H2C: case H2: case FCGI: - return new ServerConnector(server, 1, 1, provideServerConnectionFactory(transport)); + ByteBufferPool bufferPool = new ArrayByteBufferPool.Tracking(); + return new ServerConnector(server, null, null, bufferPool, 1, 1, provideServerConnectionFactory(transport)); case H3: HTTP3ServerConnector http3ServerConnector = new HTTP3ServerConnector(server, sslContextFactory, provideServerConnectionFactory(transport)); http3ServerConnector.getQuicConfiguration().setPemWorkDirectory(Path.of(System.getProperty("java.io.tmpdir")));