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 <[email protected]>
(cherry picked from commit aed20ab)

jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java

@@ -263,9 +263,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.
@@ -352,8 +350,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())

jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java

@@ -32,6 +32,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;
@@ -40,13 +42,15 @@
 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.logging.StacklessLogging;
 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.hamcrest.Matchers;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -59,9 +63,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
@@ -1353,6 +1359,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));

tests/test-integration/src/test/java/org/eclipse/jetty/test/GzipWithSendErrorTest.java

@@ -251,7 +251,8 @@ public long getLength()
         assertThat("Request Input Content Received", inputContentReceived.get(), is(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 provide rest
         content.offer(ByteBuffer.wrap(compressedRequest, sizeActuallySent, compressedRequest.length - sizeActuallySent));
@@ -351,7 +352,8 @@ public void onComplete(Request request)
         assertThat("Request Input Content Received", inputContentReceived.get(), read ? greaterThan(0L) : is(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 provide rest
         content.offer(ByteBuffer.wrap(compressedRequest, sizeActuallySent, compressedRequest.length - sizeActuallySent));