This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/7.0.x by this push:
new b191a0d Correct a regression in transfer-encoding parsing
b191a0d is described below
commit b191a0d9cf06f4e04257c221bfe41d2b108a9cc8
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Dec 17 09:27:49 2019 +0000
Correct a regression in transfer-encoding parsing
Invalid tokens are an error
---
.../coyote/http11/AbstractHttp11Processor.java | 12 ++-
.../apache/coyote/http11/LocalStrings.properties | 1 +
.../apache/tomcat/util/http/parser/TokenList.java | 43 +++++++---
.../tomcat/util/http/parser/TestTokenList.java | 95 ++++++++++++++++++----
webapps/docs/changelog.xml | 5 ++
5 files changed, 123 insertions(+), 33 deletions(-)
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Processor.java
b/java/org/apache/coyote/http11/AbstractHttp11Processor.java
index 787d388..e5dacca 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Processor.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Processor.java
@@ -1534,10 +1534,14 @@ public abstract class AbstractHttp11Processor<S>
extends AbstractProcessor<S> {
}
if (transferEncodingValueMB != null) {
List<String> encodingNames = new ArrayList<String>();
- TokenList.parseTokenList(headers.values("transfer-encoding"),
encodingNames);
- for (String encodingName : encodingNames) {
- // "identity" codings are ignored
- addInputFilter(inputFilters, encodingName);
+ if (TokenList.parseTokenList(headers.values("transfer-encoding"),
encodingNames)) {
+ for (String encodingName : encodingNames) {
+ // "identity" codings are ignored
+ addInputFilter(inputFilters, encodingName);
+ }
+ } else {
+ // Invalid transfer encoding
+ badRequest("http11processor.request.invalidTransferEncoding");
}
}
diff --git a/java/org/apache/coyote/http11/LocalStrings.properties
b/java/org/apache/coyote/http11/LocalStrings.properties
index 292e2c1..b12dd2e 100644
--- a/java/org/apache/coyote/http11/LocalStrings.properties
+++ b/java/org/apache/coyote/http11/LocalStrings.properties
@@ -27,6 +27,7 @@ http11processor.regexp.error=Error parsing regular expression
[{0}]
http11processor.request.finish=Error finishing request
http11processor.request.inconsistentHosts=The host specified in the request
line is not consistent with the host header
http11processor.request.invalidScheme=The HTTP request contained an absolute
URI with an invalid scheme
+http11processor.request.invalidTransferEncoding=The HTTP request contained an
invalid Transfer-Encoding header
http11processor.request.invalidUri=The HTTP request contained an invalid URI
http11processor.request.invalidUserInfo=The HTTP request contained an absolute
URI with an invalid userinfo
http11processor.request.multipleContentLength=The request contained multiple
content-length headers
diff --git a/java/org/apache/tomcat/util/http/parser/TokenList.java
b/java/org/apache/tomcat/util/http/parser/TokenList.java
index 7ba886c..90b0233 100644
--- a/java/org/apache/tomcat/util/http/parser/TokenList.java
+++ b/java/org/apache/tomcat/util/http/parser/TokenList.java
@@ -36,19 +36,26 @@ public class TokenList {
* Parses an enumeration of header values of the form 1#token, forcing all
* parsed values to lower case.
*
- * @param inputs The headers to parse
- * @param result The Collection (usually a list of a set) to which the
- * parsed tokens should be added
+ * @param inputs The headers to parse
+ * @param collection The Collection (usually a list of a set) to which the
+ * parsed tokens should be added
+ *
+ * @return {@code} true if the header values were parsed cleanly, otherwise
+ * {@code false} (e.g. if a non-token value was encountered)
*
* @throws IOException If an I/O error occurs reading the header
*/
- public static void parseTokenList(Enumeration<String> inputs,
Collection<String> result) throws IOException {
+ public static boolean parseTokenList(Enumeration<String> inputs,
Collection<String> collection) throws IOException {
+ boolean result = true;
while (inputs.hasMoreElements()) {
String nextHeaderValue = inputs.nextElement();
if (nextHeaderValue != null) {
- TokenList.parseTokenList(new StringReader(nextHeaderValue),
result);
+ if (!TokenList.parseTokenList(new
StringReader(nextHeaderValue), collection)) {
+ result = false;
+ }
}
}
+ return result;
}
@@ -56,17 +63,24 @@ public class TokenList {
* Parses a header of the form 1#token, forcing all parsed values to lower
* case. This is typically used when header values are case-insensitive.
*
- * @param input The header to parse
- * @param result The Collection (usually a list of a set) to which the
- * parsed tokens should be added
+ * @param input The header to parse
+ * @param collection The Collection (usually a list of a set) to which the
+ * parsed tokens should be added
+ *
+ * @return {@code} true if the header was parsed cleanly, otherwise
+ * {@code false} (e.g. if a non-token value was encountered)
*
* @throws IOException If an I/O error occurs reading the header
*/
- public static void parseTokenList(Reader input, Collection<String> result)
throws IOException {
+ public static boolean parseTokenList(Reader input, Collection<String>
collection) throws IOException {
+ boolean invalid = false;
+ boolean valid = false;
+
do {
String fieldName = HttpParser.readToken(input);
if (fieldName == null) {
// Invalid field-name, skip to the next one
+ invalid = true;
HttpParser.skipUntil(input, 0, ',');
continue;
}
@@ -79,16 +93,23 @@ public class TokenList {
SkipResult skipResult = HttpParser.skipConstant(input, ",");
if (skipResult == SkipResult.EOF) {
// EOF
- result.add(fieldName.toLowerCase(Locale.ENGLISH));
+ valid = true;
+ collection.add(fieldName.toLowerCase(Locale.ENGLISH));
break;
} else if (skipResult == SkipResult.FOUND) {
- result.add(fieldName.toLowerCase(Locale.ENGLISH));
+ valid = true;
+ collection.add(fieldName.toLowerCase(Locale.ENGLISH));
continue;
} else {
// Not a token - ignore it
+ invalid = true;
HttpParser.skipUntil(input, 0, ',');
continue;
}
} while (true);
+
+ // Only return true if at least one valid token was read and no invalid
+ // entries were found
+ return valid && !invalid;
}
}
diff --git a/test/org/apache/tomcat/util/http/parser/TestTokenList.java
b/test/org/apache/tomcat/util/http/parser/TestTokenList.java
index 74053bb..2946727 100644
--- a/test/org/apache/tomcat/util/http/parser/TestTokenList.java
+++ b/test/org/apache/tomcat/util/http/parser/TestTokenList.java
@@ -31,7 +31,7 @@ public class TestTokenList {
public void testAll() throws IOException {
Set<String> expected = new HashSet<String>();
expected.add("*");
- doTestVary("*", expected);
+ doTestVary("*", expected, true);
}
@@ -39,7 +39,7 @@ public class TestTokenList {
public void testSingle() throws IOException {
Set<String> expected = new HashSet<String>();
expected.add("host");
- doTestVary("Host", expected);
+ doTestVary("Host", expected, true);
}
@@ -49,21 +49,21 @@ public class TestTokenList {
expected.add("bar");
expected.add("foo");
expected.add("host");
- doTestVary("Host, Foo, Bar", expected);
+ doTestVary("Host, Foo, Bar", expected, true);
}
@Test
public void testEmptyString() throws IOException {
Set<String> s = Collections.emptySet();
- doTestVary("", s);
+ doTestVary("", s, false);
}
@Test
public void testSingleInvalid() throws IOException {
Set<String> s = Collections.emptySet();
- doTestVary("{{{", s);
+ doTestVary("{{{", s, false);
}
@@ -73,7 +73,7 @@ public class TestTokenList {
expected.add("bar");
expected.add("foo");
expected.add("host");
- doTestVary("{{{, Host, Foo, Bar", expected);
+ doTestVary("{{{, Host, Foo, Bar", expected, false);
}
@@ -83,7 +83,7 @@ public class TestTokenList {
expected.add("bar");
expected.add("foo");
expected.add("host");
- doTestVary("Host, {{{, Foo, Bar", expected);
+ doTestVary("Host, {{{, Foo, Bar", expected, false);
}
@@ -93,7 +93,7 @@ public class TestTokenList {
expected.add("bar");
expected.add("foo");
expected.add("host");
- doTestVary("Host, Foo, Bar, {{{", expected);
+ doTestVary("Host, Foo, Bar, {{{", expected, false);
}
@@ -103,7 +103,7 @@ public class TestTokenList {
expected.add("bar");
expected.add("foo");
expected.add("host");
- doTestVary("OK {{{, Host, Foo, Bar", expected);
+ doTestVary("OK {{{, Host, Foo, Bar", expected, false);
}
@@ -113,7 +113,7 @@ public class TestTokenList {
expected.add("bar");
expected.add("foo");
expected.add("host");
- doTestVary("Host, OK {{{, Foo, Bar", expected);
+ doTestVary("Host, OK {{{, Foo, Bar", expected, false);
}
@@ -123,21 +123,80 @@ public class TestTokenList {
expected.add("bar");
expected.add("foo");
expected.add("host");
- doTestVary("Host, Foo, Bar, OK {{{", expected);
+ doTestVary("Host, Foo, Bar, OK {{{", expected, false);
}
@SuppressWarnings("deprecation")
- private void doTestVary(String input, Set<String> expected) throws
IOException {
+ private void doTestVary(String input, Set<String> expectedTokens, boolean
expectedResult) throws IOException {
StringReader reader = new StringReader(input);
- Set<String> result = new HashSet<String>();
- Vary.parseVary(reader, result);
- Assert.assertEquals(expected, result);
+ Set<String> tokens = new HashSet<String>();
+ Vary.parseVary(reader, tokens);
+ Assert.assertEquals(expectedTokens, tokens);
// Can't use reset(). Parser uses marks.
reader = new StringReader(input);
- result.clear();
- TokenList.parseTokenList(reader, result);
- Assert.assertEquals(expected, result);
+ tokens.clear();
+ boolean result = TokenList.parseTokenList(reader, tokens);
+ Assert.assertEquals(expectedTokens, tokens);
+ Assert.assertEquals(Boolean.valueOf(expectedResult),
Boolean.valueOf(result));
}
+
+
+ @Test
+ public void testMultipleHeadersValidWithoutNull() throws IOException {
+ doTestMultipleHeadersValid(false);
+ }
+
+
+ @Test
+ public void testMultipleHeadersValidWithNull() throws IOException {
+ doTestMultipleHeadersValid(true);
+ }
+
+
+ private void doTestMultipleHeadersValid(boolean withNull) throws
IOException {
+ Set<String> expectedTokens = new HashSet<String>();
+ expectedTokens.add("bar");
+ expectedTokens.add("foo");
+ expectedTokens.add("foo2");
+
+ Set<String> inputs = new HashSet<String>();
+ inputs.add("foo");
+ if (withNull) {
+ inputs.add(null);
+ }
+ inputs.add("bar, foo2");
+
+ Set<String> tokens = new HashSet<String>();
+
+
+ boolean result =
TokenList.parseTokenList(Collections.enumeration(inputs), tokens);
+ Assert.assertEquals(expectedTokens, tokens);
+ Assert.assertTrue(result);
+ }
+
+
+ @Test
+ public void doTestMultipleHeadersInvalid() throws IOException {
+ Set<String> expectedTokens = new HashSet<String>();
+ expectedTokens.add("bar");
+ expectedTokens.add("bar2");
+ expectedTokens.add("foo");
+ expectedTokens.add("foo2");
+ expectedTokens.add("foo3");
+
+ Set<String> inputs = new HashSet<String>();
+ inputs.add("foo");
+ inputs.add("bar2, }}}, foo3");
+ inputs.add("bar, foo2");
+
+ Set<String> tokens = new HashSet<String>();
+
+
+ boolean result =
TokenList.parseTokenList(Collections.enumeration(inputs), tokens);
+ Assert.assertEquals(expectedTokens, tokens);
+ Assert.assertFalse(result);
+ }
+
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 1962d6b..99d7e83 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -129,6 +129,11 @@
When reporting / logging invalid HTTP headers encode any non-printing
characters using the 0xNN form. (markt)
</add>
+ <fix>
+ Correct a regression introduced in 7.0.98 that meant invalid tokens in
+ the <code>Transfer-Encoding</code> header were ignored rather than
+ treated as an error. (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
