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 2020/04/15 14:36:41 UTC

[tomcat] branch master updated: Improve reporting of invalid request lines

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/master by this push:
     new 71a0ec7  Improve reporting of invalid request lines
71a0ec7 is described below

commit 71a0ec7f5cefefd80b98c246b215f66efc63211f
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Apr 15 15:35:00 2020 +0100

    Improve reporting of invalid request lines
---
 .../apache/coyote/http11/Http11InputBuffer.java    | 31 +++++++++++++++++-----
 .../apache/coyote/http11/LocalStrings.properties   |  6 ++---
 webapps/docs/changelog.xml                         |  4 +++
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/java/org/apache/coyote/http11/Http11InputBuffer.java b/java/org/apache/coyote/http11/Http11InputBuffer.java
index 2794bfb..8619a32 100644
--- a/java/org/apache/coyote/http11/Http11InputBuffer.java
+++ b/java/org/apache/coyote/http11/Http11InputBuffer.java
@@ -412,10 +412,10 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler
                     request.method().setBytes(byteBuffer.array(), parsingRequestLineStart,
                             pos - parsingRequestLineStart);
                 } else if (!HttpParser.isToken(chr)) {
-                    byteBuffer.position(byteBuffer.position() - 1);
                     // Avoid unknown protocol triggering an additional error
                     request.protocol().setString(Constants.HTTP_11);
-                    throw new IllegalArgumentException(sm.getString("iib.invalidmethod"));
+                    String invalidMethodValue = parseInvalid(parsingRequestLineStart, byteBuffer);
+                    throw new IllegalArgumentException(sm.getString("iib.invalidmethod", invalidMethodValue));
                 }
             }
             parsingRequestLinePhase = 3;
@@ -460,7 +460,8 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler
                     // therefore invalid. Trigger error handling.
                     // Avoid unknown protocol triggering an additional error
                     request.protocol().setString(Constants.HTTP_11);
-                    throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
+                    String invalidRequestTarget = parseInvalid(parsingRequestLineStart, byteBuffer);
+                    throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
                 }
                 if (chr == Constants.SP || chr == Constants.HT) {
                     space = true;
@@ -486,14 +487,16 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler
                     // Avoid unknown protocol triggering an additional error
                     request.protocol().setString(Constants.HTTP_11);
                     // %nn decoding will be checked at the point of decoding
-                    throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
+                    String invalidRequestTarget = parseInvalid(parsingRequestLineStart, byteBuffer);
+                    throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
                 } else if (httpParser.isNotRequestTargetRelaxed(chr)) {
                     // Avoid unknown protocol triggering an additional error
                     request.protocol().setString(Constants.HTTP_11);
                     // This is a general check that aims to catch problems early
                     // Detailed checking of each part of the request target will
                     // happen in Http11Processor#prepareRequest()
-                    throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
+                    String invalidRequestTarget = parseInvalid(parsingRequestLineStart, byteBuffer);
+                    throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
                 }
             }
             if (parsingRequestLineQPos >= 0) {
@@ -553,7 +556,8 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler
                     end = pos - 1;
                     parsingRequestLineEol = true;
                 } else if (!HttpParser.isHttpProtocol(chr)) {
-                    throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
+                    String invalidProtocol = parseInvalid(parsingRequestLineStart, byteBuffer);
+                    throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol", invalidProtocol));
                 }
             }
 
@@ -615,6 +619,21 @@ public class Http11InputBuffer implements InputBuffer, ApplicationBufferHandler
     }
 
 
+    private String parseInvalid(int startPos, ByteBuffer buffer) {
+        // Look for the next space
+        byte b = 0;
+        while (buffer.hasRemaining() && b != 0x20) {
+            b = buffer.get();
+        }
+        String result = HeaderUtil.toPrintableString(buffer.array(), buffer.arrayOffset() + startPos, buffer.position() - startPos - 1);
+        if (b != 0x20) {
+            // Ran out of buffer rather than found a space
+            result = result + "...";
+        }
+        return result;
+    }
+
+
     /**
      * End request (consumes leftover bytes).
      *
diff --git a/java/org/apache/coyote/http11/LocalStrings.properties b/java/org/apache/coyote/http11/LocalStrings.properties
index d744815..724a99b 100644
--- a/java/org/apache/coyote/http11/LocalStrings.properties
+++ b/java/org/apache/coyote/http11/LocalStrings.properties
@@ -40,11 +40,11 @@ iib.available.readFail=A non-blocking read failed while attempting to determine
 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.invalidHttpProtocol=Invalid character found in the HTTP protocol
+iib.invalidHttpProtocol=Invalid character found in the HTTP protocol [{0}]
 iib.invalidPhase=Invalid request line parse phase [{0}]
-iib.invalidRequestTarget=Invalid character found in the request target. The valid characters are defined in RFC 7230 and RFC 3986
+iib.invalidRequestTarget=Invalid character found in the request target [{0}]. The valid characters are defined in RFC 7230 and RFC 3986
 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.invalidmethod=Invalid character found in method name [{0}]. 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
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 5f1bb84..29628ab 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -71,6 +71,10 @@
         that use a custom class loader that loads resources from non-standard
         locations. (markt)
       </fix>
+      <fix>
+        Include the problematic data in the error message when reporting that
+        the provided request line contains an invalid component. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org