File tomcat-8.0.53-CVE-2020-1935.patch of Package tomcat.19337
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -78,27 +78,56 @@ public abstract class AbstractHttp11Prot
}
- private boolean rejectIllegalHeaderName = false;
+ private boolean rejectIllegalHeader = false;
/**
- * If an HTTP request is received that contains an illegal header name (i.e.
- * the header name is not a token) will the request be rejected (with a 400
- * response) or will the illegal header be ignored.
+ * If an HTTP request is received that contains an illegal header name or
+ * value (e.g. the header name is not a token) will the request be rejected
+ * (with a 400 response) or will the illegal header be ignored?
*
* @return {@code true} if the request will be rejected or {@code false} if
* the header will be ignored
*/
- public boolean getRejectIllegalHeaderName() { return rejectIllegalHeaderName; }
+ public boolean getRejectIllegalHeader() { return rejectIllegalHeader; }
/**
- * If an HTTP request is received that contains an illegal header name (i.e.
- * the header name is not a token) should the request be rejected (with a
- * 400 response) or should the illegal header be ignored.
+ * If an HTTP request is received that contains an illegal header name or
+ * value (e.g. the header name is not a token) should the request be
+ * rejected (with a 400 response) or should the illegal header be ignored?
+ *
+ * @param rejectIllegalHeader {@code true} to reject requests with illegal
+ * header names or values, {@code false} to
+ * ignore the header
+ */
+ public void setRejectIllegalHeader(boolean rejectIllegalHeader) {
+ this.rejectIllegalHeader = rejectIllegalHeader;
+ }
+ /**
+ * If an HTTP request is received that contains an illegal header name or
+ * value (e.g. the header name is not a token) will the request be rejected
+ * (with a 400 response) or will the illegal header be ignored?
+ *
+ * @return {@code true} if the request will be rejected or {@code false} if
+ * the header will be ignored
+ *
+ * @deprecated Now an alias for {@link #getRejectIllegalHeader()}. Will be
+ * removed in Tomcat 10 onwards.
+ */
+ @Deprecated
+ public boolean getRejectIllegalHeaderName() { return rejectIllegalHeader; }
+ /**
+ * If an HTTP request is received that contains an illegal header name or
+ * value (e.g. the header name is not a token) should the request be
+ * rejected (with a 400 response) or should the illegal header be ignored?
*
* @param rejectIllegalHeaderName {@code true} to reject requests with
- * illegal header names, {@code false} to
- * ignore the header
+ * illegal header names or values,
+ * {@code false} to ignore the header
+ *
+ * @deprecated Now an alias for {@link #setRejectIllegalHeader(boolean)}.
+ * Will be removed in Tomcat 10 onwards.
*/
+ @Deprecated
public void setRejectIllegalHeaderName(boolean rejectIllegalHeaderName) {
- this.rejectIllegalHeaderName = rejectIllegalHeaderName;
+ this.rejectIllegalHeader = rejectIllegalHeaderName;
}
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractInputBuffer.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/AbstractInputBuffer.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractInputBuffer.java
@@ -110,6 +110,11 @@ public abstract class AbstractInputBuffe
protected int lastActiveFilter;
+ /**
+ * Do HTTP headers with illegal names and/or values cause the request to be
+ * rejected? Note that the field name is not quite right but cannot be
+ * easily changed without breaking binary compatibility.
+ */
protected boolean rejectIllegalHeaderName;
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11AprProcessor.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11AprProcessor.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11AprProcessor.java
@@ -61,7 +61,7 @@ public class Http11AprProcessor extends
// ----------------------------------------------------------- Constructors
- public Http11AprProcessor(int headerBufferSize, boolean rejectIllegalHeaderName,
+ public Http11AprProcessor(int headerBufferSize, boolean rejectIllegalHeader,
AprEndpoint endpoint, int maxTrailerSize, Set<String> allowedTrailerHeaders,
int maxExtensionSize, int maxSwallowSize, String relaxedPathChars,
String relaxedQueryChars) {
@@ -70,7 +70,7 @@ public class Http11AprProcessor extends
httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
- inputBuffer = new InternalAprInputBuffer(request, headerBufferSize, rejectIllegalHeaderName,
+ inputBuffer = new InternalAprInputBuffer(request, headerBufferSize, rejectIllegalHeader,
httpParser);
request.setInputBuffer(inputBuffer);
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11AprProtocol.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11AprProtocol.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11AprProtocol.java
@@ -290,7 +290,7 @@ public class Http11AprProtocol extends A
@Override
protected Http11AprProcessor createProcessor() {
Http11AprProcessor processor = new Http11AprProcessor(
- proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeaderName(),
+ proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeader(),
(AprEndpoint)proto.endpoint, proto.getMaxTrailerSize(),
proto.getAllowedTrailerHeadersAsSet(), proto.getMaxExtensionSize(),
proto.getMaxSwallowSize(), proto.getRelaxedPathChars(),
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11NioProcessor.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11NioProcessor.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11NioProcessor.java
@@ -65,7 +65,7 @@ public class Http11NioProcessor extends
// ----------------------------------------------------------- Constructors
- public Http11NioProcessor(int maxHttpHeaderSize, boolean rejectIllegalHeaderName,
+ public Http11NioProcessor(int maxHttpHeaderSize, boolean rejectIllegalHeader,
NioEndpoint endpoint, int maxTrailerSize, Set<String> allowedTrailerHeaders,
int maxExtensionSize, int maxSwallowSize, String relaxedPathChars,
String relaxedQueryChars) {
@@ -75,7 +75,7 @@ public class Http11NioProcessor extends
httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
inputBuffer = new InternalNioInputBuffer(request, maxHttpHeaderSize,
- rejectIllegalHeaderName, httpParser);
+ rejectIllegalHeader, httpParser);
request.setInputBuffer(inputBuffer);
outputBuffer = new InternalNioOutputBuffer(response, maxHttpHeaderSize);
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11NioProtocol.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11NioProtocol.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11NioProtocol.java
@@ -264,7 +264,7 @@ public class Http11NioProtocol extends A
@Override
public Http11NioProcessor createProcessor() {
Http11NioProcessor processor = new Http11NioProcessor(
- proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeaderName(),
+ proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeader(),
(NioEndpoint)proto.endpoint, proto.getMaxTrailerSize(),
proto.getAllowedTrailerHeadersAsSet(), proto.getMaxExtensionSize(),
proto.getMaxSwallowSize(), proto.getRelaxedPathChars(),
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11Processor.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11Processor.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11Processor.java
@@ -50,7 +50,7 @@ public class Http11Processor extends Abs
// ------------------------------------------------------------ Constructor
- public Http11Processor(int headerBufferSize, boolean rejectIllegalHeaderName,
+ public Http11Processor(int headerBufferSize, boolean rejectIllegalHeader,
JIoEndpoint endpoint, int maxTrailerSize, Set<String> allowedTrailerHeaders,
int maxExtensionSize, int maxSwallowSize, String relaxedPathChars,
String relaxedQueryChars) {
@@ -59,7 +59,7 @@ public class Http11Processor extends Abs
httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
- inputBuffer = new InternalInputBuffer(request, headerBufferSize, rejectIllegalHeaderName,
+ inputBuffer = new InternalInputBuffer(request, headerBufferSize, rejectIllegalHeader,
httpParser);
request.setInputBuffer(inputBuffer);
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11Protocol.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/Http11Protocol.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/Http11Protocol.java
@@ -165,7 +165,7 @@ public class Http11Protocol extends Abst
@Override
protected Http11Processor createProcessor() {
Http11Processor processor = new Http11Processor(
- proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeaderName(),
+ proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeader(),
(JIoEndpoint)proto.endpoint, proto.getMaxTrailerSize(),
proto.getAllowedTrailerHeadersAsSet(), proto.getMaxExtensionSize(),
proto.getMaxSwallowSize(), proto.getRelaxedPathChars(),
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/InternalAprInputBuffer.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/InternalAprInputBuffer.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/InternalAprInputBuffer.java
@@ -54,7 +54,7 @@ public class InternalAprInputBuffer exte
* Alternate constructor.
*/
public InternalAprInputBuffer(Request request, int headerBufferSize,
- boolean rejectIllegalHeaderName, HttpParser httpParser) {
+ boolean rejectIllegalHeader, HttpParser httpParser) {
this.request = request;
headers = request.getMimeHeaders();
@@ -66,7 +66,7 @@ public class InternalAprInputBuffer exte
bbuf = ByteBuffer.allocateDirect((headerBufferSize / 1500 + 1) * 1500);
}
- this.rejectIllegalHeaderName = rejectIllegalHeaderName;
+ this.rejectIllegalHeaderName = rejectIllegalHeader;
this.httpParser = httpParser;
inputStreamInputBuffer = new SocketInputBuffer();
@@ -354,6 +354,8 @@ public class InternalAprInputBuffer exte
//
byte chr = 0;
+ byte prevChr = 0;
+
while (true) {
// Read new bytes if needed
@@ -362,19 +364,23 @@ public class InternalAprInputBuffer exte
throw new EOFException(sm.getString("iib.eof.error"));
}
+ prevChr = chr;
chr = buf[pos];
- if (chr == Constants.CR) {
- // Skip
- } else if (chr == Constants.LF) {
- pos++;
+ if (chr == Constants.CR && prevChr != Constants.CR) {
+ // Possible start of CRLF - process the next byte.
+ } else if (prevChr == Constants.CR && chr == Constants.LF) {
+ pos++;
return false;
} else {
+ if (prevChr == Constants.CR) {
+ // Must have read two bytes (first was CR, second was not LF)
+ pos--;
+ }
break;
}
pos++;
-
}
// Mark the current buffer position
@@ -458,10 +464,24 @@ public class InternalAprInputBuffer exte
throw new EOFException(sm.getString("iib.eof.error"));
}
- if (buf[pos] == Constants.CR) {
- // Skip
- } else if (buf[pos] == Constants.LF) {
+ prevChr = chr;
+ chr = buf[pos];
+ if (chr == Constants.CR) {
+ // Possible start of CRLF - process the next byte.
+ } else if (prevChr == Constants.CR && chr == Constants.LF) {
eol = true;
+ } else if (prevChr == Constants.CR) {
+ // Invalid value
+ // Delete the header (it will be the most recent one)
+ headers.removeHeader(headers.size() - 1);
+ skipLine(start);
+ return true;
+ } else if (chr != Constants.HT && HttpParser.isControl(chr)) {
+ // Invalid value
+ // Delete the header (it will be the most recent one)
+ headers.removeHeader(headers.size() - 1);
+ skipLine(start);
+ return true;
} else if (buf[pos] == Constants.SP) {
buf[realPos] = buf[pos];
realPos++;
@@ -514,6 +534,9 @@ public class InternalAprInputBuffer exte
lastRealByte = pos - 1;
}
+ byte chr = 0;
+ byte prevChr = 0;
+
while (!eol) {
// Read new bytes if needed
@@ -522,9 +545,12 @@ public class InternalAprInputBuffer exte
throw new EOFException(sm.getString("iib.eof.error"));
}
- if (buf[pos] == Constants.CR) {
+ prevChr = chr;
+ chr = buf[pos];
+
+ if (chr == Constants.CR) {
// Skip
- } else if (buf[pos] == Constants.LF) {
+ } else if (prevChr == Constants.CR && chr == Constants.LF) {
eol = true;
} else {
lastRealByte = pos;
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/InternalInputBuffer.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/InternalInputBuffer.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/InternalInputBuffer.java
@@ -53,14 +53,14 @@ public class InternalInputBuffer extends
* Default constructor.
*/
public InternalInputBuffer(Request request, int headerBufferSize,
- boolean rejectIllegalHeaderName, HttpParser httpParser) {
+ boolean rejectIllegalHeader, HttpParser httpParser) {
this.request = request;
headers = request.getMimeHeaders();
buf = new byte[headerBufferSize];
- this.rejectIllegalHeaderName = rejectIllegalHeaderName;
+ this.rejectIllegalHeaderName = rejectIllegalHeader;
this.httpParser = httpParser;
inputStreamInputBuffer = new InputStreamInputBuffer();
@@ -313,6 +313,8 @@ public class InternalInputBuffer extends
//
byte chr = 0;
+ byte prevChr = 0;
+
while (true) {
// Read new bytes if needed
@@ -321,19 +323,23 @@ public class InternalInputBuffer extends
throw new EOFException(sm.getString("iib.eof.error"));
}
+ prevChr = chr;
chr = buf[pos];
- if (chr == Constants.CR) {
- // Skip
- } else if (chr == Constants.LF) {
+ if (chr == Constants.CR && prevChr != Constants.CR) {
+ // Possible start of CRLF - process the next byte.
+ } else if (prevChr == Constants.CR && chr == Constants.LF) {
pos++;
return false;
} else {
+ if (prevChr == Constants.CR) {
+ // Must have read two bytes (first was CR, second was not LF)
+ pos--;
+ }
break;
}
pos++;
-
}
// Mark the current buffer position
@@ -418,15 +424,29 @@ public class InternalInputBuffer extends
throw new EOFException(sm.getString("iib.eof.error"));
}
- if (buf[pos] == Constants.CR) {
- // Skip
- } else if (buf[pos] == Constants.LF) {
+ prevChr = chr;
+ chr = buf[pos];
+ if (chr == Constants.CR) {
+ // Possible start of CRLF - process the next byte.
+ } else if (prevChr == Constants.CR && chr == Constants.LF) {
eol = true;
- } else if (buf[pos] == Constants.SP) {
- buf[realPos] = buf[pos];
+ } else if (prevChr == Constants.CR) {
+ // Invalid value
+ // Delete the header (it will be the most recent one)
+ headers.removeHeader(headers.size() - 1);
+ skipLine(start);
+ return true;
+ } else if (chr != Constants.HT && HttpParser.isControl(chr)) {
+ // Invalid value
+ // Delete the header (it will be the most recent one)
+ headers.removeHeader(headers.size() - 1);
+ skipLine(start);
+ return true;
+ } else if (chr == Constants.SP) {
+ buf[realPos] = chr;
realPos++;
} else {
- buf[realPos] = buf[pos];
+ buf[realPos] = chr;
realPos++;
lastSignificantChar = realPos;
}
@@ -492,6 +512,9 @@ public class InternalInputBuffer extends
lastRealByte = pos - 1;
}
+ byte chr = 0;
+ byte prevChr = 0;
+
while (!eol) {
// Read new bytes if needed
@@ -500,9 +523,12 @@ public class InternalInputBuffer extends
throw new EOFException(sm.getString("iib.eof.error"));
}
- if (buf[pos] == Constants.CR) {
+ prevChr = chr;
+ chr = buf[pos];
+
+ if (chr == Constants.CR) {
// Skip
- } else if (buf[pos] == Constants.LF) {
+ } else if (prevChr == Constants.CR && chr == Constants.LF) {
eol = true;
} else {
lastRealByte = pos;
Index: apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/http/MimeHeaders.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/tomcat/util/http/MimeHeaders.java
+++ apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/http/MimeHeaders.java
@@ -366,7 +366,7 @@ public class MimeHeaders {
* reset and swap with last header
* @param idx the index of the header to remove.
*/
- private void removeHeader(int idx) {
+ public void removeHeader(int idx) {
MimeHeaderField mh = headers[idx];
mh.recycle();
Index: apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/http/parser/HttpParser.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/tomcat/util/http/parser/HttpParser.java
+++ apache-tomcat-8.0.53-src/java/org/apache/tomcat/util/http/parser/HttpParser.java
@@ -337,6 +337,17 @@ public class HttpParser {
}
+ public static boolean isControl(int c) {
+ // Fast for valid control characters, slower for some incorrect
+ // ones
+ try {
+ return IS_CONTROL[c];
+ } catch (ArrayIndexOutOfBoundsException ex) {
+ return false;
+ }
+ }
+
+
// Skip any LWS and position to read the next character. The next character
// is returned as being able to 'peek()' it allows a small optimisation in
// some cases.
Index: apache-tomcat-8.0.53-src/test/org/apache/coyote/http11/TestInternalInputBuffer.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/test/org/apache/coyote/http11/TestInternalInputBuffer.java
+++ apache-tomcat-8.0.53-src/test/org/apache/coyote/http11/TestInternalInputBuffer.java
@@ -163,13 +163,13 @@ public class TestInternalInputBuffer ext
@Test
- public void testBug51557Separators() throws Exception {
+ public void testBug51557SeparatorsInName() throws Exception {
char httpSeparators[] = new char[] {
'\t', ' ', '\"', '(', ')', ',', '/', ':', ';', '<',
'=', '>', '?', '@', '[', '\\', ']', '{', '}' };
for (char s : httpSeparators) {
- doTestBug51557Char(s);
+ doTestBug51557CharInName(s);
tearDown();
setUp();
}
@@ -177,13 +177,38 @@ public class TestInternalInputBuffer ext
@Test
- public void testBug51557Ctl() throws Exception {
+ public void testBug51557CtlInName() throws Exception {
for (int i = 0; i < 31; i++) {
- doTestBug51557Char((char) i);
+ doTestBug51557CharInName((char) i);
+ tearDown();
+ setUp();
+ }
+ doTestBug51557CharInName((char) 127);
+ }
+
+
+ @Test
+ public void testBug51557CtlInValue() throws Exception {
+ for (int i = 0; i < 31; i++) {
+ if (i == '\t') {
+ // TAB is allowed
+ continue;
+ }
+ doTestBug51557InvalidCharInValue((char) i);
+ tearDown();
+ setUp();
+ }
+ doTestBug51557InvalidCharInValue((char) 127);
+ }
+
+
+ @Test
+ public void testBug51557ObsTextInValue() throws Exception {
+ for (int i = 128; i < 255; i++) {
+ doTestBug51557ValidCharInValue((char) i);
tearDown();
setUp();
}
- doTestBug51557Char((char) 127);
}
@@ -226,7 +251,33 @@ public class TestInternalInputBuffer ext
}
- private void doTestBug51557Char(char s) {
+ @Test
+ public void testBug51557CRStartName() {
+
+ Bug51557Client client = new Bug51557Client("\rName=",
+ "invalid");
+
+ client.doRequest();
+ Assert.assertTrue(client.isResponse200());
+ Assert.assertEquals("abcd", client.getResponseBody());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
+ @Test
+ public void testBug51557CR2StartName() {
+
+ Bug51557Client client = new Bug51557Client("\r\rName=",
+ "invalid");
+
+ client.doRequest();
+ Assert.assertTrue(client.isResponse200());
+ Assert.assertEquals("abcd", client.getResponseBody());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
+ private void doTestBug51557CharInName(char s) {
Bug51557Client client =
new Bug51557Client("X-Bug" + s + "51557", "invalid");
@@ -236,6 +287,29 @@ public class TestInternalInputBuffer ext
Assert.assertTrue(client.isResponseBodyOK());
}
+
+ private void doTestBug51557InvalidCharInValue(char s) {
+ Bug51557Client client =
+ new Bug51557Client("X-Bug51557-Invalid", "invalid" + s + "invalid");
+
+ client.doRequest();
+ Assert.assertTrue("Testing [" + (int) s + "]", client.isResponse200());
+ Assert.assertEquals("Testing [" + (int) s + "]", "abcd", client.getResponseBody());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
+ private void doTestBug51557ValidCharInValue(char s) {
+ Bug51557Client client =
+ new Bug51557Client("X-Bug51557-Valid", "valid" + s + "valid");
+
+ client.doRequest();
+ Assert.assertTrue("Testing [" + (int) s + "]", client.isResponse200());
+ Assert.assertEquals("Testing [" + (int) s + "]", "valid" + s + "validabcd", client.getResponseBody());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
/**
* Bug 51557 test client.
*/
@@ -243,12 +317,12 @@ public class TestInternalInputBuffer ext
private final String headerName;
private final String headerLine;
- private final boolean rejectIllegalHeaderName;
+ private final boolean rejectIllegalHeader;
public Bug51557Client(String headerName) {
this.headerName = headerName;
this.headerLine = headerName;
- this.rejectIllegalHeaderName = false;
+ this.rejectIllegalHeader = false;
}
public Bug51557Client(String headerName, String headerValue) {
@@ -256,10 +330,10 @@ public class TestInternalInputBuffer ext
}
public Bug51557Client(String headerName, String headerValue,
- boolean rejectIllegalHeaderName) {
+ boolean rejectIllegalHeader) {
this.headerName = headerName;
this.headerLine = headerName + ": " + headerValue;
- this.rejectIllegalHeaderName = rejectIllegalHeaderName;
+ this.rejectIllegalHeader = rejectIllegalHeader;
}
private Exception doRequest() {
@@ -273,8 +347,8 @@ public class TestInternalInputBuffer ext
try {
Connector connector = tomcat.getConnector();
- connector.setProperty("rejectIllegalHeaderName",
- Boolean.toString(rejectIllegalHeaderName));
+ Assert.assertTrue(connector.setProperty(
+ "rejectIllegalHeader", Boolean.toString(rejectIllegalHeader)));
tomcat.start();
setPort(connector.getLocalPort());
@@ -557,6 +631,75 @@ public class TestInternalInputBuffer ext
"X-Header: Ignore" + CRLF +
"X-Header" + (char) 130 + ": Broken" + CRLF + CRLF;
+ 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;
+ }
+ }
+
+
+ /**
+ * Test case for https://bz.apache.org/bugzilla/show_bug.cgi?id=59089
+ */
+ @Test
+ public void testBug59089() {
+
+ Bug59089Client client = new Bug59089Client();
+
+ client.doRequest();
+ Assert.assertTrue(client.isResponse200());
+ Assert.assertTrue(client.isResponseBodyOK());
+ }
+
+
+ /**
+ * Bug 59089 test client.
+ */
+ private class Bug59089Client extends SimpleHttpClient {
+
+ private Exception doRequest() {
+
+ // Ensure body is read correctly
+ setUseContentLength(true);
+
+ Tomcat tomcat = getTomcatInstance();
+
+ Context root = tomcat.addContext("", TEMP_DIR);
+ Tomcat.addServlet(root, "Bug59089", new TesterServlet());
+ root.addServletMapping("/test", "Bug59089");
+
+ try {
+ Connector connector = tomcat.getConnector();
+ Assert.assertTrue(connector.setProperty("rejectIllegalHeader", "false"));
+ tomcat.start();
+ setPort(connector.getLocalPort());
+
+ // Open connection
+ connect();
+
+ String[] request = new String[1];
+ request[0] = "GET http://localhost:8080/test HTTP/1.1" + CRLF +
+ "Host: localhost:8080" + CRLF +
+ "X-Header: Ignore" + CRLF +
+ "X-Header" + (char) 130 + ": Broken" + CRLF + CRLF;
+
setRequest(request);
processRequest(); // blocks until response has been read
Index: apache-tomcat-8.0.53-src/webapps/docs/changelog.xml
===================================================================
--- apache-tomcat-8.0.53-src.orig/webapps/docs/changelog.xml
+++ apache-tomcat-8.0.53-src/webapps/docs/changelog.xml
@@ -174,6 +174,11 @@
not contain leading zeros in the IPv4 part. Based on a patch by Katya
Stoycheva. (markt)
</fix>
+ <fix>
+ Rename the HTTP Connector attribute <code>rejectIllegalHeaderName</code>
+ to <code>rejectIllegalHeader</code> and expand the underlying
+ implementation to include header values as well as names. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
Index: apache-tomcat-8.0.53-src/webapps/docs/config/http.xml
===================================================================
--- apache-tomcat-8.0.53-src.orig/webapps/docs/config/http.xml
+++ apache-tomcat-8.0.53-src/webapps/docs/config/http.xml
@@ -529,15 +529,20 @@
expected concurrent requests (synchronous and asynchronous).</p>
</attribute>
- <attribute name="rejectIllegalHeaderName" required="false">
- <p>If an HTTP request is received that contains an illegal header name
- (i.e. the header name is not a token) this setting determines if the
+ <attribute name="rejectIllegalHeader" required="false">
+ <p>If an HTTP request is received that contains an illegal header name or
+ value (e.g. the header name is not a token) this setting determines if the
request will be rejected with a 400 response (<code>true</code>) or if the
illegal header be ignored (<code>false</code>). The default value is
<code>false</code> which will cause the request to be processed but the
illegal header will be ignored.</p>
</attribute>
+ <attribute name="rejectIllegalHeaderName" required="false">
+ <p>This attribute is deprecated. It will be removed in Tomcat 10 onwards.
+ It is now an alias for <strong>rejectIllegalHeader</strong>.</p>
+ </attribute>
+
<attribute name="relaxedPathChars" required="false">
<p>The <a href="https://tools.ietf.org/rfc/rfc7230.txt">HTTP/1.1
specification</a> requires that certain characters are %nn encoded when
Index: apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractNioInputBuffer.java
===================================================================
--- apache-tomcat-8.0.53-src.orig/java/org/apache/coyote/http11/AbstractNioInputBuffer.java
+++ apache-tomcat-8.0.53-src/java/org/apache/coyote/http11/AbstractNioInputBuffer.java
@@ -77,13 +77,13 @@ public abstract class AbstractNioInputBu
* Alternate constructor.
*/
public AbstractNioInputBuffer(Request request, int headerBufferSize,
- boolean rejectIllegalHeaderName, HttpParser httpParser) {
+ boolean rejectIllegalHeader, HttpParser httpParser) {
this.request = request;
headers = request.getMimeHeaders();
this.headerBufferSize = headerBufferSize;
- this.rejectIllegalHeaderName = rejectIllegalHeaderName;
+ this.rejectIllegalHeaderName = rejectIllegalHeader;
this.httpParser = httpParser;
filterLibrary = new InputFilter[0];
@@ -428,6 +428,8 @@ public abstract class AbstractNioInputBu
//
byte chr = 0;
+ byte prevChr = 0;
+
while (headerParsePos == HeaderParsePosition.HEADER_START) {
// Read new bytes if needed
@@ -438,19 +440,23 @@ public abstract class AbstractNioInputBu
}
}
+ prevChr = chr;
chr = buf[pos];
- if (chr == Constants.CR) {
- // Skip
- } else if (chr == Constants.LF) {
+ if (chr == Constants.CR && prevChr != Constants.CR) {
+ // Possible start of CRLF - process the next byte.
+ } else if (prevChr == Constants.CR && chr == Constants.LF) {
pos++;
return HeaderParseStatus.DONE;
} else {
+ if (prevChr == Constants.CR) {
+ // Must have read two bytes (first was CR, second was not LF)
+ pos--;
+ }
break;
}
pos++;
-
}
if ( headerParsePos == HeaderParsePosition.HEADER_START ) {
@@ -545,11 +551,22 @@ public abstract class AbstractNioInputBu
}
}
+ prevChr = chr;
chr = buf[pos];
if (chr == Constants.CR) {
- // Skip
- } else if (chr == Constants.LF) {
+ // Possible start of CRLF - process the next byte.
+ } else if (prevChr == Constants.CR && chr == Constants.LF) {
eol = true;
+ } else if (prevChr == Constants.CR) {
+ // Invalid value
+ // Delete the header (it will be the most recent one)
+ headers.removeHeader(headers.size() - 1);
+ return skipLine();
+ } else if (chr != Constants.HT && HttpParser.isControl(chr)) {
+ // Invalid value
+ // Delete the header (it will be the most recent one)
+ headers.removeHeader(headers.size() - 1);
+ return skipLine();
} else if (chr == Constants.SP || chr == Constants.HT) {
buf[headerData.realPos] = chr;
headerData.realPos++;
@@ -606,6 +623,9 @@ public abstract class AbstractNioInputBu
headerParsePos = HeaderParsePosition.HEADER_SKIPLINE;
boolean eol = false;
+ byte chr = 0;
+ byte prevChr = 0;
+
// Reading bytes until the end of the line
while (!eol) {
@@ -616,9 +636,12 @@ public abstract class AbstractNioInputBu
}
}
- if (buf[pos] == Constants.CR) {
+ prevChr = chr;
+ chr = buf[pos];
+
+ if (chr == Constants.CR) {
// Skip
- } else if (buf[pos] == Constants.LF) {
+ } else if (prevChr == Constants.CR && chr == Constants.LF) {
eol = true;
} else {
headerData.lastSignificantChar = pos;
