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
commit eb5c094e5560764cda436362254997511a3ca1f6
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Oct 5 20:51:48 2023 +0100
Align processing of trailer headers with standard processing
---
java/org/apache/coyote/http11/Http11InputBuffer.java | 8 +++++++-
.../apache/coyote/http11/filters/ChunkedInputFilter.java | 15 ++++++++++++++-
.../apache/coyote/http11/filters/LocalStrings.properties | 2 ++
webapps/docs/changelog.xml | 3 +++
4 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http11/Http11InputBuffer.java
b/java/org/apache/coyote/http11/Http11InputBuffer.java
index 78c9c1463b..1ae9eba04b 100644
--- a/java/org/apache/coyote/http11/Http11InputBuffer.java
+++ b/java/org/apache/coyote/http11/Http11InputBuffer.java
@@ -826,6 +826,12 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
*/
private HeaderParseStatus parseHeader() throws IOException {
+ /*
+ * Implementation note: Any changes to this method probably need to be
echoed in
+ * ChunkedInputFilter.parseHeader(). Why not use a common
implementation? In short, this code uses non-blocking
+ * reads whereas ChunkedInputFilter using blocking reads. The code is
just different enough that a common
+ * implementation wasn't viewed as practical.
+ */
while (headerParsePos == HeaderParsePosition.HEADER_START) {
// Read new bytes if needed
@@ -968,7 +974,7 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
} else if (prevChr == Constants.CR) {
// Invalid value - also need to delete header
return skipLine();
- } else if (chr != Constants.HT &&
HttpParser.isControl(chr)) {
+ } else if (HttpParser.isControl(chr) && chr !=
Constants.HT) {
// Invalid value - also need to delete header
return skipLine();
} else if (chr == Constants.SP || chr == Constants.HT) {
diff --git a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
index 7fe17f7b9d..3e844afb11 100644
--- a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
+++ b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
@@ -30,6 +30,7 @@ import org.apache.coyote.http11.Constants;
import org.apache.coyote.http11.InputFilter;
import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.buf.HexUtils;
+import org.apache.tomcat.util.http.parser.HttpParser;
import org.apache.tomcat.util.net.ApplicationBufferHandler;
import org.apache.tomcat.util.res.StringManager;
@@ -443,6 +444,13 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
private boolean parseHeader() throws IOException {
+ /*
+ * Implementation note: Any changes to this method probably need to be
echoed in
+ * Http11InputBuffer.parseHeader(). Why not use a common
implementation? In short, this code uses blocking
+ * reads whereas Http11InputBuffer using non-blocking reads. The code
is just different enough that a common
+ * implementation wasn't viewed as practical.
+ */
+
Map<String,String> headers = request.getTrailerFields();
byte chr = 0;
@@ -489,6 +497,9 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
if (chr == Constants.COLON) {
colon = true;
+ } else if (!HttpParser.isToken(chr)) {
+ // Non-token characters are illegal in header names
+ throw new
IOException(sm.getString("chunkedInputFilter.invalidTrailerHeaderName"));
} else {
trailingHeaders.append(chr);
}
@@ -550,7 +561,9 @@ public class ChunkedInputFilter implements InputFilter,
ApplicationBufferHandler
if (chr == Constants.CR || chr == Constants.LF) {
parseCRLF(true);
eol = true;
- } else if (chr == Constants.SP) {
+ } else if (HttpParser.isControl(chr) && chr != Constants.HT) {
+ throw new
IOException(sm.getString("chunkedInputFilter.invalidTrailerHeaderValue"));
+ } else if (chr == Constants.SP || chr == Constants.HT) {
trailingHeaders.append(chr);
} else {
trailingHeaders.append(chr);
diff --git a/java/org/apache/coyote/http11/filters/LocalStrings.properties
b/java/org/apache/coyote/http11/filters/LocalStrings.properties
index 72dc788bc5..3114a87d0a 100644
--- a/java/org/apache/coyote/http11/filters/LocalStrings.properties
+++ b/java/org/apache/coyote/http11/filters/LocalStrings.properties
@@ -24,6 +24,8 @@ chunkedInputFilter.invalidCrlfCRCR=Invalid end of line
sequence (CRCR)
chunkedInputFilter.invalidCrlfNoCR=Invalid end of line sequence (No CR before
LF)
chunkedInputFilter.invalidCrlfNoData=Invalid end of line sequence (no data
available to read)
chunkedInputFilter.invalidHeader=Invalid chunk header
+chunkedInputFilter.invalidTrailerHeaderName=Invalid trailer header name
(non-token character in name)
+chunkedInputFilter.invalidTrailerHeaderValue=Invalid trailer header value
(control character in value)
chunkedInputFilter.maxExtension=maxExtensionSize exceeded
chunkedInputFilter.maxTrailer=maxTrailerSize exceeded
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index de54d83039..73a337dcfa 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -170,6 +170,9 @@
<code>newPushBuilder()</code> will always return <code>null</code>.
(markt)
</update>
+ <fix>
+ Align validation of HTTP trailer fields with standard fields. (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
