[tomcat] branch 10.0.x updated: Handle servlet responding to a request with an expectation without ack or reading request body. (#550)

[ma...@apache.org]

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

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


The following commit(s) were added to refs/heads/10.0.x by this push:
     new 76b8ecff1e Handle servlet responding to a request with an expectation without ack or reading request body. (#550)
76b8ecff1e is described below

commit 76b8ecff1ebd6e3f4d407d7b1fb6f7e551c50a06
Author: Malay Shah <ma...@gmail.com>
AuthorDate: Mon Sep 12 08:42:03 2022 -0700

    Handle servlet responding to a request with an expectation without ack or reading request body. (#550)
    
    * Fixing a bug where a connection would be kept alive without swallowing the input from the request stream.
    
    When a request comes in with a 100 Continue expectation, and the server immediately responds with a 200 status without reading the request body at all, the next request on the connection will fail because Tomcat cannot parse the HTTP verb.
    
    The new unit test replicates the issue and is addressed by removing the unnecessary changes to the swallowInput field of the Http11InputBuffer.
---
 java/org/apache/coyote/http11/Http11Processor.java |  2 -
 .../apache/coyote/http11/TestHttp11Processor.java  | 54 ++++++++++++++++++++++
 webapps/docs/changelog.xml                         |  5 ++
 3 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java
index 8fdbe7a55f..4d61a1554c 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -802,7 +802,6 @@ public class Http11Processor extends AbstractProcessor {
         MessageBytes expectMB = headers.getValue("expect");
         if (expectMB != null && !expectMB.isNull()) {
             if (expectMB.toString().trim().equalsIgnoreCase("100-continue")) {
-                inputBuffer.setSwallowInput(false);
                 request.setExpectation(true);
             } else {
                 response.setStatus(HttpServletResponse.SC_EXPECTATION_FAILED);
@@ -1233,7 +1232,6 @@ public class Http11Processor extends AbstractProcessor {
             // Send a 100 status back if it makes sense (response not committed
             // yet, and client specified an expectation for 100-continue)
             if (!response.isCommitted() && request.hasExpectation()) {
-                inputBuffer.setSwallowInput(true);
                 try {
                     outputBuffer.sendAck();
                 } catch (IOException e) {
diff --git a/test/org/apache/coyote/http11/TestHttp11Processor.java b/test/org/apache/coyote/http11/TestHttp11Processor.java
index cad207b7ab..60afa56b2f 100644
--- a/test/org/apache/coyote/http11/TestHttp11Processor.java
+++ b/test/org/apache/coyote/http11/TestHttp11Processor.java
@@ -1942,4 +1942,58 @@ public class TestHttp11Processor extends TomcatBaseTest {
         Assert.assertTrue(client.isResponse200());
         Assert.assertTrue(client.getResponseBody().contains("test - data"));
     }
+
+
+    @Test
+    public void test100ContinueWithNoAck() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        final Connector connector = tomcat.getConnector();
+        connector.setProperty("continueResponseTiming", "onRead");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TestPostNoReadServlet", new TestPostNoReadServlet());
+        ctx.addServletMappingDecoded("/foo", "TestPostNoReadServlet");
+
+        tomcat.start();
+
+        String request =
+            "POST /foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                "Host: localhost:" + getPort() + SimpleHttpClient.CRLF +
+                "Expect: 100-continue"  + SimpleHttpClient.CRLF +
+                "Content-Length: 10"  + SimpleHttpClient.CRLF +
+                SimpleHttpClient.CRLF +
+                "0123456789";
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest(false);
+
+        Assert.assertTrue(client.isResponse200());
+
+        if (client.getResponseHeaders().contains("Connection: close")) {
+            client.connect();
+        }
+
+        client.processRequest(false);
+
+        Assert.assertTrue(client.isResponse200());
+
+    }
+
+
+    private static class TestPostNoReadServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+
+        }
+    }
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 713dbe3f94..1020995712 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -213,6 +213,11 @@
         the maximum value to 255. Based on a PR <pr>548</pr> by Stefan Mayr.
         (lihan)
       </fix>
+      <fix>
+        <pr>550</pr>: Correctly handle case where a Servlet responds to a
+        request with an expectation with a 2xx response without reading the
+        request body. Pull request provided by Malay Shah. (markt)
+      </fix>
       <fix>
         <pr>551</pr>: Avoid potential IndexOutOfBoundsException by fixing
         incorrect check when matching HTTP/2 preface. Submitted by 刘文章.


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