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 2011/07/27 11:10:13 UTC
svn commit: r1151394 - in /tomcat/trunk: java/org/apache/coyote/http11/
test/org/apache/coyote/http11/ webapps/docs/
Author: markt
Date: Wed Jul 27 09:10:11 2011
New Revision: 1151394
URL: http://svn.apache.org/viewvc?rev=1151394&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51557
Ignore HTTP headers that do not comply with RFC 2616 and use header names that are not tokens.
Modified:
tomcat/trunk/java/org/apache/coyote/http11/AbstractInputBuffer.java
tomcat/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java
tomcat/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java
tomcat/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
tomcat/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractInputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractInputBuffer.java?rev=1151394&r1=1151393&r2=1151394&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractInputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractInputBuffer.java Wed Jul 27 09:10:11 2011
@@ -27,20 +27,7 @@ import org.apache.tomcat.util.res.String
public abstract class AbstractInputBuffer implements InputBuffer{
- public abstract boolean parseRequestLine(boolean useAvailableDataOnly) throws IOException;
-
- public abstract boolean parseHeaders() throws IOException;
-
- protected abstract boolean fill(boolean block) throws IOException;
-
- // -------------------------------------------------------------- Constants
-
-
- // ----------------------------------------------------------- Constructors
-
-
- // -------------------------------------------------------------- Variables
-
+ protected static final boolean[] HTTP_TOKEN_CHAR = new boolean[128];
/**
* The string manager for this package.
@@ -49,7 +36,55 @@ public abstract class AbstractInputBuffe
StringManager.getManager(Constants.Package);
- // ----------------------------------------------------- Instance Variables
+ 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 if (i == '\t') {
+ HTTP_TOKEN_CHAR[i] = false;
+ } else {
+ HTTP_TOKEN_CHAR[i] = true;
+ }
+ }
+ }
/**
@@ -217,6 +252,13 @@ public abstract class AbstractInputBuffe
}
+ public abstract boolean parseRequestLine(boolean useAvailableDataOnly) throws IOException;
+
+ public abstract boolean parseHeaders() throws IOException;
+
+ protected abstract boolean fill(boolean block) throws IOException;
+
+
// --------------------------------------------------------- Public Methods
@@ -308,6 +350,4 @@ public abstract class AbstractInputBuffe
return activeFilters[lastActiveFilter].doRead(chunk,req);
}
-
-
}
Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java?rev=1151394&r1=1151393&r2=1151394&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/InternalAprInputBuffer.java Wed Jul 27 09:10:11 2011
@@ -14,17 +14,18 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-
package org.apache.coyote.http11;
import java.io.EOFException;
import java.io.IOException;
import java.net.SocketTimeoutException;
import java.nio.ByteBuffer;
+import java.nio.charset.Charset;
import org.apache.coyote.InputBuffer;
import org.apache.coyote.Request;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.jni.Socket;
import org.apache.tomcat.jni.Status;
import org.apache.tomcat.util.buf.ByteChunk;
@@ -38,9 +39,8 @@ import org.apache.tomcat.util.buf.Messag
*/
public class InternalAprInputBuffer extends AbstractInputBuffer {
-
- // -------------------------------------------------------------- Constants
-
+ private static final Log log =
+ LogFactory.getLog(InternalAprInputBuffer.class);
// ----------------------------------------------------------- Constructors
@@ -394,6 +394,11 @@ public class InternalAprInputBuffer exte
if (buf[pos] == Constants.COLON) {
colon = true;
headerValue = headers.addValue(buf, start, pos - start);
+ } else if (!HTTP_TOKEN_CHAR[buf[pos]]) {
+ // If a non-token header is detected, skip the line and
+ // ignore the header
+ skipLine(start);
+ return true;
}
chr = buf[pos];
if ((chr >= Constants.A) && (chr <= Constants.Z)) {
@@ -496,6 +501,38 @@ public class InternalAprInputBuffer exte
}
+ private void skipLine(int start) throws IOException {
+ boolean eol = false;
+ int lastRealByte = start;
+ if (pos - 1 > start) {
+ lastRealByte = pos - 1;
+ }
+
+ while (!eol) {
+
+ // Read new bytes if needed
+ if (pos >= lastValid) {
+ if (!fill())
+ throw new EOFException(sm.getString("iib.eof.error"));
+ }
+
+ if (buf[pos] == Constants.CR) {
+ // Skip
+ } else if (buf[pos] == Constants.LF) {
+ eol = true;
+ } else {
+ lastRealByte = pos;
+ }
+ pos++;
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("iib.invalidheader", new String(buf, start,
+ lastRealByte - start + 1, Charset.forName("ISO-8859-1"))));
+ }
+ }
+
+
/**
* Available bytes (note that due to encoding, this may not correspond )
*/
Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java?rev=1151394&r1=1151393&r2=1151394&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/InternalInputBuffer.java Wed Jul 27 09:10:11 2011
@@ -14,15 +14,16 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-
package org.apache.coyote.http11;
import java.io.EOFException;
import java.io.IOException;
+import java.nio.charset.Charset;
import org.apache.coyote.InputBuffer;
import org.apache.coyote.Request;
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.buf.MessageBytes;
@@ -34,6 +35,9 @@ import org.apache.tomcat.util.buf.Messag
*/
public class InternalInputBuffer extends AbstractInputBuffer {
+ private static final Log log = LogFactory.getLog(InternalInputBuffer.class);
+
+
/**
* Default constructor.
*/
@@ -316,7 +320,13 @@ public class InternalInputBuffer extends
if (buf[pos] == Constants.COLON) {
colon = true;
headerValue = headers.addValue(buf, start, pos - start);
+ } else if (!HTTP_TOKEN_CHAR[buf[pos]]) {
+ // If a non-token header is detected, skip the line and
+ // ignore the header
+ skipLine(start);
+ return true;
}
+
chr = buf[pos];
if ((chr >= Constants.A) && (chr <= Constants.Z)) {
buf[pos] = (byte) (chr - Constants.LC_OFFSET);
@@ -421,6 +431,37 @@ public class InternalInputBuffer extends
// ------------------------------------------------------ Protected Methods
+ private void skipLine(int start) throws IOException {
+ boolean eol = false;
+ int lastRealByte = start;
+ if (pos - 1 > start) {
+ lastRealByte = pos - 1;
+ }
+
+ while (!eol) {
+
+ // Read new bytes if needed
+ if (pos >= lastValid) {
+ if (!fill())
+ throw new EOFException(sm.getString("iib.eof.error"));
+ }
+
+ if (buf[pos] == Constants.CR) {
+ // Skip
+ } else if (buf[pos] == Constants.LF) {
+ eol = true;
+ } else {
+ lastRealByte = pos;
+ }
+ pos++;
+ }
+
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("iib.invalidheader", new String(buf, start,
+ lastRealByte - start + 1, Charset.forName("ISO-8859-1"))));
+ }
+ }
+
/**
* Fill the internal buffer using data from the underlying input stream.
*
Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java?rev=1151394&r1=1151393&r2=1151394&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/InternalNioInputBuffer.java Wed Jul 27 09:10:11 2011
@@ -14,8 +14,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-
package org.apache.coyote.http11;
import java.io.EOFException;
@@ -40,9 +38,6 @@ import org.apache.tomcat.util.net.NioSel
*/
public class InternalNioInputBuffer extends AbstractInputBuffer {
- /**
- * Logger.
- */
private static final org.apache.juli.logging.Log log =
org.apache.juli.logging.LogFactory.getLog(InternalNioInputBuffer.class);
@@ -52,7 +47,8 @@ public class InternalNioInputBuffer exte
// -------------------------------------------------------------- Constants
enum HeaderParseStatus {DONE, HAVE_MORE_HEADERS, NEED_MORE_DATA}
- enum HeaderParsePosition {HEADER_START, HEADER_NAME, HEADER_VALUE, HEADER_MULTI_LINE}
+ enum HeaderParsePosition {HEADER_START, HEADER_NAME, HEADER_VALUE,
+ HEADER_MULTI_LINE, HEADER_SKIPLINE}
// ----------------------------------------------------------- Constructors
@@ -561,6 +557,10 @@ public class InternalNioInputBuffer exte
if (buf[pos] == Constants.COLON) {
headerParsePos = HeaderParsePosition.HEADER_VALUE;
headerData.headerValue = headers.addValue(buf, headerData.start, pos - headerData.start);
+ } else if (!HTTP_TOKEN_CHAR[buf[pos]]) {
+ // If a non-token header is detected, skip the line and
+ // ignore the header
+ return skipLine();
}
chr = buf[pos];
if ((chr >= Constants.A) && (chr <= Constants.Z)) {
@@ -576,6 +576,10 @@ public class InternalNioInputBuffer exte
}
+ while (headerParsePos == HeaderParsePosition.HEADER_SKIPLINE) {
+ return skipLine();
+ }
+
//
// Reading the header value (which can be spanned over multiple lines)
//
@@ -673,6 +677,41 @@ public class InternalNioInputBuffer exte
return HeaderParseStatus.HAVE_MORE_HEADERS;
}
+ private HeaderParseStatus skipLine() throws IOException {
+ headerParsePos = HeaderParsePosition.HEADER_SKIPLINE;
+ boolean eol = false;
+
+ // Reading bytes until the end of the line
+ while (!eol) {
+
+ // Read new bytes if needed
+ if (pos >= lastValid) {
+ if (!fill(true,false)) {
+ return HeaderParseStatus.NEED_MORE_DATA;
+ }
+ }
+
+ if (buf[pos] == Constants.CR) {
+ // Skip
+ } else if (buf[pos] == Constants.LF) {
+ eol = true;
+ } else {
+ headerData.lastSignificantChar = pos;
+ }
+
+ pos++;
+ }
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("iib.invalidheader", new String(buf,
+ headerData.start,
+ headerData.lastSignificantChar - headerData.start + 1,
+ DEFAULT_CHARSET)));
+ }
+
+ headerParsePos = HeaderParsePosition.HEADER_START;
+ return HeaderParseStatus.HAVE_MORE_HEADERS;
+ }
+
protected HeaderParseData headerData = new HeaderParseData();
public static class HeaderParseData {
int start = 0;
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=1151394&r1=1151393&r2=1151394&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties Wed Jul 27 09:10:11 2011
@@ -38,5 +38,6 @@ http11processor.comet.notsupported=The C
http11processor.sendfile.error=Error sending data using sendfile. May be caused by invalid request attributes for start/end points
iib.eof.error=Unexpected EOF read on the socket
-iib.requestheadertoolarge.error=Request header is too large
+iib.invalidheader=The HTTP header line [{0}] does not conform to RFC 2616 and has been ignored.
iib.invalidmethod=Invalid character (CR or LF) found in method name
+iib.requestheadertoolarge.error=Request header is too large
Modified: tomcat/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java?rev=1151394&r1=1151393&r2=1151394&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http11/TestInternalInputBuffer.java Wed Jul 27 09:10:11 2011
@@ -26,6 +26,7 @@ import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import org.junit.Test;
@@ -124,4 +125,199 @@ public class TestInternalInputBuffer ext
}
}
}
+
+
+ @Test
+ public void testBug51557NoColon() {
+
+ Bug51557Client client = new Bug51557Client("X-Bug51557NoColon");
+ client.setPort(getPort());
+
+ client.doRequest();
+ assertTrue(client.isResponse200());
+ assertEquals("abcd", client.getResponseBody());
+ assertTrue(client.isResponseBodyOK());
+ }
+
+
+ @Test
+ public void testBug51557Separators() throws Exception {
+ char httpSeparators[] = new char[] {
+ '\t', ' ', '\"', '(', ')', ',', '/', ':', ';', '<',
+ '=', '>', '?', '@', '[', '\\', ']', '{', '}' };
+
+ for (char s : httpSeparators) {
+ doTestBug51557Char(s);
+ tearDown();
+ setUp();
+ }
+ }
+
+
+ @Test
+ public void testBug51557Ctl() throws Exception {
+ for (int i = 0; i < 31; i++) {
+ doTestBug51557Char((char) i);
+ tearDown();
+ setUp();
+ }
+ doTestBug51557Char((char) 127);
+ }
+
+
+ @Test
+ public void testBug51557Continuation() {
+
+ Bug51557Client client = new Bug51557Client("X-Bug=51557NoColon",
+ "foo" + SimpleHttpClient.CRLF + " bar");
+ client.setPort(getPort());
+
+ client.doRequest();
+ assertTrue(client.isResponse200());
+ assertEquals("abcd", client.getResponseBody());
+ assertTrue(client.isResponseBodyOK());
+ }
+
+
+ @Test
+ public void testBug51557BoundaryStart() {
+
+ Bug51557Client client = new Bug51557Client("=X-Bug51557",
+ "invalid");
+ client.setPort(getPort());
+
+ client.doRequest();
+ assertTrue(client.isResponse200());
+ assertEquals("abcd", client.getResponseBody());
+ assertTrue(client.isResponseBodyOK());
+ }
+
+
+ @Test
+ public void testBug51557BoundaryEnd() {
+
+ Bug51557Client client = new Bug51557Client("X-Bug51557=",
+ "invalid");
+ client.setPort(getPort());
+
+ client.doRequest();
+ assertTrue(client.isResponse200());
+ assertEquals("abcd", client.getResponseBody());
+ assertTrue(client.isResponseBodyOK());
+ }
+
+
+ private void doTestBug51557Char(char s) {
+ Bug51557Client client =
+ new Bug51557Client("X-Bug" + s + "51557", "invalid");
+
+ client.setPort(getPort());
+ client.doRequest();
+ assertTrue(client.isResponse200());
+ assertEquals("abcd", client.getResponseBody());
+ assertTrue(client.isResponseBodyOK());
+ }
+
+ /**
+ * Bug 51557 test client.
+ */
+ private class Bug51557Client extends SimpleHttpClient {
+
+ private String headerName;
+ private String headerLine;
+
+ public Bug51557Client(String headerName) {
+ this.headerName = headerName;
+ this.headerLine = headerName;
+ }
+
+ public Bug51557Client(String headerName, String headerValue) {
+ this.headerName = headerName;
+ this.headerLine = headerName + ": " + headerValue;
+ }
+
+ private Exception doRequest() {
+
+ Tomcat tomcat = getTomcatInstance();
+
+ Context root = tomcat.addContext("", TEMP_DIR);
+ Tomcat.addServlet(root, "Bug51557",
+ new Bug51557Servlet(headerName));
+ root.addServletMapping("/test", "Bug51557");
+
+ try {
+ tomcat.start();
+
+ // Open connection
+ connect();
+
+ String[] request = new String[1];
+ request[0] =
+ "GET http://localhost:8080/test HTTP/1.1" + CRLF +
+ headerLine + CRLF +
+ "X-Bug51557: abcd" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF;
+
+ setRequest(request);
+ processRequest(); // blocks until response has been read
+
+ // Close the connection
+ disconnect();
+ } catch (Exception e) {
+ return e;
+ }
+ return null;
+ }
+
+ @Override
+ public boolean isResponseBodyOK() {
+ if (getResponseBody() == null) {
+ return false;
+ }
+ if (!getResponseBody().contains("abcd")) {
+ return false;
+ }
+ return true;
+ }
+
+ }
+
+ private static class Bug51557Servlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ private String invalidHeaderName;
+
+ /**
+ * @param invalidHeaderName The header name should be invalid and
+ * therefore ignored by the header parsing code
+ */
+ public Bug51557Servlet(String invalidHeaderName) {
+ this.invalidHeaderName = invalidHeaderName;
+ }
+
+ /**
+ * Only interested in the request headers from a GET request
+ */
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+ throws ServletException, IOException {
+ // Just echo the header value back as plain text
+ resp.setContentType("text/plain");
+
+ PrintWriter out = resp.getWriter();
+
+ processHeaders(invalidHeaderName, req, out);
+ processHeaders("X-Bug51557", req, out);
+ }
+
+ private void processHeaders(String header, HttpServletRequest req,
+ PrintWriter out) {
+ Enumeration<String> values = req.getHeaders(header);
+ while (values.hasMoreElements()) {
+ out.println(values.nextElement());
+ }
+ }
+ }
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1151394&r1=1151393&r2=1151394&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Jul 27 09:10:11 2011
@@ -116,6 +116,10 @@
<bug>51503</bug>: Add additional validation that prevents a connector
from starting if it does not have a port > 0. (markt)
</fix>
+ <fix>
+ <bug>51557</bug>: Ignore HTTP headers that do not comply with RFC 2616
+ and use header names that are not tokens. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org