Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=46984

CR & LF in middle of method name should cause a 400
So client sees 400, stop processing the request once we know it is bad

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@763654 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/coyote/http11/Http11AprProcessor.java b/java/org/apache/coyote/http11/Http11AprProcessor.java
index 626367b..2276785 100644 (file)
--- a/java/org/apache/coyote/http11/Http11AprProcessor.java
+++ b/java/org/apache/coyote/http11/Http11AprProcessor.java
@@ -818,17 +818,19 @@ public class Http11AprProcessor implements ActionHook {
                 error = true;
             }
 
-            // Setting up filters, and parse some request headers
-            rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
-            try {
-                prepareRequest();
-            } catch (Throwable t) {
-                if (log.isDebugEnabled()) {
-                    log.debug(sm.getString("http11processor.request.prepare"), t);
+            if (!error) {
+                // Setting up filters, and parse some request headers
+                rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
+                try {
+                    prepareRequest();
+                } catch (Throwable t) {
+                    if (log.isDebugEnabled()) {
+                        log.debug(sm.getString("http11processor.request.prepare"), t);
+                    }
+                    // 400 - Internal Server Error
+                    response.setStatus(400);
+                    error = true;
                 }
-                // 400 - Internal Server Error
-                response.setStatus(400);
-                error = true;
             }
 
             if (maxKeepAliveRequests > 0 && --keepAliveLeft == 0)

diff --git a/java/org/apache/coyote/http11/Http11NioProcessor.java b/java/org/apache/coyote/http11/Http11NioProcessor.java
index 3de3453..6b188df 100644 (file)
--- a/java/org/apache/coyote/http11/Http11NioProcessor.java
+++ b/java/org/apache/coyote/http11/Http11NioProcessor.java
@@ -849,17 +849,19 @@ public class Http11NioProcessor implements ActionHook {
                 error = true;
             }
 
-            // Setting up filters, and parse some request headers
-            rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
-            try {
-                prepareRequest();
-            } catch (Throwable t) {
-                if (log.isDebugEnabled()) {
-                    log.debug(sm.getString("http11processor.request.prepare"), t);
+            if (!error) {
+                // Setting up filters, and parse some request headers
+                rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
+                try {
+                    prepareRequest();
+                } catch (Throwable t) {
+                    if (log.isDebugEnabled()) {
+                        log.debug(sm.getString("http11processor.request.prepare"), t);
+                    }
+                    // 400 - Internal Server Error
+                    response.setStatus(400);
+                    error = true;
                 }
-                // 400 - Internal Server Error
-                response.setStatus(400);
-                error = true;
             }
             
             if (maxKeepAliveRequests == 1 )

diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java
index d629b07..7eed3fc 100644 (file)
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -807,17 +807,19 @@ public class Http11Processor implements ActionHook {
                 error = true;
             }
 
-            // Setting up filters, and parse some request headers
-            rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
-            try {
-                prepareRequest();
-            } catch (Throwable t) {
-                if (log.isDebugEnabled()) {
-                    log.debug(sm.getString("http11processor.request.prepare"), t);
+            if (!error) {
+                // Setting up filters, and parse some request headers
+                rp.setStage(org.apache.coyote.Constants.STAGE_PREPARE);
+                try {
+                    prepareRequest();
+                } catch (Throwable t) {
+                    if (log.isDebugEnabled()) {
+                        log.debug(sm.getString("http11processor.request.prepare"), t);
+                    }
+                    // 400 - Internal Server Error
+                    response.setStatus(400);
+                    error = true;
                 }
-                // 400 - Internal Server Error
-                response.setStatus(400);
-                error = true;
             }
 
             if (maxKeepAliveRequests > 0 && --keepAliveLeft == 0)

diff --git a/java/org/apache/coyote/http11/InternalAprInputBuffer.java b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
index a00ce8c..0b1e231 100644 (file)
--- a/java/org/apache/coyote/http11/InternalAprInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
@@ -403,6 +403,11 @@ public class InternalAprInputBuffer implements InputBuffer {
                     throw new EOFException(sm.getString("iib.eof.error"));
             }
 
+            // 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 single SP but it also says be tolerant of HT
             if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) {
                 space = true;

diff --git a/java/org/apache/coyote/http11/InternalInputBuffer.java b/java/org/apache/coyote/http11/InternalInputBuffer.java
index 326b4c3..416364d 100644 (file)
--- a/java/org/apache/coyote/http11/InternalInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalInputBuffer.java
@@ -391,6 +391,11 @@ public class InternalInputBuffer implements InputBuffer {
                     throw new EOFException(sm.getString("iib.eof.error"));
             }
 
+            // 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 single SP but it also says be tolerant of HT
             if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) {
                 space = true;

diff --git a/java/org/apache/coyote/http11/InternalNioInputBuffer.java b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
index 3438d36..d10ae0a 100644 (file)
--- a/java/org/apache/coyote/http11/InternalNioInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
@@ -454,6 +454,11 @@ public class InternalNioInputBuffer implements InputBuffer {
                     if (!fill(true, 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"));
+                }
                 if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) {
                     space = true;
                     request.method().setBytes(buf, parsingRequestLineStart, pos - parsingRequestLineStart);

diff --git a/java/org/apache/coyote/http11/LocalStrings.properties b/java/org/apache/coyote/http11/LocalStrings.properties
index 8fb7244..8c47a61 100644 (file)
--- a/java/org/apache/coyote/http11/LocalStrings.properties
+++ b/java/org/apache/coyote/http11/LocalStrings.properties
@@ -63,4 +63,5 @@ http11processor.socket.timeout=Error setting socket timeout
 
 iib.eof.error=Unexpected EOF read on the socket
 iib.requestheadertoolarge.error=Request header is too large
+iib.invalidmethod=Invalid character (CR or LF) found in method name