You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2016/11/02 11:57:14 UTC
svn commit: r1767641 - in /tomcat/trunk: java/org/apache/coyote/http11/
java/org/apache/tomcat/util/http/parser/
test/org/apache/catalina/valves/rewrite/ webapps/docs/
Author: markt
Date: Wed Nov 2 11:57:14 2016
New Revision: 1767641
URL: http://svn.apache.org/viewvc?rev=1767641&view=rev
Log:
Add additional checks for valid characters to the HTTP request line
parsing so invalid request lines are rejected sooner.
Modified:
tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java
tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java?rev=1767641&r1=1767640&r2=1767641&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java Wed Nov 2 11:57:14 2016
@@ -27,6 +27,7 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
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.net.SocketWrapperBase;
import org.apache.tomcat.util.res.StringManager;
@@ -47,56 +48,6 @@ public class Http11InputBuffer implement
private static final StringManager sm = StringManager.getManager(Http11InputBuffer.class);
- private static final boolean[] HTTP_TOKEN_CHAR = new boolean[128];
- static {
- for (int i = 0; i < 128; i++) {
- if (i < 32) {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == 127) {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '(') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == ')') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '<') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '>') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '@') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == ',') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == ';') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == ':') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '\\') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '\"') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '/') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '[') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == ']') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '?') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '=') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '{') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == '}') {
- HTTP_TOKEN_CHAR[i] = false;
- } else if (i == ' ') {
- HTTP_TOKEN_CHAR[i] = false;
- } else {
- HTTP_TOKEN_CHAR[i] = true;
- }
- }
- }
-
-
private static final byte[] CLIENT_PREFACE_START =
"PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n".getBytes(StandardCharsets.ISO_8859_1);
@@ -446,7 +397,7 @@ public class Http11InputBuffer implement
space = true;
request.method().setBytes(byteBuffer.array(), parsingRequestLineStart,
pos - parsingRequestLineStart);
- } else if (!HTTP_TOKEN_CHAR[chr]) {
+ } else if (!HttpParser.isToken(chr)) {
byteBuffer.position(byteBuffer.position() - 1);
throw new IllegalArgumentException(sm.getString("iib.invalidmethod"));
}
@@ -497,6 +448,8 @@ public class Http11InputBuffer implement
end = pos;
} else if (chr == Constants.QUESTION && parsingRequestLineQPos == -1) {
parsingRequestLineQPos = pos;
+ } else if (HttpParser.isNotRequestTarget(chr)) {
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
}
}
if (parsingRequestLineQPos >= 0) {
@@ -534,7 +487,7 @@ public class Http11InputBuffer implement
if (parsingRequestLinePhase == 6) {
//
// Reading the protocol
- // Protocol is always US-ASCII
+ // Protocol is always "HTTP/" DIGIT "." DIGIT
//
while (!parsingRequestLineEol) {
// Read new bytes if needed
@@ -552,6 +505,8 @@ public class Http11InputBuffer implement
end = pos;
}
parsingRequestLineEol = true;
+ } else if (!HttpParser.isHttpProtocol(chr)) {
+ throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
}
}
@@ -816,7 +771,7 @@ public class Http11InputBuffer implement
headerData.realPos = pos;
headerData.lastSignificantChar = pos;
break;
- } else if (chr < 0 || !HTTP_TOKEN_CHAR[chr]) {
+ } else if (chr < 0 || !HttpParser.isToken(chr)) {
// If a non-token header is detected, skip the line and
// ignore the header
headerData.lastSignificantChar = pos;
Modified: tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties?rev=1767641&r1=1767640&r2=1767641&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties Wed Nov 2 11:57:14 2016
@@ -31,8 +31,10 @@ iib.available.readFail=A non-blocking re
iib.eof.error=Unexpected EOF read on the socket
iib.failedread.apr=Read failed with APR/native error code [{0}]
iib.filter.npe=You may not add a null filter
-iib.invalidheader=The HTTP header line [{0}] does not conform to RFC 2616 and has been ignored.
+iib.invalidheader=The HTTP header line [{0}] does not conform to RFC 7230 and has been ignored.
iib.invalidmethod=Invalid character found in method name. HTTP method names must be tokens
+iib.invalidRequestTarget=Invalid character found in the request target. The valid characters are defined in RFC 7230 and RFC 3986
+iib.invalidHttpProtocol=Invalid character found in the HTTP protocol
iib.parseheaders.ise.error=Unexpected state: headers already parsed. Buffer not recycled?
iib.readtimeout=Timeout attempting to read data from the socket
iib.requestheadertoolarge.error=Request header is too large
Modified: tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java?rev=1767641&r1=1767640&r2=1767641&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/parser/HttpParser.java Wed Nov 2 11:57:14 2016
@@ -40,6 +40,8 @@ public class HttpParser {
private static final boolean[] IS_SEPARATOR = new boolean[ARRAY_SIZE];
private static final boolean[] IS_TOKEN = new boolean[ARRAY_SIZE];
private static final boolean[] IS_HEX = new boolean[ARRAY_SIZE];
+ private static final boolean[] IS_NOT_REQUEST_TARGET = new boolean[ARRAY_SIZE];
+ private static final boolean[] IS_HTTP_PROTOCOL = new boolean[ARRAY_SIZE];
static {
for (int i = 0; i < ARRAY_SIZE; i++) {
@@ -65,6 +67,21 @@ public class HttpParser {
if ((i >= '0' && i <='9') || (i >= 'a' && i <= 'f') || (i >= 'A' && i <= 'F')) {
IS_HEX[i] = true;
}
+
+ // Not valid for request target.
+ // Combination of multiple rules from RFC7230 and RFC 3986. Must be
+ // ASCII, no controls plus a few additional characters excluded
+ if (IS_CONTROL[i] || i > 127 ||
+ i == ' ' || i == '\"' || i == '#' || i == '<' || i == '>' || i == '\\' ||
+ i == '^' || i == '`' || i == '{' || i == '|' || i == '}') {
+ IS_NOT_REQUEST_TARGET[i] = true;
+ }
+
+ // Not valid for HTTP protocol
+ // "HTTP/" DIGIT "." DIGIT
+ if (i == 'H' || i == 'T' || i == 'P' || i == '/' || i == '.' || (i >= '0' && i <= '9')) {
+ IS_HTTP_PROTOCOL[i] = true;
+ }
}
}
@@ -99,6 +116,7 @@ public class HttpParser {
return result.toString();
}
+
public static boolean isToken(int c) {
// Fast for correct values, slower for incorrect ones
try {
@@ -108,8 +126,9 @@ public class HttpParser {
}
}
+
public static boolean isHex(int c) {
- // Fast for correct values, slower for incorrect ones
+ // Fast for correct values, slower for some incorrect ones
try {
return IS_HEX[c];
} catch (ArrayIndexOutOfBoundsException ex) {
@@ -117,6 +136,29 @@ public class HttpParser {
}
}
+
+ public static boolean isNotRequestTarget(int c) {
+ // Fast for valid request target characters, slower for some incorrect
+ // ones
+ try {
+ return IS_NOT_REQUEST_TARGET[c];
+ } catch (ArrayIndexOutOfBoundsException ex) {
+ return true;
+ }
+ }
+
+
+ public static boolean isHttpProtocol(int c) {
+ // Fast for valid HTTP protocol characters, slower for some incorrect
+ // ones
+ try {
+ return IS_HTTP_PROTOCOL[c];
+ } catch (ArrayIndexOutOfBoundsException ex) {
+ return false;
+ }
+ }
+
+
// Skip any LWS and return the next char
static int skipLws(StringReader input, boolean withReset) throws IOException {
Modified: tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java?rev=1767641&r1=1767640&r2=1767641&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java (original)
+++ tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java Wed Nov 2 11:57:14 2016
@@ -19,6 +19,7 @@ package org.apache.catalina.valves.rewri
import java.nio.charset.StandardCharsets;
import org.junit.Assert;
+import org.junit.Ignore;
import org.junit.Test;
import org.apache.catalina.Context;
@@ -227,6 +228,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8WithBothQsFlagsRNE() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
@@ -237,6 +239,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8WithBothQsFlagsRBNE() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
@@ -274,6 +277,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8WithBothQsFlagsRNEQSA() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
@@ -285,6 +289,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8WithBothQsFlagsRBNEQSA() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
@@ -328,6 +333,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8WithOriginalQsFlagsRNE() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
@@ -338,6 +344,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8WithOriginalQsFlagsRBNE() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
@@ -389,6 +396,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8WithRewriteQsFlagsRNE() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
@@ -399,6 +407,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8WithRewriteQsFlagsRBNE() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
@@ -446,6 +455,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8FlagsRNE() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
@@ -456,6 +466,7 @@ public class TestRewriteValve extends To
@Test
+ @Ignore // Use of NE results in invalid characters in request-target
public void testUtf8FlagsRBNE() throws Exception {
// Note %C2%A1 == \u00A1
// Failing to escape the redirect means UTF-8 bytes in the Location
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1767641&r1=1767640&r2=1767641&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Nov 2 11:57:14 2016
@@ -174,6 +174,10 @@
Improve detection of I/O errors during async processing on non-container
threads and trigger async error handling when they are detected. (markt)
</fix>
+ <add>
+ Add additional checks for valid characters to the HTTP request line
+ parsing so invalid request lines are rejected sooner. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org