[tomcat] branch 8.5.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 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


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

commit 5c657011c81cfeb0bcfd4045ab51fe66aae1bcf7
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 1392bf56ff..d7d23fe58a 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -1042,7 +1042,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);
@@ -1474,7 +1473,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 ca7db29d54..2628dba2b1 100644
--- a/test/org/apache/coyote/http11/TestHttp11Processor.java
+++ b/test/org/apache/coyote/http11/TestHttp11Processor.java
@@ -1958,4 +1958,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 04f9d6f844..76766752dc 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -172,6 +172,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