This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new c9fe754e5d Requests with invalid content-length should always be
rejected
c9fe754e5d is described below
commit c9fe754e5d17e262dfbd3eab2a03ca96ff372dc3
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Oct 3 11:59:01 2022 +0100
Requests with invalid content-length should always be rejected
---
.../apache/coyote/http11/Http11InputBuffer.java | 42 +++++++++++++++-------
.../coyote/http11/TestHttp11InputBuffer.java | 31 ++++++++++++++++
webapps/docs/changelog.xml | 5 +++
3 files changed, 65 insertions(+), 13 deletions(-)
diff --git a/java/org/apache/coyote/http11/Http11InputBuffer.java
b/java/org/apache/coyote/http11/Http11InputBuffer.java
index 6496d8c2b2..ddd7e2d1e2 100644
--- a/java/org/apache/coyote/http11/Http11InputBuffer.java
+++ b/java/org/apache/coyote/http11/Http11InputBuffer.java
@@ -919,7 +919,7 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
headerData.lastSignificantChar = pos;
byteBuffer.position(byteBuffer.position() - 1);
// skipLine() will handle the error
- return skipLine();
+ return skipLine(false);
}
// chr is next byte of header name. Convert to lowercase.
@@ -930,7 +930,7 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
// Skip the line and ignore the header
if (headerParsePos == HeaderParsePosition.HEADER_SKIPLINE) {
- return skipLine();
+ return skipLine(false);
}
//
@@ -987,15 +987,11 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
// CRLF or LF is an acceptable line terminator
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) {
byteBuffer.put(headerData.realPos, chr);
headerData.realPos++;
@@ -1043,7 +1039,27 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
}
- private HeaderParseStatus skipLine() throws IOException {
+ private HeaderParseStatus skipLine(boolean deleteHeader) throws
IOException {
+ boolean rejectThisHeader = rejectIllegalHeader;
+ // 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;
@@ -1069,11 +1085,11 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
headerData.lastSignificantChar = pos;
}
}
- if (rejectIllegalHeader || log.isDebugEnabled()) {
+ if (rejectThisHeader || log.isDebugEnabled()) {
String message = sm.getString("iib.invalidheader",
HeaderUtil.toPrintableString(byteBuffer.array(),
headerData.lineStart,
headerData.lastSignificantChar -
headerData.lineStart + 1));
- if (rejectIllegalHeader) {
+ if (rejectThisHeader) {
throw new IllegalArgumentException(message);
}
log.debug(message);
diff --git a/test/org/apache/coyote/http11/TestHttp11InputBuffer.java
b/test/org/apache/coyote/http11/TestHttp11InputBuffer.java
index c8b10d0135..1857882478 100644
--- a/test/org/apache/coyote/http11/TestHttp11InputBuffer.java
+++ b/test/org/apache/coyote/http11/TestHttp11InputBuffer.java
@@ -709,6 +709,37 @@ public class TestHttp11InputBuffer extends TomcatBaseTest {
}
+ @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.
*/
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 560749e7e4..90393db49f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -124,6 +124,11 @@
<bug>66281</bug>: Fix unexpected timeouts that may appear as client
disconnections when using HTTP/2 and NIO2. (markt)
</fix>
+ <fix>
+ Enforce the requirement of RFC 7230 onwards that a request with a
+ malformed <code>content-length</code> header should always be rejected
+ with a 400 response. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
