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 f15c077 Process HTTP/0.9 requests with extra request line data as
HTTP/1.1
f15c077 is described below
commit f15c077cdd1080d6f3d46be0d47f82978d9c03b9
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
---
.../coyote/http11/InternalAprInputBuffer.java | 13 ++--
.../apache/coyote/http11/InternalInputBuffer.java | 13 ++--
.../coyote/http11/InternalNioInputBuffer.java | 17 +++--
.../coyote/http11/TestHttp11InputBufferCRLF.java | 73 +++++++++++++++++++---
webapps/docs/changelog.xml | 5 ++
5 files changed, 99 insertions(+), 22 deletions(-)
diff --git a/java/org/apache/coyote/http11/InternalAprInputBuffer.java
b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
index b2e9a41..bfa769e 100644
--- a/java/org/apache/coyote/http11/InternalAprInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
@@ -238,8 +238,12 @@ public class InternalAprInputBuffer extends
AbstractInputBuffer<Long> {
// HTTP/0.9 style request. CR is optional. LF is not.
} else if (buf[pos] == Constants.LF) {
// HTTP/0.9 style request
- eol = true;
+ // Stop this processing loop
space = true;
+ // Set blank protocol (indicates HTTP/0.9)
+ request.protocol().setString("");
+ // Skip the protocol processing
+ eol = true;
if (buf[pos - 1] == Constants.CR) {
end = pos - 1;
} else {
@@ -314,12 +318,13 @@ public class InternalAprInputBuffer extends
AbstractInputBuffer<Long> {
if ((end - start) > 0) {
request.protocol().setBytes(buf, start, end - start);
- } else {
- request.protocol().setString("");
}
- return true;
+ if (request.protocol().isNull()) {
+ throw new
IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
+ }
+ return true;
}
diff --git a/java/org/apache/coyote/http11/InternalInputBuffer.java
b/java/org/apache/coyote/http11/InternalInputBuffer.java
index e3f36d6..1050e57 100644
--- a/java/org/apache/coyote/http11/InternalInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalInputBuffer.java
@@ -192,8 +192,12 @@ public class InternalInputBuffer extends
AbstractInputBuffer<Socket> {
// HTTP/0.9 style request. CR is optional. LF is not.
} else if (buf[pos] == Constants.LF) {
// HTTP/0.9 style request
- eol = true;
+ // Stop this processing loop
space = true;
+ // Set blank protocol (indicates HTTP/0.9)
+ request.protocol().setString("");
+ // Skip the protocol processing
+ eol = true;
if (buf[pos - 1] == Constants.CR) {
end = pos - 1;
} else {
@@ -266,12 +270,13 @@ public class InternalInputBuffer extends
AbstractInputBuffer<Socket> {
if ((end - start) > 0) {
request.protocol().setBytes(buf, start, end - start);
- } else {
- request.protocol().setString("");
}
- return true;
+ if (request.protocol().isNull()) {
+ throw new
IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
+ }
+ return true;
}
diff --git a/java/org/apache/coyote/http11/InternalNioInputBuffer.java
b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
index 6b80d01..3cdbdfa 100644
--- a/java/org/apache/coyote/http11/InternalNioInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
@@ -322,10 +322,12 @@ public class InternalNioInputBuffer extends
AbstractInputBuffer<NioChannel> {
// HTTP/0.9 style request. CR is optional. LF is not.
} else if (buf[pos] == Constants.LF) {
// HTTP/0.9 style request
- parsingRequestLineEol = true;
+ // Stop this processing loop
space = true;
+ // Set blank protocol (indicates HTTP/0.9)
+ request.protocol().setString("");
// Skip the protocol processing
- parsingRequestLinePhase = 6;
+ parsingRequestLinePhase = 7;
if (buf[pos - 1] == Constants.CR) {
end = pos - 1;
} else {
@@ -352,7 +354,9 @@ public class InternalNioInputBuffer extends
AbstractInputBuffer<NioChannel> {
} else {
request.requestURI().setBytes(buf, 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;
}
}
@@ -402,9 +406,12 @@ public class InternalNioInputBuffer extends
AbstractInputBuffer<NioChannel> {
if ( (end - parsingRequestLineStart) > 0) {
request.protocol().setBytes(buf, 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 1acb03e..d674ac6 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<String>();
@@ -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<String>();
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 b6f6239..5c75e31 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -75,6 +75,11 @@
<code>URIEncoding</code> is not a superset of US-ASCII as required by
RFC7230. (markt)
</add>
+ <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
