Author: markt Date: Thu May 12 19:56:17 2016 New Revision: 1743554 URL: http://svn.apache.org/viewvc?rev=1743554&view=rev Log: Stricter validation of HTTP method names
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties tomcat/trunk/test/org/apache/coyote/http11/TestHttp11InputBuffer.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=1743554&r1=1743553&r2=1743554&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11InputBuffer.java Thu May 12 19:56:17 2016 @@ -444,7 +444,7 @@ public class Http11InputBuffer implement if ( parsingRequestLinePhase == 2 ) { // // Reading the method name - // Method name is always US-ASCII + // Method name is a token // boolean space = false; while (!space) { @@ -453,21 +453,20 @@ public class Http11InputBuffer implement if (!fill(false)) //request line parsing return false; } - // Spec says no CR or LF in method name - if (buf[pos] == Constants.CR || buf[pos] == Constants.LF) { - throw new IllegalArgumentException( - sm.getString("iib.invalidmethod")); - } + // Spec says method name is a token followed by a single SP but + // also be tolerant of multiple SP and/or HT. if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) { space = true; request.method().setBytes(buf, parsingRequestLineStart, pos - parsingRequestLineStart); + } else if (!HTTP_TOKEN_CHAR[buf[pos]]) { + throw new IllegalArgumentException(sm.getString("iib.invalidmethod")); } pos++; } parsingRequestLinePhase = 3; } if ( parsingRequestLinePhase == 3 ) { - // Spec says single SP but also be tolerant of multiple and/or HT + // Spec says single SP but also be tolerant of multiple SP and/or HT boolean space = true; while (space) { // Read new bytes if needed 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=1743554&r1=1743553&r2=1743554&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties Thu May 12 19:56:17 2016 @@ -34,7 +34,7 @@ iib.eof.error=Unexpected EOF read on the 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.invalidmethod=Invalid character (CR or LF) found in method name +iib.invalidmethod=Invalid character found in method name. HTTP method names must be tokens 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/test/org/apache/coyote/http11/TestHttp11InputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestHttp11InputBuffer.java?rev=1743554&r1=1743553&r2=1743554&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/coyote/http11/TestHttp11InputBuffer.java (original) +++ tomcat/trunk/test/org/apache/coyote/http11/TestHttp11InputBuffer.java Thu May 12 19:56:17 2016 @@ -544,4 +544,61 @@ public class TestHttp11InputBuffer exten return true; } } + + + @Test + public void testInvalidMethod() { + + InvalidMethodClient client = new InvalidMethodClient(); + + client.doRequest(); + assertTrue(client.getResponseLine(), client.isResponse400()); + assertTrue(client.isResponseBodyOK()); + } + + + /** + * Bug 48839 test client. + */ + private class InvalidMethodClient extends SimpleHttpClient { + + private Exception doRequest() { + + Tomcat tomcat = getTomcatInstance(); + + tomcat.addContext("", TEMP_DIR); + + try { + tomcat.start(); + setPort(tomcat.getConnector().getLocalPort()); + + // Open connection + connect(); + + String[] request = new String[1]; + request[0] = + "GET" + (char) 0 + " /test HTTP/1.1" + CRLF + + "Host: localhost:8080" + 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; + } + return true; + } + } } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1743554&r1=1743553&r2=1743554&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Thu May 12 19:56:17 2016 @@ -45,6 +45,14 @@ issues do not "pop up" wrt. others). --> <section name="Tomcat 9.0.0.M7" rtext="in development"> + <subsection name="Coyote"> + <changelog> + <fix> + Ensure that requests with HTTP method names that are not tokens (as + required by RFC 7231) are rejected with a 400 response. (markt) + </fix> + </changelog> + </subsection> </section> <section name="Tomcat 9.0.0.M6" rtext="release in progress"> <subsection name="Catalina"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org