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/05/12 19:56:17 UTC

svn commit: r1743554 - in /tomcat/trunk: java/org/apache/coyote/http11/Http11InputBuffer.java java/org/apache/coyote/http11/LocalStrings.properties test/org/apache/coyote/http11/TestHttp11InputBuffer.java webapps/docs/changelog.xml

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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org