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 <simone.bordet@gmail.com>
(cherry picked from commit aed20abcbe)

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

@@ -263,9 +263,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
             {
                 // 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 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
                 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.List;
 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,6 +42,7 @@ import javax.servlet.http.HttpServletResponse;
 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;
@@ -47,6 +50,7 @@ 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 org.slf4j.LoggerFactory;
 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 class HttpConnectionTest
         }
     }
 
+    @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 class GzipWithSendErrorTest
         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 class GzipWithSendErrorTest
         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));