From 038f94a0759f6ef374b22fd2a0e4b96bf647a5bf Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Thu, 10 Nov 2022 20:31:10 -0600 Subject: [PATCH 1/4] Added test for invalid Content-Length header with rejectIllegalHeader config --- .../http11/TestInternalInputBuffer.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/org/apache/coyote/http11/TestInternalInputBuffer.java b/test/org/apache/coyote/http11/TestInternalInputBuffer.java index 4c149b0afbfe..8cab874edff2 100644 --- a/test/org/apache/coyote/http11/TestInternalInputBuffer.java +++ b/test/org/apache/coyote/http11/TestInternalInputBuffer.java @@ -748,6 +748,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. From 3e82f5a05f8088fc8a3c8f9ec2ece8b7fa6555ba Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Tue, 15 Nov 2022 13:14:13 -0600 Subject: [PATCH 2/4] updated github actions to java 7 352 --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c50eb5c51432..7d0a1fad7e92 100644 --- a/.github/workflows/main.yml +++ b/.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 From f673bbf71950a37d31f5a8ae88e7666036528535 Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Fri, 11 Nov 2022 13:31:58 -0600 Subject: [PATCH 3/4] backport a1c07906d8dcaf7957e5cc97f5cdbac7d18a205a from apache 8.5.x --- .../coyote/http11/InternalInputBuffer.java | 41 ++++++++++++------ .../coyote/http11/InternalNioInputBuffer.java | 42 +++++++++++++------ .../http11/TestInternalInputBuffer.java | 4 ++ 3 files changed, 62 insertions(+), 25 deletions(-) diff --git a/java/org/apache/coyote/http11/InternalInputBuffer.java b/java/org/apache/coyote/http11/InternalInputBuffer.java index 4782d50819bc..c305d3f17f28 100644 --- a/java/org/apache/coyote/http11/InternalInputBuffer.java +++ b/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 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); diff --git a/java/org/apache/coyote/http11/InternalNioInputBuffer.java b/java/org/apache/coyote/http11/InternalNioInputBuffer.java index 1da4c4cd7640..a0e087790ec9 100644 --- a/java/org/apache/coyote/http11/InternalNioInputBuffer.java +++ b/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); diff --git a/test/org/apache/coyote/http11/TestInternalInputBuffer.java b/test/org/apache/coyote/http11/TestInternalInputBuffer.java index 8cab874edff2..42981f86033e 100644 --- a/test/org/apache/coyote/http11/TestInternalInputBuffer.java +++ b/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(); From 74e50de91b5d118582a25791f3ebbee2359d8993 Mon Sep 17 00:00:00 2001 From: Cesar Hernandez Date: Tue, 15 Nov 2022 13:20:19 -0600 Subject: [PATCH 4/4] Prepare for release 7.0.104.SP.6 --- build.properties.default | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.properties.default b/build.properties.default index 8098e23c4f70..c5ea7ddcbcd3 100644 --- a/build.properties.default +++ b/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