Skip to content

Commit

Permalink
Fixes #6105 - HttpConnection.getBytesIn() incorrect for requests with…
Browse files Browse the repository at this point in the history
… chunked content

Moved recording of bytes to fillRequestBuffer(),
so they are accounted also for async reads.
Added test case.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
sbordet committed Mar 26, 2021
1 parent d239258 commit 5f0bd97
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 5 deletions.
Expand Up @@ -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.
Expand Down Expand Up @@ -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())
Expand Down
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 5f0bd97

Please sign in to comment.