Merge pull request #16 from cesarhernandezgt/tomcat-7.0.104-TT-patch

Added test for invalid Content-Length header with  rejectIllegalHeader config

.github/workflows/main.yml

@@ -41,7 +41,7 @@ jobs:
       run: unzip -qq tomcat6-build-libs-.zip -d /home/runner/
 
     - name: replace java.7.home
-      run: sed -i.bak 's:.*java.7.home.*:java.7.home=/opt/hostedtoolcache/jdk/7.0.342/x64:' build.properties.default
+      run: sed -i.bak 's:.*java.7.home.*:java.7.home=/opt/hostedtoolcache/jdk/7.0.352/x64:' build.properties.default
 
     - name: ant_build
       run: ant

build.properties.default

@@ -27,7 +27,7 @@ version.major=7
 version.minor=0
 version.build=104
 version.patch=0
-version.suffix=-SP.5
+version.suffix=-SP.6
 
 # ----- Source control flags -----
 git.branch=7.0.x

java/org/apache/coyote/http11/InternalInputBuffer.java

@@ -369,7 +369,7 @@ private boolean parseHeader() throws IOException {
                 // Non-token characters are illegal in header names
                 // Parsing continues so the error can be reported in context
                 // skipLine() will handle the error
-                skipLine(lineStart, start);
+                skipLine(lineStart, start, false);
                 return true;
             }
 
@@ -432,16 +432,12 @@ private boolean parseHeader() throws IOException {
                 } else if (prevChr == Constants.CR && chr == Constants.LF) {
                     eol = true;
                 } else if (prevChr == Constants.CR) {
-                    // Invalid value
-                    // Delete the header (it will be the most recent one)
-                    headers.removeHeader(headers.size() - 1);
-                    skipLine(lineStart, start);
+                    // Invalid value - also need to delete header
+                    skipLine(lineStart, start, true);
                     return true;
                 } else if (chr != Constants.HT && HttpParser.isControl(chr)) {
-                    // Invalid value
-                    // Delete the header (it will be the most recent one)
-                    headers.removeHeader(headers.size() - 1);
-                    skipLine(lineStart, start);
+                    // Invalid value - also need to delete header
+                    skipLine(lineStart, start, true);
                     return true;
                 } else if (chr == Constants.SP) {
                     buf[realPos] = chr;
@@ -506,7 +502,28 @@ protected void init(SocketWrapper<Socket> socketWrapper,
 
 
 
-    private void skipLine(int lineStart, int start) throws IOException {
+    private void skipLine(int lineStart, int start, boolean deleteHeader) throws IOException {
+
+        boolean rejectThisHeader = rejectIllegalHeaderName;
+        // Check if rejectIllegalHeader is disabled and needs to be overridden
+        // for this header. The header name is required to determine if this
+        // override is required. The header name is only available once the
+        // header has been created. If the header has been created then
+        // deleteHeader will be true.
+        if (!rejectThisHeader && deleteHeader) {
+            if (headers.getName(headers.size() - 1).equalsIgnoreCase("content-length")) {
+                // Malformed content-length headers must always be rejected
+                // RFC 9112, section 6.3, bullet 5.
+                rejectThisHeader = true;
+            } else {
+                // Only need to delete the header if the request isn't going to
+                // be rejected (it will be the most recent one)
+                headers.removeHeader(headers.size() - 1);
+            }
+        }
+
+
+
         boolean eol = false;
         int lastRealByte = start;
         if (pos - 1 > start) {
@@ -534,10 +551,10 @@ private void skipLine(int lineStart, int start) throws IOException {
             pos++;
         }
 
-        if (rejectIllegalHeaderName || log.isDebugEnabled()) {
+        if (rejectThisHeader || log.isDebugEnabled()) {
             String message = sm.getString("iib.invalidheader", HeaderUtil.toPrintableString(
                     buf, lineStart, lastRealByte - lineStart + 1));
-            if (rejectIllegalHeaderName) {
+            if (rejectThisHeader) {
                 throw new IllegalArgumentException(message);
             }
             log.debug(message);

java/org/apache/coyote/http11/InternalNioInputBuffer.java

@@ -604,7 +604,7 @@ private HeaderParseStatus parseHeader()
                 // Parsing continues so the error can be reported in context
                 headerData.lastSignificantChar = pos;
                 // skipLine() will handle the error
-                return skipLine();
+                return skipLine(false);
             }
 
             // chr is next byte of header name. Convert to lowercase.
@@ -616,7 +616,7 @@ private HeaderParseStatus parseHeader()
 
         // Skip the line and ignore the header
         if (headerParsePos == HeaderParsePosition.HEADER_SKIPLINE) {
-            return skipLine();
+            return skipLine(false);
         }
 
         //
@@ -668,15 +668,11 @@ private HeaderParseStatus parseHeader()
                     } else if (prevChr == Constants.CR && chr == Constants.LF) {
                         eol = true;
                     } else if (prevChr == Constants.CR) {
-                        // Invalid value
-                        // Delete the header (it will be the most recent one)
-                        headers.removeHeader(headers.size() - 1);
-                        return skipLine();
+                        // Invalid value - also need to delete header
+                        return skipLine(true);
                     } else if (chr != Constants.HT && HttpParser.isControl(chr)) {
-                        // Invalid value
-                        // Delete the header (it will be the most recent one)
-                        headers.removeHeader(headers.size() - 1);
-                        return skipLine();
+                        // Invalid value - also need to delete header
+                        return skipLine(true);
                     } else if (chr == Constants.SP || chr == Constants.HT) {
                         buf[headerData.realPos] = chr;
                         headerData.realPos++;
@@ -730,7 +726,27 @@ public int getParsingRequestLinePhase() {
         return parsingRequestLinePhase;
     }
 
-    private HeaderParseStatus skipLine() throws IOException {
+    private HeaderParseStatus skipLine(boolean deleteHeader) throws IOException {
+        boolean rejectThisHeader = rejectIllegalHeaderName;
+        // Check if rejectIllegalHeader is disabled and needs to be overridden
+        // for this header. The header name is required to determine if this
+        // override is required. The header name is only available once the
+        // header has been created. If the header has been created then
+        // deleteHeader will be true.
+        if (!rejectThisHeader && deleteHeader) {
+            if (headers.getName(headers.size() - 1).equalsIgnoreCase("content-length")) {
+                // Malformed content-length headers must always be rejected
+                // RFC 9112, section 6.3, bullet 5.
+                rejectThisHeader = true;
+            } else {
+                // Only need to delete the header if the request isn't going to
+                // be rejected (it will be the most recent one)
+                headers.removeHeader(headers.size() - 1);
+            }
+        }
+
+        // Parse the rest of the invalid header so we can construct a useful
+        // exception and/or debug message.
         headerParsePos = HeaderParsePosition.HEADER_SKIPLINE;
         boolean eol = false;
 
@@ -757,10 +773,10 @@ private HeaderParseStatus skipLine() throws IOException {
 
             pos++;
         }
-        if (rejectIllegalHeaderName || log.isDebugEnabled()) {
+        if (rejectThisHeader || log.isDebugEnabled()) {
             String message = sm.getString("iib.invalidheader", HeaderUtil.toPrintableString(
                     buf, headerData.lineStart, headerData.lastSignificantChar - headerData.lineStart + 1));
-            if (rejectIllegalHeaderName) {
+            if (rejectThisHeader) {
                 throw new IllegalArgumentException(message);
             }
             log.debug(message);

test/org/apache/coyote/http11/TestInternalInputBuffer.java

@@ -198,6 +198,10 @@ public void testBug51557CtlInValue() throws Exception {
                 // TAB is allowed
                 continue;
             }
+            if (i == '\n') {
+                // LF is the optional line terminator
+                continue;
+            }
             doTestBug51557InvalidCharInValue((char) i);
             tearDown();
             setUp();
@@ -748,6 +752,35 @@ public void testInvalidHeader01() {
         Assert.assertTrue(client.isResponseBodyOK());
     }
 
+    @Test
+    public void testInvalidContentLength01() {
+        doTestInvalidContentLength(false);
+    }
+
+
+    @Test
+    public void testInvalidContentLength02() {
+        doTestInvalidContentLength(true);
+    }
+
+
+    private void doTestInvalidContentLength(boolean rejectIllegalHeader) {
+        getTomcatInstance().getConnector().setProperty("rejectIllegalHeader", Boolean.toString(rejectIllegalHeader));
+
+        String[] request = new String[1];
+        request[0] =
+                "POST /test HTTP/1.1" + CRLF +
+                        "Host: localhost:8080" + CRLF +
+                        "Content-Length: 12\u000734" + CRLF +
+                        "Connection: close" + CRLF +
+                        CRLF;
+
+        InvalidClient client = new InvalidClient(request);
+
+        client.doRequest();
+        Assert.assertTrue(client.getResponseLine(), client.isResponse400());
+        Assert.assertTrue(client.isResponseBodyOK());
+    }
 
     /**
      * Invalid request test client.