From 9421f7d3d4e90ffa1049e0c410044ba90d89052e Mon Sep 17 00:00:00 2001
From: kkolinko <kkolinko@13f79535-47bb-0310-9956-ffa450edef68>
Date: Mon, 22 Nov 2010 22:43:51 +0000
Subject: [PATCH] Impose a limit on the total length of the trailing headers.
 Otherwise the ByteChunk buffer in the ChunkedInputFilter might grow
 unlimitedly.

Implemented as a system property. It might be better to implement it as an attribute of a connector (like maxPostSize and maxSavePostSize attributes are), but I am not sure that it is worth the effort.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1037924 13f79535-47bb-0310-9956-ffa450edef68
---
 java/org/apache/coyote/Constants.java              |  8 ++++
 .../coyote/http11/filters/ChunkedInputFilter.java  |  9 ++++-
 .../http11/filters/TestChunkedInputFilter.java     | 47 +++++++++++++++++++++-
 webapps/docs/changelog.xml                         |  5 +++
 webapps/docs/config/systemprops.xml                |  7 ++++
 5 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/coyote/Constants.java b/java/org/apache/coyote/Constants.java
index 1d20ea27f..a610e3bc2 100644
--- a/java/org/apache/coyote/Constants.java
+++ b/java/org/apache/coyote/Constants.java
@@ -69,4 +69,12 @@ public final class Constants {
                 "org.apache.coyote.USE_CUSTOM_STATUS_MSG_IN_HEADER",
                 "false")).booleanValue(); 
 
+    /**
+     * Limit on the total length of the trailer headers in
+     * a chunked HTTP request.
+     */
+    public static final int MAX_TRAILER_SIZE =
+        Integer.parseInt(System.getProperty(
+                "org.apache.coyote.MAX_TRAILER_SIZE",
+                "8192"));
 }
diff --git a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
index e9e83c1f6..ab91acf06 100644
--- a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
+++ b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
@@ -102,7 +102,14 @@ public class ChunkedInputFilter implements InputFilter {
     /**
      * Byte chunk used to store trailing headers.
      */
-    protected ByteChunk trailingHeaders = new ByteChunk();
+    protected ByteChunk trailingHeaders;
+
+    {
+        trailingHeaders = new ByteChunk();
+        if (org.apache.coyote.Constants.MAX_TRAILER_SIZE > 0) {
+            trailingHeaders.setLimit(org.apache.coyote.Constants.MAX_TRAILER_SIZE);
+        }
+    }
 
 
     /**
diff --git a/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java b/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
index cd40d5cc2..265d7f54a 100644
--- a/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
+++ b/test/org/apache/coyote/http11/filters/TestChunkedInputFilter.java
@@ -70,7 +70,52 @@ public class TestChunkedInputFilter extends TomcatBaseTest {
         client.processRequest();
         assertEquals("null7TestTestTest0123456789abcdefghijABCDEFGHIJopqrstuvwxyz", client.getResponseBody());
     }
-    
+
+    public void testTrailingHeadersSizeLimit() throws Exception {
+        // Setup Tomcat instance
+        Tomcat tomcat = getTomcatInstance();
+
+        // Must have a real docBase - just use temp
+        Context ctx = 
+            tomcat.addContext("", System.getProperty("java.io.tmpdir"));
+
+        Tomcat.addServlet(ctx, "servlet", new EchoHeaderServlet());
+        ctx.addServletMapping("/", "servlet");
+
+        tomcat.start();
+
+        StringBuilder longText = new StringBuilder("Test1234567890");
+        while (longText.length() <= 8192) {
+            longText.append(longText.toString());
+        }
+
+        String[] request = new String[]{
+            "POST /echo-params.jsp 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 +
+            "3" + SimpleHttpClient.CRLF +
+            "a=0" + SimpleHttpClient.CRLF +
+            "4" + SimpleHttpClient.CRLF +
+            "&b=1" + SimpleHttpClient.CRLF +
+            "0" + SimpleHttpClient.CRLF +
+            "x-trailer: Test" + longText + SimpleHttpClient.CRLF +
+            SimpleHttpClient.CRLF };
+
+        TrailerClient client = new TrailerClient();
+        client.setPort(getPort());
+        client.setRequest(request);
+
+        client.connect();
+        client.processRequest();
+        // Expected to fail because the trailers are longer
+        // than the default limit of 8Kb
+        assertTrue(client.isResponse500());
+    }
+
     public void testNoTrailingHeaders() throws Exception {
         // Setup Tomcat instance
         Tomcat tomcat = getTomcatInstance();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d9f802f15..5d70d273b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -167,6 +167,11 @@
         <bug>49860</bug>: Complete support for handling trailing headers in
         chunked HTTP requests. (markt)
       </fix>
+      <add>
+        Impose a limit on the length of the trailing headers. The limit
+        is configurable with a system property and is <code>8192</code>
+        by default. (kkolinko)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Jasper">
diff --git a/webapps/docs/config/systemprops.xml b/webapps/docs/config/systemprops.xml
index f2dc78eca..56c8c1cb6 100644
--- a/webapps/docs/config/systemprops.xml
+++ b/webapps/docs/config/systemprops.xml
@@ -474,6 +474,13 @@
       <p>If not specified, the default value of <code>false</code> will be used.</p>
     </property>
 
+    <property name="org.apache.coyote.MAX_TRAILER_SIZE">
+      <p>Limits the total length of trailing headers in the last chunk of
+      a chunked HTTP request.
+      If the value is <code>-1</code>, no limit will be imposed.</p>
+      <p>If not specified, the default value of <code>8192</code> will be used.</p>
+    </property>
+
     <property name="catalina.useNaming">
       <p>If this is <code>false</code> it will override the
       <code>useNaming</code> attribute for all <a href="context.html">
-- 
2.11.0