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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
