[tomcat] branch 9.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 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 51291448af Handle servlet responding to a request with an expectation without ack or reading request body. (#550)
51291448af is described below

commit 51291448af5dbc0c6f717666d1632c71ba11c420
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 4e9a8b3998..83eea9df33 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);
@@ -1239,7 +1238,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 aaad9816a9..0c7ac848c7 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 8e873472e8..599c6f50a6 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