Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #6105 - HttpConnection.getBytesIn() incorrect for requests with chunked content #6145

Merged
merged 1 commit into from Apr 10, 2021
Merged

Fixes #6105 - HttpConnection.getBytesIn() incorrect for requests with chunked content #6145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
View file
Open in desktop
Expand Up @@ -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.
Expand Down Expand Up @@ -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())
Expand Down
68 changes: 68 additions & 0 deletions jetty-server/src/test/java/org/eclipse/jetty/server/HttpConnectionTest.java
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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));
Expand Down
6 changes: 4 additions & 2 deletions tests/test-integration/src/test/java/org/eclipse/jetty/test/GzipWithSendErrorTest.java
View file
Open in desktop
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down