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/03/10 21:32:43 UTC
[tomcat] 03/03: Fix BZ 64210. Correct parsing of CRLF across packet
boundaries,
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
commit 8f03960ca7e6e7a4b5ca2bd4c4daea45583b3e59
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Mar 10 21:32:16 2020 +0000
Fix BZ 64210. Correct parsing of CRLF across packet boundaries,
Also improve request line parsing including HTTP/0.9.
---
.../coyote/http11/AbstractHttp11Processor.java | 43 +++---
.../apache/coyote/http11/AbstractInputBuffer.java | 6 +-
.../coyote/http11/InternalAprInputBuffer.java | 55 ++++----
.../apache/coyote/http11/InternalInputBuffer.java | 53 ++++---
.../coyote/http11/InternalNioInputBuffer.java | 51 ++++---
.../coyote/http11/TestHttp11InputBufferCRLF.java | 156 +++++++++++++++++++++
.../coyote/http11/TestInternalInputBuffer.java | 101 +++++++++++--
webapps/docs/changelog.xml | 17 ++-
8 files changed, 371 insertions(+), 111 deletions(-)
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Processor.java b/java/org/apache/coyote/http11/AbstractHttp11Processor.java
index 8453067..33c0b27 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Processor.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Processor.java
@@ -1111,6 +1111,11 @@ public abstract class AbstractHttp11Processor<S> extends AbstractProcessor<S> {
}
}
+ // Process the Protocol component of the request line
+ // Need to know if this is an HTTP 0.9 request before trying to
+ // parse headers.
+ prepareRequestProtocol();
+
if (endpoint.isPaused()) {
// 503 - Service unavailable
response.setStatus(503);
@@ -1121,7 +1126,8 @@ public abstract class AbstractHttp11Processor<S> extends AbstractProcessor<S> {
request.getMimeHeaders().setLimit(endpoint.getMaxHeaderCount());
request.getCookies().setLimit(getMaxCookieCount());
// Currently only NIO will ever return false here
- if (!getInputBuffer().parseHeaders()) {
+ // Don't parse headers for HTTP/0.9
+ if (!http09 && !getInputBuffer().parseHeaders()) {
// We've read part of the request, don't recycle it
// instead associate it with the socket
openSocket = true;
@@ -1320,26 +1326,14 @@ public abstract class AbstractHttp11Processor<S> extends AbstractProcessor<S> {
}
- /**
- * After reading the request headers, we have to setup the request filters.
- */
- protected void prepareRequest() throws IOException {
-
- http11 = true;
- http09 = false;
- contentDelimitation = false;
- expectation = false;
-
- prepareRequestInternal();
-
- if (endpoint.isSSLEnabled()) {
- request.scheme().setString("https");
- }
+ private void prepareRequestProtocol() {
MessageBytes protocolMB = request.protocol();
if (protocolMB.equals(Constants.HTTP_11)) {
+ http09 = false;
http11 = true;
protocolMB.setString(Constants.HTTP_11);
} else if (protocolMB.equals(Constants.HTTP_10)) {
+ http09 = false;
http11 = false;
keepAlive = false;
protocolMB.setString(Constants.HTTP_10);
@@ -1350,6 +1344,7 @@ public abstract class AbstractHttp11Processor<S> extends AbstractProcessor<S> {
keepAlive = false;
} else {
// Unsupported protocol
+ http09 = false;
http11 = false;
// Send 505; Unsupported HTTP version
response.setStatus(505);
@@ -1359,6 +1354,22 @@ public abstract class AbstractHttp11Processor<S> extends AbstractProcessor<S> {
" Unsupported HTTP version \""+protocolMB+"\"");
}
}
+ }
+
+
+ /**
+ * After reading the request headers, we have to setup the request filters.
+ */
+ protected void prepareRequest() throws IOException {
+
+ contentDelimitation = false;
+ expectation = false;
+
+ prepareRequestInternal();
+
+ if (endpoint.isSSLEnabled()) {
+ request.scheme().setString("https");
+ }
MessageBytes methodMB = request.method();
if (methodMB.equals(Constants.GET)) {
diff --git a/java/org/apache/coyote/http11/AbstractInputBuffer.java b/java/org/apache/coyote/http11/AbstractInputBuffer.java
index 379fab5..7f9b856 100644
--- a/java/org/apache/coyote/http11/AbstractInputBuffer.java
+++ b/java/org/apache/coyote/http11/AbstractInputBuffer.java
@@ -117,11 +117,12 @@ public abstract class AbstractInputBuffer<S> implements InputBuffer{
protected HttpParser httpParser;
+ protected byte prevChr = 0;
+ protected byte chr = 0;
// ------------------------------------------------------------- Properties
-
/**
* Add an input filter to the filter library.
*/
@@ -214,12 +215,13 @@ public abstract class AbstractInputBuffer<S> implements InputBuffer{
activeFilters[i].recycle();
}
+ prevChr = 0;
+ chr = 0;
lastValid = 0;
pos = 0;
lastActiveFilter = -1;
parsingHeader = true;
swallowInput = true;
-
}
diff --git a/java/org/apache/coyote/http11/InternalAprInputBuffer.java b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
index 72850ba..b2e9a41 100644
--- a/java/org/apache/coyote/http11/InternalAprInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
@@ -130,7 +130,6 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
// Skipping blank lines
//
- byte chr = 0;
do {
// Read new bytes if needed
@@ -147,7 +146,7 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
request.setStartTime(System.currentTimeMillis());
}
chr = buf[pos++];
- } while ((chr == Constants.CR) || (chr == Constants.LF));
+ } while (chr == Constants.CR || chr == Constants.LF);
pos--;
@@ -223,16 +222,29 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
throw new EOFException(sm.getString("iib.eof.error"));
}
+ if (buf[pos -1] == Constants.CR && buf[pos] != Constants.LF) {
+ // CR not followed by LF so not an HTTP/0.9 request and
+ // 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"));
+ }
+
// Spec says single SP but it also says be tolerant of HT
if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) {
space = true;
end = pos;
- } else if ((buf[pos] == Constants.CR)
- || (buf[pos] == Constants.LF)) {
+ } else if (buf[pos] == Constants.CR) {
+ // HTTP/0.9 style request. CR is optional. LF is not.
+ } else if (buf[pos] == Constants.LF) {
// HTTP/0.9 style request
eol = true;
space = true;
- end = pos;
+ if (buf[pos - 1] == Constants.CR) {
+ end = pos - 1;
+ } else {
+ end = pos;
+ }
} else if ((buf[pos] == Constants.QUESTION) && (questionPos == -1)) {
questionPos = pos;
} else if (questionPos != -1 && !httpParser.isQueryRelaxed(buf[pos])) {
@@ -244,11 +256,8 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
// happen in AbstractHttp11Processor#prepareRequest()
throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
}
-
pos++;
-
}
-
request.unparsedURI().setBytes(buf, start, end - start);
if (questionPos >= 0) {
request.queryString().setBytes(buf, questionPos + 1,
@@ -259,7 +268,7 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
}
// Spec says single SP but also says be tolerant of multiple and/or HT
- while (space) {
+ while (space && !eol) {
// Read new bytes if needed
if (pos >= lastValid) {
if (!fill())
@@ -291,10 +300,9 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
}
if (buf[pos] == Constants.CR) {
- end = pos;
- } else if (buf[pos] == Constants.LF) {
- if (end == 0)
- end = pos;
+ // Possible end of request line. Need LF next.
+ } else if (buf[pos - 1] == Constants.CR && buf[pos] == Constants.LF) {
+ end = pos - 1;
eol = true;
} else if (!HttpParser.isHttpProtocol(buf[pos])) {
throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
@@ -343,15 +351,7 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
* HTTP header parsing is done
*/
@SuppressWarnings("null") // headerValue cannot be null
- private boolean parseHeader()
- throws IOException {
-
- //
- // Check for blank line
- //
-
- byte chr = 0;
- byte prevChr = 0;
+ private boolean parseHeader() throws IOException {
while (true) {
@@ -367,7 +367,7 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
if (chr == Constants.CR && prevChr != Constants.CR) {
// Possible start of CRLF - process the next byte.
} else if (prevChr == Constants.CR && chr == Constants.LF) {
- pos++;
+ pos++;
return false;
} else {
if (prevChr == Constants.CR) {
@@ -504,14 +504,14 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
throw new EOFException(sm.getString("iib.eof.error"));
}
- chr = buf[pos];
- if ((chr != Constants.SP) && (chr != Constants.HT)) {
+ byte peek = buf[pos];
+ if (peek != Constants.SP && peek != Constants.HT) {
validLine = false;
} else {
eol = false;
// Copying one extra space in the buffer (since there must
// be at least one space inserted between the lines)
- buf[realPos] = chr;
+ buf[realPos] = peek;
realPos++;
}
@@ -532,9 +532,6 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
lastRealByte = pos - 1;
}
- byte chr = 0;
- byte prevChr = 0;
-
while (!eol) {
// Read new bytes if needed
diff --git a/java/org/apache/coyote/http11/InternalInputBuffer.java b/java/org/apache/coyote/http11/InternalInputBuffer.java
index d1f9e48..e3f36d6 100644
--- a/java/org/apache/coyote/http11/InternalInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalInputBuffer.java
@@ -95,7 +95,6 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
// Skipping blank lines
//
- byte chr = 0;
do {
// Read new bytes if needed
@@ -109,7 +108,7 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
request.setStartTime(System.currentTimeMillis());
}
chr = buf[pos++];
- } while ((chr == Constants.CR) || (chr == Constants.LF));
+ } while (chr == Constants.CR || chr == Constants.LF);
pos--;
@@ -177,16 +176,29 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
throw new EOFException(sm.getString("iib.eof.error"));
}
+ if (buf[pos -1] == Constants.CR && buf[pos] != Constants.LF) {
+ // CR not followed by LF so not an HTTP/0.9 request and
+ // 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"));
+ }
+
// Spec says single SP but it also says be tolerant of HT
if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) {
space = true;
end = pos;
- } else if ((buf[pos] == Constants.CR)
- || (buf[pos] == Constants.LF)) {
+ } else if (buf[pos] == Constants.CR) {
+ // HTTP/0.9 style request. CR is optional. LF is not.
+ } else if (buf[pos] == Constants.LF) {
// HTTP/0.9 style request
eol = true;
space = true;
- end = pos;
+ if (buf[pos - 1] == Constants.CR) {
+ end = pos - 1;
+ } else {
+ end = pos;
+ }
} else if ((buf[pos] == Constants.QUESTION) && (questionPos == -1)) {
questionPos = pos;
} else if (questionPos != -1 && !httpParser.isQueryRelaxed(buf[pos])) {
@@ -198,11 +210,8 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
// happen in AbstractHttp11Processor#prepareRequest()
throw new IllegalArgumentException(sm.getString("iib.invalidRequestTarget"));
}
-
pos++;
-
}
-
request.unparsedURI().setBytes(buf, start, end - start);
if (questionPos >= 0) {
request.queryString().setBytes(buf, questionPos + 1,
@@ -213,7 +222,7 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
}
// Spec says single SP but also says be tolerant of multiple SP and/or HT
- while (space) {
+ while (space && !eol) {
// Read new bytes if needed
if (pos >= lastValid) {
if (!fill())
@@ -243,10 +252,9 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
}
if (buf[pos] == Constants.CR) {
- end = pos;
- } else if (buf[pos] == Constants.LF) {
- if (end == 0)
- end = pos;
+ // Possible end of request line. Need LF next.
+ } else if (buf[pos - 1] == Constants.CR && buf[pos] == Constants.LF) {
+ end = pos - 1;
eol = true;
} else if (!HttpParser.isHttpProtocol(buf[pos])) {
throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
@@ -295,15 +303,7 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
* HTTP header parsing is done
*/
@SuppressWarnings("null") // headerValue cannot be null
- private boolean parseHeader()
- throws IOException {
-
- //
- // Check for blank line
- //
-
- byte chr = 0;
- byte prevChr = 0;
+ private boolean parseHeader() throws IOException {
while (true) {
@@ -457,14 +457,14 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
throw new EOFException(sm.getString("iib.eof.error"));
}
- chr = buf[pos];
- if ((chr != Constants.SP) && (chr != Constants.HT)) {
+ byte peek = buf[pos];
+ if (peek != Constants.SP && peek != Constants.HT) {
validLine = false;
} else {
eol = false;
// Copying one extra space in the buffer (since there must
// be at least one space inserted between the lines)
- buf[realPos] = chr;
+ buf[realPos] = peek;
realPos++;
}
@@ -503,9 +503,6 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
lastRealByte = pos - 1;
}
- byte chr = 0;
- byte prevChr = 0;
-
while (!eol) {
// Read new bytes if needed
diff --git a/java/org/apache/coyote/http11/InternalNioInputBuffer.java b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
index 9cf0dd0..6b80d01 100644
--- a/java/org/apache/coyote/http11/InternalNioInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
@@ -220,7 +220,6 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
// Skipping blank lines
//
if ( parsingRequestLinePhase == 0 ) {
- byte chr = 0;
do {
// Read new bytes if needed
@@ -306,15 +305,32 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
if (!fill(true,false)) //request line parsing
return false;
}
+
+ if (buf[pos -1] == Constants.CR && buf[pos] != Constants.LF) {
+ // CR not followed by LF so not an HTTP/0.9 request and
+ // 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"));
+ }
+
+ // Spec says single SP but it also says be tolerant of HT
if (buf[pos] == Constants.SP || buf[pos] == Constants.HT) {
space = true;
end = pos;
- } else if ((buf[pos] == Constants.CR)
- || (buf[pos] == Constants.LF)) {
+ } else if (buf[pos] == Constants.CR) {
+ // HTTP/0.9 style request. CR is optional. LF is not.
+ } else if (buf[pos] == Constants.LF) {
// HTTP/0.9 style request
parsingRequestLineEol = true;
space = true;
- end = pos;
+ // Skip the protocol processing
+ parsingRequestLinePhase = 6;
+ if (buf[pos - 1] == Constants.CR) {
+ end = pos - 1;
+ } else {
+ end = pos;
+ }
} else if ((buf[pos] == Constants.QUESTION) && (parsingRequestLineQPos == -1)) {
parsingRequestLineQPos = pos;
} else if (parsingRequestLineQPos != -1 && !httpParser.isQueryRelaxed(buf[pos])) {
@@ -336,7 +352,9 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
} else {
request.requestURI().setBytes(buf, parsingRequestLineStart, end - parsingRequestLineStart);
}
- parsingRequestLinePhase = 5;
+ if (!parsingRequestLineEol) {
+ parsingRequestLinePhase = 5;
+ }
}
if ( parsingRequestLinePhase == 5 ) {
// Spec says single SP but also be tolerant of multiple and/or HT
@@ -372,10 +390,9 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
}
if (buf[pos] == Constants.CR) {
- end = pos;
- } else if (buf[pos] == Constants.LF) {
- if (end == 0)
- end = pos;
+ // Possible end of request line. Need LF next.
+ } else if (buf[pos - 1] == Constants.CR && buf[pos] == Constants.LF) {
+ end = pos - 1;
parsingRequestLineEol = true;
} else if (!HttpParser.isHttpProtocol(buf[pos])) {
throw new IllegalArgumentException(sm.getString("iib.invalidHttpProtocol"));
@@ -510,13 +527,6 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
private HeaderParseStatus parseHeader()
throws IOException {
- //
- // Check for blank line
- //
-
- byte chr = 0;
- byte prevChr = 0;
-
while (headerParsePos == HeaderParsePosition.HEADER_START) {
// Read new bytes if needed
@@ -683,15 +693,15 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
}
}
- chr = buf[pos];
+ byte peek = buf[pos];
if ( headerParsePos == HeaderParsePosition.HEADER_MULTI_LINE ) {
- if ( (chr != Constants.SP) && (chr != Constants.HT)) {
+ if (peek != Constants.SP && peek != Constants.HT) {
headerParsePos = HeaderParsePosition.HEADER_START;
break;
} else {
// Copying one extra space in the buffer (since there must
// be at least one space inserted between the lines)
- buf[headerData.realPos] = chr;
+ buf[headerData.realPos] = peek;
headerData.realPos++;
headerParsePos = HeaderParsePosition.HEADER_VALUE_START;
}
@@ -712,9 +722,6 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
headerParsePos = HeaderParsePosition.HEADER_SKIPLINE;
boolean eol = false;
- byte chr = 0;
- byte prevChr = 0;
-
// Reading bytes until the end of the line
while (!eol) {
diff --git a/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java b/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
new file mode 100644
index 0000000..1acb03e
--- /dev/null
+++ b/test/org/apache/coyote/http11/TestHttp11InputBufferCRLF.java
@@ -0,0 +1,156 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.coyote.http11;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.startup.SimpleHttpClient;
+import org.apache.catalina.startup.TesterServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+
+@RunWith(Parameterized.class)
+public class TestHttp11InputBufferCRLF extends TomcatBaseTest {
+
+ private static final String CR = "\r";
+ private static final String LF = "\n";
+ private static final String CRLF = CR + LF;
+
+ @Parameterized.Parameters
+ public static Collection<Object[]> parameters() {
+ List<Object[]> parameterSets = new ArrayList<Object[]>();
+
+ // 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 +
+ "Host: localhost:8080" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF } } );
+
+ // Requests to simulate package boundaries
+ // HTTP/0.9 request
+ addRequestWithSplits("GET /test" + CRLF, Boolean.TRUE, parameterSets);
+
+ // HTTP/0.9 request (no optional CR)
+ addRequestWithSplits("GET /test" + LF, Boolean.TRUE, parameterSets);
+
+ // Standard HTTP/1.1 request
+ addRequestWithSplits("GET http://localhost:8080/test HTTP/1.1" + CRLF +
+ "Host: localhost:8080" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF,
+ Boolean.FALSE, parameterSets);
+
+ return parameterSets;
+ }
+
+
+ private static void addRequestWithSplits(String request, Boolean isHttp09, List<Object[]> parameterSets) {
+ // Add as a single String
+ parameterSets.add(new Object[] { isHttp09, new String[] { request } } );
+
+ // Add with all CRLF split between the CR and LF
+ List<String> parts = new ArrayList<String>();
+ int lastPos = 0;
+ int pos = request.indexOf("\n");
+ while (pos > -1) {
+ parts.add(request.substring(lastPos, pos));
+ lastPos = pos;
+ pos = request.indexOf("\n", lastPos + 1);
+ }
+ parts.add(request.substring(lastPos));
+ parameterSets.add(new Object[] { isHttp09, parts.toArray(new String[0]) });
+
+ // 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]) });
+ }
+
+ @Parameter(0)
+ public boolean isHttp09;
+
+ @Parameter(1)
+ public String[] request;
+
+ @Test
+ public void testBug54947() {
+
+ Client client = new Client(request, isHttp09);
+
+ client.doRequest();
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
+ private class Client extends SimpleHttpClient {
+
+ public Client(String[] request, boolean isHttp09) {
+ setRequest(request);
+ setUseHttp09(isHttp09);
+ }
+
+ private Exception doRequest() {
+
+ Tomcat tomcat = getTomcatInstance();
+
+ Context root = tomcat.addContext("", TEMP_DIR);
+ Tomcat.addServlet(root, "TesterServlet", new TesterServlet());
+ root.addServletMapping("/test", "TesterServlet");
+
+ try {
+ tomcat.start();
+ setPort(tomcat.getConnector().getLocalPort());
+ setRequestPause(20);
+
+ // Open connection
+ connect();
+
+ setRequest(request);
+ processRequest(); // blocks until response has been read
+
+ // Close the connection
+ disconnect();
+ } catch (Exception e) {
+ return e;
+ }
+ return null;
+ }
+
+ @Override
+ public boolean isResponseBodyOK() {
+ if (getResponseBody() == null) {
+ return false;
+ }
+ if (!getResponseBody().contains("OK")) {
+ return false;
+ }
+ return true;
+ }
+ }
+}
diff --git a/test/org/apache/coyote/http11/TestInternalInputBuffer.java b/test/org/apache/coyote/http11/TestInternalInputBuffer.java
index 24005fe..4c149b0 100644
--- a/test/org/apache/coyote/http11/TestInternalInputBuffer.java
+++ b/test/org/apache/coyote/http11/TestInternalInputBuffer.java
@@ -38,6 +38,10 @@ import org.apache.catalina.startup.TomcatBaseTest;
public class TestInternalInputBuffer extends TomcatBaseTest {
+ private static final String CR = "\r";
+ private static final String LF = "\n";
+ private static final String CRLF = CR + LF;
+
/**
* Test case for https://bz.apache.org/bugzilla/show_bug.cgi?id=48839
*/
@@ -661,7 +665,83 @@ public class TestInternalInputBuffer extends TomcatBaseTest {
@Test
public void testInvalidMethod() {
- InvalidMethodClient client = new InvalidMethodClient();
+ String[] request = new String[1];
+ request[0] =
+ "GET" + (char) 0 + " /test HTTP/1.1" + CRLF +
+ "Host: localhost:8080" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF;
+
+ InvalidClient client = new InvalidClient(request);
+
+ client.doRequest();
+ Assert.assertTrue(client.getResponseLine(), client.isResponse400());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
+ @Test
+ public void testInvalidHttp09() {
+
+ String[] request = new String[1];
+ request[0] = "GET /test" + CR + " " + LF;
+
+ InvalidClient client = new InvalidClient(request);
+
+ client.doRequest();
+ Assert.assertTrue(client.getResponseLine(), client.isResponse400());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
+ @Test
+ public void testInvalidEndOfRequestLine01() {
+
+ String[] request = new String[1];
+ request[0] =
+ "GET /test HTTP/1.1" + CR +
+ "Host: localhost:8080" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF;
+
+ InvalidClient client = new InvalidClient(request);
+
+ client.doRequest();
+ Assert.assertTrue(client.getResponseLine(), client.isResponse400());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
+ @Test
+ public void testInvalidEndOfRequestLine02() {
+
+ String[] request = new String[1];
+ request[0] =
+ "GET /test HTTP/1.1" + LF +
+ "Host: localhost:8080" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF;
+
+ InvalidClient client = new InvalidClient(request);
+
+ client.doRequest();
+ Assert.assertTrue(client.getResponseLine(), client.isResponse400());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
+ @Test
+ public void testInvalidHeader01() {
+
+ String[] request = new String[1];
+ request[0] =
+ "GET /test HTTP/1.1" + CRLF +
+ "Host: localhost:8080" + CRLF +
+ CR + "X-Header: xxx" + CRLF +
+ "Connection: close" + CRLF +
+ CRLF;
+
+ InvalidClient client = new InvalidClient(request);
client.doRequest();
Assert.assertTrue(client.getResponseLine(), client.isResponse400());
@@ -670,13 +750,20 @@ public class TestInternalInputBuffer extends TomcatBaseTest {
/**
- * Bug 48839 test client.
+ * Invalid request test client.
*/
- private class InvalidMethodClient extends SimpleHttpClient {
+ private class InvalidClient extends SimpleHttpClient {
+
+ private final String[] request;
+
+ public InvalidClient(String[] request) {
+ this.request = request;
+ }
private Exception doRequest() {
Tomcat tomcat = getTomcatInstance();
+ tomcat.getConnector().setProperty("rejectIllegalHeader", "true");
tomcat.addContext("", TEMP_DIR);
@@ -686,14 +773,6 @@ public class TestInternalInputBuffer extends TomcatBaseTest {
// Open connection
connect();
-
- String[] request = new String[1];
- request[0] =
- "GET" + (char) 0 + " /test HTTP/1.1" + CRLF +
- "Host: localhost:8080" + CRLF +
- "Connection: close" + CRLF +
- CRLF;
-
setRequest(request);
processRequest(); // blocks until response has been read
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ada2d32..df12a96 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -60,6 +60,17 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 7.0.102 (violetagg)">
+ <subsection name="Coyote">
+ <changelog>
+ <fix>
+ <bug>64210</bug>: Correct a regression in the improvements to HTTP
+ header validation that caused requests to be incorrectly treated as
+ invalid if a <code>CRLF</code> sequence was split between TCP packets.
+ Improve validation of request lines, including for HTTP/0.9 requests.
+ (markt)
+ </fix>
+ </changelog>
+ </subsection>
</section>
<section name="Tomcat 7.0.101 (violetagg)">
<subsection name="Catalina">
@@ -146,9 +157,9 @@
used by JULI's <code>OneLineFormatter</code>. (markt)
</add>
<fix>
- <bug>64206</bug>: Correct regression introduced in 10.0.0-M1 that meant
- the HTTP specified when using the Windows Installer was ignored and 8080
- was always used. (markt)
+ <bug>64206</bug>: Correct a regression introduced in 7.0.100 that meant
+ that the HTTP port specified when using the Windows Installer was
+ ignored and 8080 was always used. (markt)
</fix>
</changelog>
</subsection>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org