This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 1d0e79ad72 Fix regression in reading chunked request bodies
1d0e79ad72 is described below
commit 1d0e79ad723ec181a3650c5d9c9ad9dd62178333
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Aug 14 10:53:04 2024 +0100
Fix regression in reading chunked request bodies
available() was returning a non-zero value when there was no data to
read. In some circumstances this could cause a blocking read to block
waiting for more data rather than return the data it had already
received.
---
.../coyote/http11/filters/ChunkedInputFilter.java | 10 ++
.../http11/filters/TestChunkedInputFilter.java | 115 +++++++++++++++++++--
webapps/docs/changelog.xml | 11 ++
3 files changed, 130 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
index 36389576d4..a967ef65a8 100644
--- a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
+++ b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
@@ -198,6 +198,16 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
if (readChunk != null) {
available = readChunk.remaining();
}
+
+ // Handle some edge cases
+ if (available == 1 && parseState == ParseState.CHUNK_BODY_CRLF) {
+ // Either just the CR or just the LF are left in the buffer.
There is no data to read.
+ available = 0;
+ } else if (available == 2 && !crFound && parseState ==
ParseState.CHUNK_BODY_CRLF) {
+ // Just CRLF is left in the buffer. There is no data to read.
+ available = 0;
+ }
+
if (available == 0) {
// No data buffered here. Try the next filter in the chain.
return buffer.available();
diff --git a/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
b/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
index 9a0d41eff6..6f9e906080 100644
--- a/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
+++ b/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
@@ -16,9 +16,13 @@
*/
package org.apache.coyote.http11.filters;
+import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
+import java.io.InputStreamReader;
import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.TimeUnit;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
@@ -36,7 +40,6 @@ import org.apache.catalina.startup.TomcatBaseTest;
public class TestChunkedInputFilter extends TomcatBaseTest {
- private static final String LF = "\n";
private static final int EXT_SIZE_LIMIT = 10;
@Test
@@ -117,16 +120,16 @@ public class TestChunkedInputFilter extends
TomcatBaseTest {
SimpleHttpClient.CRLF +
"Connection: close" + SimpleHttpClient.CRLF +
SimpleHttpClient.CRLF +
- "3" + (chunkHeaderUsesCRLF ? SimpleHttpClient.CRLF : LF) +
- "a=0" + (chunkUsesCRLF ? SimpleHttpClient.CRLF : LF) +
+ "3" + (chunkHeaderUsesCRLF ? SimpleHttpClient.CRLF :
SimpleHttpClient.LF) +
+ "a=0" + (chunkUsesCRLF ? SimpleHttpClient.CRLF :
SimpleHttpClient.LF) +
"4" + SimpleHttpClient.CRLF +
"&b=1" + SimpleHttpClient.CRLF +
"0" + SimpleHttpClient.CRLF +
"x-trailer1: Test", "Value1" +
- (firstheaderUsesCRLF ? SimpleHttpClient.CRLF : LF) +
+ (firstheaderUsesCRLF ? SimpleHttpClient.CRLF :
SimpleHttpClient.LF) +
"x-trailer2: TestValue2" +
- (secondheaderUsesCRLF ? SimpleHttpClient.CRLF : LF) +
- (endUsesCRLF ? SimpleHttpClient.CRLF : LF) };
+ (secondheaderUsesCRLF ? SimpleHttpClient.CRLF :
SimpleHttpClient.LF) +
+ (endUsesCRLF ? SimpleHttpClient.CRLF : SimpleHttpClient.LF) };
TrailerClient client =
new TrailerClient(tomcat.getConnector().getLocalPort());
@@ -777,4 +780,104 @@ public class TestChunkedInputFilter extends
TomcatBaseTest {
Assert.assertNull(client.getResponseLine());
}
}
+
+
+ private static class BodyReadLineServlet extends HttpServlet {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doPost(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+
+ int lineCount = 0;
+ int pauseCount = 0;
+
+ // Read the body one line at a time. There should be ~1s between
reads.
+ try (InputStream is = req.getInputStream();
+ InputStreamReader isr = new InputStreamReader(is,
StandardCharsets.UTF_8);
+ BufferedReader br = new BufferedReader(isr)) {
+ long lastRead = 0;
+ while (br.readLine() != null) {
+ long thisRead = System.nanoTime();
+ if (lineCount > 0) {
+ /*
+ * After the first line, look for a pause of at least
800ms between reads.
+ */
+ if ((thisRead - lastRead) >
TimeUnit.MILLISECONDS.toNanos(800)) {
+ pauseCount++;
+ }
+ }
+ lastRead = thisRead;
+ lineCount++;
+ }
+ }
+
+ resp.setContentType("text/plain");
+ PrintWriter pw = resp.getWriter();
+ pw.write(Integer.toString(lineCount) + "," +
Integer.toString(pauseCount));
+ }
+ }
+
+
+ private static class ReadLineClient extends SimpleHttpClient {
+
+ ReadLineClient(int port) {
+ setPort(port);
+ }
+
+ @Override
+ public boolean isResponseBodyOK() {
+ return getResponseBody().equals("5");
+ }
+ }
+
+
+ @Test
+ public void testChunkedSplitWithReader() throws Exception {
+ // Setup Tomcat instance
+ Tomcat tomcat = getTomcatInstance();
+
+ // No file system docBase required
+ Context ctx = getProgrammaticRootContext();
+
+ BodyReadLineServlet servlet = new BodyReadLineServlet();
+ Tomcat.addServlet(ctx, "servlet", servlet);
+ ctx.addServletMappingDecoded("/test", "servlet");
+
+ tomcat.getConnector().setProperty("connectionTimeout", "300000");
+ tomcat.start();
+
+ String[] request = new String[]{
+ "POST /test HTTP/1.1" + SimpleHttpClient.CRLF +
+ "Host: any" + SimpleHttpClient.CRLF +
+ "Transfer-encoding: chunked" + SimpleHttpClient.CRLF +
+ "Content-Type: application/x-www-form-urlencoded" +
SimpleHttpClient.CRLF +
+ "Connection: close" + SimpleHttpClient.CRLF +
+ SimpleHttpClient.CRLF +
+ "7" + SimpleHttpClient.CRLF +
+ "DATA01\n" + SimpleHttpClient.CRLF,
+ "7" + SimpleHttpClient.CRLF +
+ "DATA02\n" + SimpleHttpClient.CRLF,
+ "7" + SimpleHttpClient.CRLF +
+ // Split the CRLF between writes
+ "DATA03\n" + SimpleHttpClient.CR,
+ SimpleHttpClient.LF +
+ "7" + SimpleHttpClient.CRLF +
+ "DATA04\n" + SimpleHttpClient.CRLF,
+ "7" + SimpleHttpClient.CRLF +
+ "DATA05\n" + SimpleHttpClient.CRLF +
+ "0" + SimpleHttpClient.CRLF +
+ SimpleHttpClient.CRLF};
+
+ ReadLineClient client = new
ReadLineClient(tomcat.getConnector().getLocalPort());
+ client.setRequest(request);
+
+ client.connect(300000,300000);
+ client.processRequest();
+ Assert.assertTrue(client.isResponse200());
+ /*
+ * Output is "<lines read>,<pauses observer>" so there should be 5
lines read with a pause between each.
+ */
+ Assert.assertEquals("5,4", client.getResponseBody());
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 85e6ab0c8c..2f3f85194b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,17 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 9.0.94 (remm)" rtext="in development">
+ <subsection name="Coyote">
+ <changelog>
+ <fix>
+ Correct a regression in the fix for non-blocking reads of chunked
+ request bodies that caused <code>InputStream.available()</code> to
+ return a non-zero value when there was no data to read. In some
+ circumstances this could cause a blocking read to block waiting for
more
+ data rather than return the data it had already received. (markt)
+ </fix>
+ </changelog>
+ </subsection>
<subsection name="Jasper">
<changelog>
<fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
