You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2024/08/14 09:54:55 UTC

(tomcat) branch 10.1.x updated (abcb2b1faf -> adc2a3bbe9)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


    from abcb2b1faf Add test case for null property
     new a6d6e3dd7e Fix typo
     new adc2a3bbe9 Fix regression in reading chunked request bodies

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../coyote/http11/filters/ChunkedInputFilter.java  |  10 ++
 .../http11/filters/TestChunkedInputFilter.java     | 115 +++++++++++++++++++--
 webapps/docs/changelog.xml                         |  13 ++-
 3 files changed, 131 insertions(+), 7 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


(tomcat) 02/02: Fix regression in reading chunked request bodies

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit adc2a3bbe9b56d66048da65d16c842da17d79bdf
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 b264092ed0..5bb587d620 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 jakarta.servlet.ServletException;
 import jakarta.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 01bececc19..f8d8d478e9 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 10.1.29 (schultz)" 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


(tomcat) 01/02: Fix typo

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit a6d6e3dd7ef0aa624b2427d79a58396ecdaba5e8
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Aug 13 11:48:43 2024 +0100

    Fix typo
---
 webapps/docs/changelog.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 5ba613df47..01bececc19 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -960,7 +960,7 @@
         Make asynchronous error handling more robust. Ensure that once the call
         to <code>AsyncListener.onError()</code> has returned to the container,
         only container threads can access the <code>AsyncContext</code>. This
-        protects against various race conditions that woudl otherwise occur if
+        protects against various race conditions that would otherwise occur if
         application threads continued to access the <code>AsyncContext</code>.
       </fix>
       <fix>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org