From aed20abcbe157d06a7e70e0550586222b2aa433c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 8 Apr 2021 12:19:17 +0200 Subject: [PATCH] =?UTF-8?q?Fixes=20#6105=20-=20HttpConnection.getBytesIn()?= =?UTF-8?q?=20incorrect=20for=20requests=20with=E2=80=A6=20(#6108)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fixes #6105 - HttpConnection.getBytesIn() incorrect for requests with chunked content Moved recording of bytes to fillRequestBuffer(), so they are accounted also for async reads. Added test case. Fixed test that was too strictly comparing HttpConnection.bytesIn, that now report a correct, but larger value. Signed-off-by: Simone Bordet --- .../eclipse/jetty/server/HttpConnection.java | 9 ++- .../jetty/server/HttpConnectionTest.java | 68 +++++++++++++++++++ .../jetty/test/GzipWithSendErrorTest.java | 6 +- 3 files changed, 76 insertions(+), 7 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 aa1acf5f7dfe..0bcd46e5b9c5 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 @@ -258,9 +258,7 @@ public void onFillable() { // Fill the request buffer (if needed). int filled = fillRequestBuffer(); - if (filled > 0) - bytesIn.add(filled); - else if (filled == -1 && getEndPoint().isOutputShutdown()) + if (filled < 0 && getEndPoint().isOutputShutdown()) close(); // Parse the request buffer. @@ -343,8 +341,9 @@ private int fillRequestBuffer() if (filled == 0) // Do a retry on fill 0 (optimization for SSL connections) filled = getEndPoint().fill(_requestBuffer); - // tell parser - if (filled < 0) + if (filled > 0) + bytesIn.add(filled); + else if (filled < 0) _parser.atEOF(); if (LOG.isDebugEnabled()) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java index 72197e6867a3..de3b00f811fc 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java @@ -37,6 +37,8 @@ import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import java.util.stream.Stream; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -45,12 +47,14 @@ import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpParser; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.server.LocalConnector.LocalEndPoint; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.log.StacklessLogging; @@ -65,9 +69,11 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; public class HttpConnectionTest @@ -1361,6 +1367,68 @@ public void testCONNECT() throws Exception } } + @Test + public void testBytesIn() throws Exception + { + String chunk1 = "0123456789ABCDEF"; + String chunk2 = IntStream.range(0, 64).mapToObj(i -> chunk1).collect(Collectors.joining()); + long dataLength = chunk1.length() + chunk2.length(); + server.stop(); + server.setHandler(new AbstractHandler() + { + @Override + public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + jettyRequest.setHandled(true); + IO.copy(request.getInputStream(), IO.getNullStream()); + + HttpConnection connection = HttpConnection.getCurrentConnection(); + long bytesIn = connection.getBytesIn(); + assertThat(bytesIn, greaterThan(dataLength)); + } + }); + server.start(); + + LocalEndPoint localEndPoint = connector.executeRequest("" + + "POST / HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Content-Length: " + dataLength + "\r\n" + + "\r\n" + + chunk1); + + // Wait for the server to block on the read(). + Thread.sleep(500); + + // Send more content. + localEndPoint.addInput(chunk2); + + HttpTester.Response response = HttpTester.parseResponse(localEndPoint.getResponse()); + assertEquals(response.getStatus(), HttpStatus.OK_200); + localEndPoint.close(); + + localEndPoint = connector.executeRequest("" + + "POST / HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Transfer-Encoding: chunked\r\n" + + "\r\n" + + Integer.toHexString(chunk1.length()) + "\r\n" + + chunk1 + "\r\n"); + + // Wait for the server to block on the read(). + Thread.sleep(500); + + // Send more content. + localEndPoint.addInput("" + + Integer.toHexString(chunk2.length()) + "\r\n" + + chunk2 + "\r\n" + + "0\r\n" + + "\r\n"); + + response = HttpTester.parseResponse(localEndPoint.getResponse()); + assertEquals(response.getStatus(), HttpStatus.OK_200); + localEndPoint.close(); + } + private int checkContains(String s, int offset, String c) { assertThat(s.substring(offset), Matchers.containsString(c)); diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/GzipWithSendErrorTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/GzipWithSendErrorTest.java index 551bc12a240c..8614ba85bf0d 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/GzipWithSendErrorTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/GzipWithSendErrorTest.java @@ -276,7 +276,8 @@ public long getLength() assertThat("Request Input Content Received should have seen content", inputContentReceived.get(), greaterThan(0L)); assertThat("Request Input Content Received less then initial buffer", inputContentReceived.get(), lessThanOrEqualTo((long)sizeActuallySent)); assertThat("Request Connection BytesIn should have some minimal data", inputBytesIn.get(), greaterThanOrEqualTo(1024L)); - assertThat("Request Connection BytesIn read should not have read all of the data", inputBytesIn.get(), lessThanOrEqualTo((long)sizeActuallySent)); + long requestBytesSent = sizeActuallySent + 512; // Take into account headers and chunked metadata. + assertThat("Request Connection BytesIn read should not have read all of the data", inputBytesIn.get(), lessThanOrEqualTo(requestBytesSent)); // Now use the deferred content to complete writing of the request body content contentProvider.offer(ByteBuffer.wrap(compressedRequest, sizeActuallySent, compressedRequest.length - sizeActuallySent)); @@ -395,7 +396,8 @@ public void onComplete(Request request) assertThat("Request Input Content Received should have seen content", inputContentReceived.get(), greaterThan(0L)); assertThat("Request Input Content Received less then initial buffer", inputContentReceived.get(), lessThanOrEqualTo((long)sizeActuallySent)); assertThat("Request Connection BytesIn should have some minimal data", inputBytesIn.get(), greaterThanOrEqualTo(1024L)); - assertThat("Request Connection BytesIn read should not have read all of the data", inputBytesIn.get(), lessThanOrEqualTo((long)sizeActuallySent)); + long requestBytesSent = sizeActuallySent + 512; // Take into account headers and chunked metadata. + assertThat("Request Connection BytesIn read should not have read all of the data", inputBytesIn.get(), lessThanOrEqualTo(requestBytesSent)); // Now use the deferred content to complete writing of the request body content contentProvider.offer(ByteBuffer.wrap(compressedRequest, sizeActuallySent, compressedRequest.length - sizeActuallySent));