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 15:13:14 UTC
[tomcat] branch 7.0.x 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 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/7.0.x by this push:
new 3834041 Improve reporting of invalid request lines
3834041 is described below
commit 3834041925d549cab2562ff8a146e8e9134bd023
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Apr 15 16:12:55 2020 +0100
Improve reporting of invalid request lines
---
java/org/apache/coyote/http11/AbstractInputBuffer.java | 16 ++++++++++++++++
.../org/apache/coyote/http11/InternalAprInputBuffer.java | 15 ++++++++++-----
java/org/apache/coyote/http11/InternalInputBuffer.java | 15 ++++++++++-----
.../org/apache/coyote/http11/InternalNioInputBuffer.java | 15 ++++++++++-----
java/org/apache/coyote/http11/LocalStrings.properties | 6 +++---
webapps/docs/changelog.xml | 4 ++++
6 files changed, 53 insertions(+), 18 deletions(-)
diff --git a/java/org/apache/coyote/http11/AbstractInputBuffer.java b/java/org/apache/coyote/http11/AbstractInputBuffer.java
index 7f9b856..b08194b 100644
--- a/java/org/apache/coyote/http11/AbstractInputBuffer.java
+++ b/java/org/apache/coyote/http11/AbstractInputBuffer.java
@@ -21,6 +21,7 @@ import java.io.IOException;
import org.apache.coyote.InputBuffer;
import org.apache.coyote.Request;
import org.apache.tomcat.util.buf.ByteChunk;
+import org.apache.tomcat.util.http.HeaderUtil;
import org.apache.tomcat.util.http.MimeHeaders;
import org.apache.tomcat.util.http.parser.HttpParser;
import org.apache.tomcat.util.net.AbstractEndpoint;
@@ -183,6 +184,21 @@ public abstract class AbstractInputBuffer<S> implements InputBuffer{
}
+ protected String parseInvalid(int startPos, byte[] buffer) {
+ // Look for the next space
+ int pos = startPos;
+ while (pos < lastValid && buffer[pos] != 0x20) {
+ pos++;
+ }
+ String result = HeaderUtil.toPrintableString(buffer, startPos, pos - startPos);
+ if (pos == lastValid) {
+ // Ran out of buffer rather than found a space
+ result = result + "...";
+ }
+ return result;
+ }
+
+
/**
* Implementations are expected to call {@link Request#setStartTime(long)}
* as soon as the first byte is read from the request.
diff --git a/java/org/apache/coyote/http11/InternalAprInputBuffer.java b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
index bfa769e..6f153a9 100644
--- a/java/org/apache/coyote/http11/InternalAprInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
@@ -182,7 +182,8 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
space = true;
request.method().setBytes(buf, start, pos - start);
} else if (!HttpParser.isToken(buf[pos])) {
- throw new IllegalArgumentException(sm.getString("iib.invalidmethod"));
+ String invalidMethodValue = parseInvalid(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidmethod", invalidMethodValue));
}
pos++;
@@ -227,7 +228,8 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
// 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(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
}
// Spec says single SP but it also says be tolerant of HT
@@ -253,12 +255,14 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
questionPos = pos;
} else if (questionPos != -1 && !httpParser.isQueryRelaxed(buf[pos])) {
// %nn decoding will be checked at the point of decoding
- throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
+ String invalidRequestTarget = parseInvalid(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
} else if (httpParser.isNotRequestTargetRelaxed(buf[pos])) {
// This is a general check that aims to catch problems early
// Detailed checking of each part of the request target will
// happen in AbstractHttp11Processor#prepareRequest()
- throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
+ String invalidRequestTarget = parseInvalid(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
}
pos++;
}
@@ -309,7 +313,8 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
end = pos - 1;
eol = true;
} else if (!HttpParser.isHttpProtocol(buf[pos])) {
- throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
+ String invalidProtocol = parseInvalid(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol", invalidProtocol));
}
pos++;
diff --git a/java/org/apache/coyote/http11/InternalInputBuffer.java b/java/org/apache/coyote/http11/InternalInputBuffer.java
index 1050e57..4782d50 100644
--- a/java/org/apache/coyote/http11/InternalInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalInputBuffer.java
@@ -136,7 +136,8 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
space = true;
request.method().setBytes(buf, start, pos - start);
} else if (!HttpParser.isToken(buf[pos])) {
- throw new IllegalArgumentException(sm.getString("iib.invalidmethod"));
+ String invalidMethodValue = parseInvalid(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidmethod", invalidMethodValue));
}
pos++;
@@ -181,7 +182,8 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
// 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(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
}
// Spec says single SP but it also says be tolerant of HT
@@ -207,12 +209,14 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
questionPos = pos;
} else if (questionPos != -1 && !httpParser.isQueryRelaxed(buf[pos])) {
// %nn decoding will be checked at the point of decoding
- throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
+ String invalidRequestTarget = parseInvalid(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
} else if (httpParser.isNotRequestTargetRelaxed(buf[pos])) {
// This is a general check that aims to catch problems early
// Detailed checking of each part of the request target will
// happen in AbstractHttp11Processor#prepareRequest()
- throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
+ String invalidRequestTarget = parseInvalid(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
}
pos++;
}
@@ -261,7 +265,8 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
end = pos - 1;
eol = true;
} else if (!HttpParser.isHttpProtocol(buf[pos])) {
- throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
+ String invalidProtocol = parseInvalid(start, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol", invalidProtocol));
}
pos++;
diff --git a/java/org/apache/coyote/http11/InternalNioInputBuffer.java b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
index 3cdbdfa..1da4c4c 100644
--- a/java/org/apache/coyote/http11/InternalNioInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
@@ -267,7 +267,8 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
space = true;
request.method().setBytes(buf, parsingRequestLineStart, pos - parsingRequestLineStart);
} else if (!HttpParser.isToken(buf[pos])) {
- throw new IllegalArgumentException(sm.getString("iib.invalidmethod"));
+ String invalidMethodValue = parseInvalid(parsingRequestLineStart, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidmethod", invalidMethodValue));
}
pos++;
}
@@ -311,7 +312,8 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
// 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, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
}
// Spec says single SP but it also says be tolerant of HT
@@ -337,12 +339,14 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
parsingRequestLineQPos = pos;
} else if (parsingRequestLineQPos != -1 && !httpParser.isQueryRelaxed(buf[pos])) {
// %nn decoding will be checked at the point of decoding
- throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
+ String invalidRequestTarget = parseInvalid(parsingRequestLineStart, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
} else if (httpParser.isNotRequestTargetRelaxed(buf[pos])) {
// This is a general check that aims to catch problems early
// Detailed checking of each part of the request target will
// happen in AbstractHttp11Processor#prepareRequest()
- throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
+ String invalidRequestTarget = parseInvalid(parsingRequestLineStart, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget", invalidRequestTarget));
}
pos++;
}
@@ -399,7 +403,8 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
end = pos - 1;
parsingRequestLineEol = true;
} else if (!HttpParser.isHttpProtocol(buf[pos])) {
- throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
+ String invalidProtocol = parseInvalid(parsingRequestLineStart, buf);
+ throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol", invalidProtocol));
}
pos++;
}
diff --git a/java/org/apache/coyote/http11/LocalStrings.properties b/java/org/apache/coyote/http11/LocalStrings.properties
index d31f697..3a05ede 100644
--- a/java/org/apache/coyote/http11/LocalStrings.properties
+++ b/java/org/apache/coyote/http11/LocalStrings.properties
@@ -53,11 +53,11 @@ http11protocol.start=Starting Coyote HTTP/1.1 on [{0}]
iib.apr.sslGeneralError=An APR general error was returned by the SSL read operation on APR/native socket [{0}] with wrapper [{1}]. It will be treated as EAGAIN and the socket returned to the poller.
iib.eof.error=Unexpected EOF read on the socket
-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.requestheadertoolarge.error=Request header is too large
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 65cb37c..27af2ad 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -107,6 +107,10 @@
to the application without decoding it in addition to rejecting such
sequences and decoding such sequences. (markt)
</add>
+ <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="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org