This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit c83fe47725f7ae9ae213568d9039171124fb7ec6 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 da3e456d2f..60e5027961 100644 --- a/java/org/apache/coyote/http11/Http11InputBuffer.java +++ b/java/org/apache/coyote/http11/Http11InputBuffer.java @@ -830,6 +830,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 @@ -972,7 +978,7 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler } else if (prevChr == Constants.CR) { // Invalid value - also need to delete header return skipLine(true); - } 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(true); } 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 d19b4eaa13..90489e7f45 100644 --- a/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java +++ b/java/org/apache/coyote/http11/filters/ChunkedInputFilter.java @@ -31,6 +31,7 @@ import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.HexUtils; import org.apache.tomcat.util.buf.MessageBytes; import org.apache.tomcat.util.http.MimeHeaders; +import org.apache.tomcat.util.http.parser.HttpParser; import org.apache.tomcat.util.net.ApplicationBufferHandler; import org.apache.tomcat.util.res.StringManager; @@ -503,6 +504,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. + */ + MimeHeaders headers = request.getMimeHeaders(); byte chr = 0; @@ -549,6 +557,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); } @@ -610,7 +621,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 52a94c90e6..06207e9937 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -156,6 +156,9 @@ <fix> Avoid rare thread safety issue accessing message digest map. (remm) </fix> + <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