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 f4c3950 Process HTTP/0.9 requests with extra request line data as
HTTP/1.1
f4c3950 is described below
commit f4c39506d8b0d5e37ea9f404d16b236cd1f7de70
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Mar 24 09:50:02 2020 +0000
Process HTTP/0.9 requests with extra request line data as HTTP/1.1
https://bz.apache.org/bugzilla/show_bug.cgi?id=64240
---
.../apache/coyote/http11/Http11InputBuffer.java | 19 ++++--
.../coyote/http11/TestHttp11InputBufferCRLF.java | 73 +++++++++++++++++++---
webapps/docs/changelog.xml | 5 ++
3 files changed, 83 insertions(+), 14 deletions(-)
diff --git a/java/org/apache/coyote/http11/Http11InputBuffer.java
b/java/org/apache/coyote/http11/Http11InputBuffer.java
index 7be3e01..289a0bc 100644
--- a/java/org/apache/coyote/http11/Http11InputBuffer.java
+++ b/java/org/apache/coyote/http11/Http11InputBuffer.java
@@ -462,6 +462,9 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
request.protocol().setString(Constants.HTTP_11);
throw new
IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
}
+ if (chr == '<') {
+ System.out.println("debug");
+ }
if (chr == Constants.SP || chr == Constants.HT) {
space = true;
end = pos;
@@ -471,9 +474,10 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
// HTTP/0.9 style request
// Stop this processing loop
space = true;
+ // Set blank protocol (indicates HTTP/0.9)
+ request.protocol().setString("");
// Skip the protocol processing
- parsingRequestLinePhase = 6;
- parsingRequestLineEol = true;
+ parsingRequestLinePhase = 7;
if (prevChr == Constants.CR) {
end = pos - 1;
} else {
@@ -504,7 +508,9 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
request.requestURI().setBytes(byteBuffer.array(),
parsingRequestLineStart,
end - parsingRequestLineStart);
}
- if (!parsingRequestLineEol) {
+ // HTTP/0.9 processing jumps to stage 7.
+ // Don't want to overwrite that here.
+ if (parsingRequestLinePhase == 4) {
parsingRequestLinePhase = 5;
}
}
@@ -557,9 +563,12 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
if ((end - parsingRequestLineStart) > 0) {
request.protocol().setBytes(byteBuffer.array(),
parsingRequestLineStart,
end - parsingRequestLineStart);
- } else {
- request.protocol().setString("");
+ parsingRequestLinePhase = 7;
}
+ // If no protocol is found, the ISE below will be triggered.
+ }
+ if (parsingRequestLinePhase == 7) {
+ // Parsing is complete. Return and clean-up.
parsingRequestLine = false;
parsingRequestLinePhase = 0;
parsingRequestLineEol = false;
diff --git a/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
b/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
index 82315f5..ee033a1 100644
--- a/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
+++ b/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
@@ -45,32 +45,74 @@ public class TestHttp11InputBufferCRLF extends
TomcatBaseTest {
// Requests to exercise code that allows HT in place of SP
parameterSets.add(new Object[] { Boolean.FALSE, new String[] {
- "GET\thttp://localhost:8080/test\tHTTP/1.1"; + CRLF +
+ "GET\t/test\tHTTP/1.1" + CRLF +
"Host: localhost:8080" + CRLF +
- "Connection: close" + CRLF +
- CRLF } } );
+ "Connection: close" + CRLF +
+ CRLF }, Boolean.TRUE } );
// Requests to simulate package boundaries
// HTTP/0.9 request
addRequestWithSplits("GET /test" + CRLF, Boolean.TRUE, parameterSets);
+ // HTTP/0.9 request with space
+ // Either malformed but acceptable HTTP/0.9 or invalid HTTP/1.1
+ // Tomcat opts for invalid HTTP/1.1
+ addRequestWithSplits("GET /test " + CRLF, Boolean.FALSE,
Boolean.FALSE, parameterSets);
+
// HTTP/0.9 request (no optional CR)
addRequestWithSplits("GET /test" + LF, Boolean.TRUE, parameterSets);
+ // HTTP/0.9 request with space (no optional CR)
+ // Either malformed but acceptable HTTP/0.9 or invalid HTTP/1.1
+ // Tomcat opts for invalid HTTP/1.1
+ addRequestWithSplits("GET /test " + LF, Boolean.FALSE, Boolean.FALSE,
parameterSets);
+
// Standard HTTP/1.1 request
- addRequestWithSplits("GET http://localhost:8080/test HTTP/1.1" + CRLF +
+ addRequestWithSplits("GET /test HTTP/1.1" + CRLF +
+ "Host: localhost:8080" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF,
+ Boolean.FALSE, parameterSets);
+
+ // Invalid HTTP/1.1 request
+ addRequestWithSplits("GET /te<st HTTP/1.1" + CRLF +
+ "Host: localhost:8080" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF,
+ Boolean.FALSE, Boolean.FALSE, parameterSets);
+
+ // Standard HTTP/1.1 request with a query string
+ addRequestWithSplits("GET /test?a=b HTTP/1.1" + CRLF +
"Host: localhost:8080" + CRLF +
"Connection: close" + CRLF +
CRLF,
Boolean.FALSE, parameterSets);
+ // Standard HTTP/1.1 request with a query string that includes ?
+ addRequestWithSplits("GET /test?a=?b HTTP/1.1" + CRLF +
+ "Host: localhost:8080" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF,
+ Boolean.FALSE, parameterSets);
+
+ // Standard HTTP/1.1 request with an invalid query string
+ addRequestWithSplits("GET /test?a=<b HTTP/1.1" + CRLF +
+ "Host: localhost:8080" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF,
+ Boolean.FALSE, Boolean.FALSE, parameterSets);
+
return parameterSets;
}
private static void addRequestWithSplits(String request, Boolean isHttp09,
List<Object[]> parameterSets) {
+ addRequestWithSplits(request, isHttp09, Boolean.TRUE, parameterSets);
+ }
+
+ private static void addRequestWithSplits(String request, Boolean isHttp09,
Boolean valid, List<Object[]> parameterSets) {
// Add as a single String
- parameterSets.add(new Object[] { isHttp09, new String[] { request } }
);
+ parameterSets.add(new Object[] { isHttp09, new String[] { request },
valid });
// Add with all CRLF split between the CR and LF
List<String> parts = new ArrayList<>();
@@ -82,14 +124,14 @@ public class TestHttp11InputBufferCRLF extends
TomcatBaseTest {
pos = request.indexOf("\n", lastPos + 1);
}
parts.add(request.substring(lastPos));
- parameterSets.add(new Object[] { isHttp09, parts.toArray(new
String[0]) });
+ parameterSets.add(new Object[] { isHttp09, parts.toArray(new
String[0]), valid });
// Add with a split between each character
List<String> chars = new ArrayList<>();
for (char c : request.toCharArray()) {
chars.add(Character.toString(c));
}
- parameterSets.add(new Object[] { isHttp09, chars.toArray(new
String[0]) });
+ parameterSets.add(new Object[] { isHttp09, chars.toArray(new
String[0]), valid });
}
@Parameter(0)
@@ -98,13 +140,26 @@ public class TestHttp11InputBufferCRLF extends
TomcatBaseTest {
@Parameter(1)
public String[] request;
+ @Parameter(2)
+ public boolean valid;
+
+
@Test
public void testBug54947() {
Client client = new Client(request, isHttp09);
- client.doRequest();
- Assert.assertTrue(client.isResponseBodyOK());
+ Exception e = client.doRequest();
+
+ if (valid) {
+ Assert.assertTrue(client.isResponseBodyOK());
+ } else if (e == null){
+ Assert.assertTrue(client.isResponse400());
+ } else {
+ // The invalid request was detected before the entire request had
+ // been sent to Tomcat reset the connection when the client tried
to
+ // continue sending the invalid request.
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 1e8e357..14738ab 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -89,6 +89,11 @@
Avoid always retrieving the NIO poller selection key when processing
to reduce sync. (remm)
</fix>
+ <fix>
+ <bug>64240</bug>: Ensure that HTTP/0.9 requests that contain additional
+ data on the request line after the URI are treated consistently. Such
+ requests will now always be treated as HTTP/1.1. (markt)
+ </fix>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
