diff -up ./java/org/apache/coyote/http11/AbstractHttp11Protocol.java.orig ./java/org/apache/coyote/http11/AbstractHttp11Protocol.java
--- ./java/org/apache/coyote/http11/AbstractHttp11Protocol.java.orig 2020-04-24 14:57:33.400591691 -0400
+++ ./java/org/apache/coyote/http11/AbstractHttp11Protocol.java 2020-04-24 14:28:55.936278683 -0400
@@ -62,6 +62,59 @@ public abstract class AbstractHttp11Prot
this.relaxedQueryChars = relaxedQueryChars;
}
+ private boolean rejectIllegalHeader = false;
+ /**
+ * 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 getRejectIllegalHeader() { 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 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 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.rejectIllegalHeader = rejectIllegalHeaderName;
+ }
+
+
private int socketBuffer = 9000;
public int getSocketBuffer() { return socketBuffer; }
public void setSocketBuffer(int socketBuffer) {
diff -up ./java/org/apache/coyote/http11/AbstractInputBuffer.java.orig ./java/org/apache/coyote/http11/AbstractInputBuffer.java
--- ./java/org/apache/coyote/http11/AbstractInputBuffer.java.orig 2020-04-24 14:57:33.402591687 -0400
+++ ./java/org/apache/coyote/http11/AbstractInputBuffer.java 2020-04-24 14:28:55.936278683 -0400
@@ -34,7 +34,6 @@ public abstract class AbstractInputBuffe
*/
protected static final StringManager sm = StringManager.getManager(Constants.Package);
-
/**
* Associated Coyote request.
*/
@@ -112,6 +111,14 @@ public abstract class AbstractInputBuffe
protected HttpParser httpParser;
+ /**
+ * 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;
+
+
// ------------------------------------------------------------- Properties
diff -up ./java/org/apache/coyote/http11/Http11AprProcessor.java.orig ./java/org/apache/coyote/http11/Http11AprProcessor.java
--- ./java/org/apache/coyote/http11/Http11AprProcessor.java.orig 2020-04-24 14:57:33.387591718 -0400
+++ ./java/org/apache/coyote/http11/Http11AprProcessor.java 2020-04-24 14:46:07.375029423 -0400
@@ -61,8 +61,8 @@ public class Http11AprProcessor extends
// ----------------------------------------------------------- Constructors
- public Http11AprProcessor(int headerBufferSize, AprEndpoint endpoint, int maxTrailerSize,
- Set<String> allowedTrailerHeaders,
+ public Http11AprProcessor(int headerBufferSize, boolean rejectIllegalHeader,
+ AprEndpoint endpoint, int maxTrailerSize, Set<String> allowedTrailerHeaders,
int maxExtensionSize, int maxSwallowSize, String relaxedPathChars,
String relaxedQueryChars) {
@@ -70,7 +70,8 @@ public class Http11AprProcessor extends
httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
- inputBuffer = new InternalAprInputBuffer(request, headerBufferSize, httpParser);
+ inputBuffer = new InternalAprInputBuffer(request, headerBufferSize, rejectIllegalHeader,
+ httpParser);
request.setInputBuffer(inputBuffer);
outputBuffer = new InternalAprOutputBuffer(response, headerBufferSize);
diff -up ./java/org/apache/coyote/http11/Http11AprProtocol.java.orig ./java/org/apache/coyote/http11/Http11AprProtocol.java
--- ./java/org/apache/coyote/http11/Http11AprProtocol.java.orig 2020-04-24 14:57:33.393591706 -0400
+++ ./java/org/apache/coyote/http11/Http11AprProtocol.java 2020-04-24 14:45:37.327092381 -0400
@@ -299,9 +299,9 @@ public class Http11AprProtocol extends A
@Override
protected Http11AprProcessor createProcessor() {
Http11AprProcessor processor = new Http11AprProcessor(
- proto.getMaxHttpHeaderSize(), (AprEndpoint)proto.endpoint,
- proto.getMaxTrailerSize(), proto.getAllowedTrailerHeadersAsSet(),
- proto.getMaxExtensionSize(),
+ proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeader(),
+ (AprEndpoint)proto.endpoint, proto.getMaxTrailerSize(),
+ proto.getAllowedTrailerHeadersAsSet(), proto.getMaxExtensionSize(),
proto.getMaxSwallowSize(), proto.getRelaxedPathChars(),
proto.getRelaxedQueryChars());
processor.setAdapter(proto.adapter);
diff -up ./java/org/apache/coyote/http11/Http11NioProcessor.java.orig ./java/org/apache/coyote/http11/Http11NioProcessor.java
--- ./java/org/apache/coyote/http11/Http11NioProcessor.java.orig 2020-04-24 14:57:33.399591693 -0400
+++ ./java/org/apache/coyote/http11/Http11NioProcessor.java 2020-04-24 14:45:24.452119361 -0400
@@ -66,8 +66,8 @@ public class Http11NioProcessor extends
// ----------------------------------------------------------- Constructors
- public Http11NioProcessor(int maxHttpHeaderSize, NioEndpoint endpoint, int maxTrailerSize,
- Set<String> allowedTrailerHeaders,
+ public Http11NioProcessor(int maxHttpHeaderSize, boolean rejectIllegalHeader,
+ NioEndpoint endpoint, int maxTrailerSize, Set<String> allowedTrailerHeaders,
int maxExtensionSize, int maxSwallowSize, String relaxedPathChars,
String relaxedQueryChars) {
@@ -75,7 +75,8 @@ public class Http11NioProcessor extends
httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
- inputBuffer = new InternalNioInputBuffer(request, maxHttpHeaderSize, httpParser);
+ inputBuffer = new InternalNioInputBuffer(request, maxHttpHeaderSize,
+ rejectIllegalHeader, httpParser);
request.setInputBuffer(inputBuffer);
outputBuffer = new InternalNioOutputBuffer(response, maxHttpHeaderSize);
diff -up ./java/org/apache/coyote/http11/Http11NioProtocol.java.orig ./java/org/apache/coyote/http11/Http11NioProtocol.java
--- ./java/org/apache/coyote/http11/Http11NioProtocol.java.orig 2020-04-24 14:57:33.392591708 -0400
+++ ./java/org/apache/coyote/http11/Http11NioProtocol.java 2020-04-24 14:28:55.937278681 -0400
@@ -264,9 +264,9 @@ public class Http11NioProtocol extends A
@Override
public Http11NioProcessor createProcessor() {
Http11NioProcessor processor = new Http11NioProcessor(
- proto.getMaxHttpHeaderSize(), (NioEndpoint)proto.endpoint,
- proto.getMaxTrailerSize(), proto.getAllowedTrailerHeadersAsSet(),
- proto.getMaxExtensionSize(),
+ proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeader(),
+ (NioEndpoint)proto.endpoint, proto.getMaxTrailerSize(),
+ proto.getAllowedTrailerHeadersAsSet(), proto.getMaxExtensionSize(),
proto.getMaxSwallowSize(), proto.getRelaxedPathChars(),
proto.getRelaxedQueryChars());
processor.setAdapter(proto.adapter);
diff -up ./java/org/apache/coyote/http11/Http11Processor.java.orig ./java/org/apache/coyote/http11/Http11Processor.java
--- ./java/org/apache/coyote/http11/Http11Processor.java.orig 2020-04-24 14:57:33.390591712 -0400
+++ ./java/org/apache/coyote/http11/Http11Processor.java 2020-04-24 15:09:20.651109515 -0400
@@ -51,8 +51,8 @@ public class Http11Processor extends Abs
// ------------------------------------------------------------ Constructor
- public Http11Processor(int headerBufferSize, JIoEndpoint endpoint, int maxTrailerSize,
- Set<String> allowedTrailerHeaders,
+ public Http11Processor(int headerBufferSize, boolean rejectIllegalHeader,
+ JIoEndpoint endpoint, int maxTrailerSize, Set<String> allowedTrailerHeaders,
int maxExtensionSize, int maxSwallowSize, String relaxedPathChars,
String relaxedQueryChars) {
@@ -60,7 +60,7 @@ public class Http11Processor extends Abs
httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
- inputBuffer = new InternalInputBuffer(request, headerBufferSize, httpParser);
+ inputBuffer = new InternalInputBuffer(request, headerBufferSize, rejectIllegalHeader, httpParser);
request.setInputBuffer(inputBuffer);
diff -up ./java/org/apache/coyote/http11/Http11Protocol.java.orig ./java/org/apache/coyote/http11/Http11Protocol.java
--- ./java/org/apache/coyote/http11/Http11Protocol.java.orig 2020-04-24 14:57:33.395591701 -0400
+++ ./java/org/apache/coyote/http11/Http11Protocol.java 2020-04-24 14:45:08.641152489 -0400
@@ -163,9 +163,9 @@ public class Http11Protocol extends Abst
@Override
protected Http11Processor createProcessor() {
Http11Processor processor = new Http11Processor(
- proto.getMaxHttpHeaderSize(), (JIoEndpoint)proto.endpoint,
- proto.getMaxTrailerSize(), proto.getAllowedTrailerHeadersAsSet(),
- proto.getMaxExtensionSize(),
+ proto.getMaxHttpHeaderSize(), proto.getRejectIllegalHeader(),
+ (JIoEndpoint)proto.endpoint, proto.getMaxTrailerSize(),
+ proto.getAllowedTrailerHeadersAsSet(), proto.getMaxExtensionSize(),
proto.getMaxSwallowSize(), proto.getRelaxedPathChars(),
proto.getRelaxedQueryChars());
processor.setAdapter(proto.adapter);
diff -up ./java/org/apache/coyote/http11/InternalAprInputBuffer.java.orig ./java/org/apache/coyote/http11/InternalAprInputBuffer.java
--- ./java/org/apache/coyote/http11/InternalAprInputBuffer.java.orig 2020-04-24 14:57:33.398591695 -0400
+++ ./java/org/apache/coyote/http11/InternalAprInputBuffer.java 2020-04-24 15:08:35.338204464 -0400
@@ -52,7 +52,7 @@ public class InternalAprInputBuffer exte
* Alternate constructor.
*/
public InternalAprInputBuffer(Request request, int headerBufferSize,
- HttpParser httpParser) {
+ boolean rejectIllegalHeader, HttpParser httpParser) {
this.request = request;
headers = request.getMimeHeaders();
@@ -66,6 +66,8 @@ public class InternalAprInputBuffer exte
this.httpParser = httpParser;
+ this.rejectIllegalHeaderName = rejectIllegalHeader;
+
inputStreamInputBuffer = new SocketInputBuffer();
filterLibrary = new InputFilter[0];
@@ -350,6 +352,8 @@ public class InternalAprInputBuffer exte
//
byte chr = 0;
+ byte prevChr = 0;
+
while (true) {
// Read new bytes if needed
@@ -358,14 +362,19 @@ 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;
}
@@ -396,8 +405,9 @@ public class InternalAprInputBuffer exte
colon = true;
headerValue = headers.addValue(buf, start, pos - start);
} else if (!HttpParser.isToken(buf[pos])) {
- // If a non-token header is detected, skip the line and
- // ignore the header
+ // Non-token characters are illegal in header names
+ // Parsing continues so the error can be reported in context
+ // skipLine() will handle the error
skipLine(start);
return true;
}
@@ -453,10 +463,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++;
@@ -509,6 +533,9 @@ public class InternalAprInputBuffer exte
lastRealByte = pos - 1;
}
+ byte chr = 0;
+ byte prevChr = 0;
+
while (!eol) {
// Read new bytes if needed
@@ -517,9 +544,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;
@@ -527,9 +557,13 @@ public class InternalAprInputBuffer exte
pos++;
}
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("iib.invalidheader", new String(buf, start,
- lastRealByte - start + 1, Charset.forName("ISO-8859-1"))));
+ if (rejectIllegalHeaderName || log.isDebugEnabled()) {
+ String message = sm.getString("iib.invalidheader", new String(buf, start,
+ lastRealByte - start + 1, Charset.forName("ISO-8859-1")));
+ if (rejectIllegalHeaderName) {
+ throw new IllegalArgumentException(message);
+ }
+ log.debug(message);
}
}
diff -up ./java/org/apache/coyote/http11/InternalInputBuffer.java.orig ./java/org/apache/coyote/http11/InternalInputBuffer.java
--- ./java/org/apache/coyote/http11/InternalInputBuffer.java.orig 2020-04-24 14:57:33.389591714 -0400
+++ ./java/org/apache/coyote/http11/InternalInputBuffer.java 2020-04-24 15:08:54.534164236 -0400
@@ -53,13 +53,14 @@ public class InternalInputBuffer extends
* Default constructor.
*/
public InternalInputBuffer(Request request, int headerBufferSize,
- HttpParser httpParser) {
+ boolean rejectIllegalHeader, HttpParser httpParser) {
this.request = request;
headers = request.getMimeHeaders();
buf = new byte[headerBufferSize];
+ this.rejectIllegalHeaderName = rejectIllegalHeader;
this.httpParser = httpParser;
inputStreamInputBuffer = new InputStreamInputBuffer();
@@ -302,6 +303,8 @@ public class InternalInputBuffer extends
//
byte chr = 0;
+ byte prevChr = 0;
+
while (true) {
// Read new bytes if needed
@@ -310,19 +313,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
@@ -348,8 +355,9 @@ public class InternalInputBuffer extends
colon = true;
headerValue = headers.addValue(buf, start, pos - start);
} else if (!HttpParser.isToken(buf[pos])) {
- // If a non-token header is detected, skip the line and
- // ignore the header
+ // Non-token characters are illegal in header names
+ // Parsing continues so the error can be reported in context
+ // skipLine() will handle the error
skipLine(start);
return true;
}
@@ -406,15 +414,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;
}
@@ -480,6 +502,9 @@ public class InternalInputBuffer extends
lastRealByte = pos - 1;
}
+ byte chr = 0;
+ byte prevChr = 0;
+
while (!eol) {
// Read new bytes if needed
@@ -488,9 +513,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;
@@ -498,9 +526,13 @@ public class InternalInputBuffer extends
pos++;
}
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("iib.invalidheader", new String(buf, start,
- lastRealByte - start + 1, Charset.forName("ISO-8859-1"))));
+ if (rejectIllegalHeaderName || log.isDebugEnabled()) {
+ String message = sm.getString("iib.invalidheader", new String(buf, start,
+ lastRealByte - start + 1, Charset.forName("ISO-8859-1")));
+ if (rejectIllegalHeaderName) {
+ throw new IllegalArgumentException(message);
+ }
+ log.debug(message);
}
}
diff -up ./java/org/apache/coyote/http11/InternalNioInputBuffer.java.orig ./java/org/apache/coyote/http11/InternalNioInputBuffer.java
--- ./java/org/apache/coyote/http11/InternalNioInputBuffer.java.orig 2020-04-24 14:57:33.396591699 -0400
+++ ./java/org/apache/coyote/http11/InternalNioInputBuffer.java 2020-04-24 14:28:58.728272282 -0400
@@ -99,12 +99,13 @@ public class InternalNioInputBuffer exte
* Alternate constructor.
*/
public InternalNioInputBuffer(Request request, int headerBufferSize,
- HttpParser httpParser) {
+ boolean rejectIllegalHeader, HttpParser httpParser) {
this.request = request;
headers = request.getMimeHeaders();
this.headerBufferSize = headerBufferSize;
+ this.rejectIllegalHeaderName = rejectIllegalHeader;
this.httpParser = httpParser;
inputStreamInputBuffer = new SocketInputBuffer();
@@ -514,6 +515,8 @@ public class InternalNioInputBuffer exte
//
byte chr = 0;
+ byte prevChr = 0;
+
while (headerParsePos == HeaderParsePosition.HEADER_START) {
// Read new bytes if needed
@@ -524,19 +527,23 @@ public class InternalNioInputBuffer exte
}
}
+ 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 ) {
@@ -570,9 +577,10 @@ public class InternalNioInputBuffer exte
headerData.lastSignificantChar = pos;
break;
} else if (!HttpParser.isToken(chr)) {
- // If a non-token header is detected, skip the line and
- // ignore the header
+ // Non-token characters are illegal in header names
+ // Parsing continues so the error can be reported in context
headerData.lastSignificantChar = pos;
+ // skipLine() will handle the error
return skipLine();
}
@@ -630,11 +638,22 @@ public class InternalNioInputBuffer exte
}
}
+ 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++;
@@ -692,6 +711,9 @@ public class InternalNioInputBuffer exte
headerParsePos = HeaderParsePosition.HEADER_SKIPLINE;
boolean eol = false;
+ byte chr = 0;
+ byte prevChr = 0;
+
// Reading bytes until the end of the line
while (!eol) {
@@ -702,9 +724,12 @@ public class InternalNioInputBuffer exte
}
}
- 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;
@@ -712,11 +737,13 @@ public class InternalNioInputBuffer exte
pos++;
}
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("iib.invalidheader", new String(buf,
- headerData.start,
- headerData.lastSignificantChar - headerData.start + 1,
- DEFAULT_CHARSET)));
+ if (rejectIllegalHeaderName || log.isDebugEnabled()) {
+ String message = sm.getString("iib.invalidheader", new String(buf, headerData.start,
+ headerData.lastSignificantChar - headerData.start + 1, DEFAULT_CHARSET));
+ if (rejectIllegalHeaderName) {
+ throw new IllegalArgumentException(message);
+ }
+ log.debug(message);
}
headerParsePos = HeaderParsePosition.HEADER_START;
diff -up ./java/org/apache/tomcat/util/http/MimeHeaders.java.orig ./java/org/apache/tomcat/util/http/MimeHeaders.java
--- ./java/org/apache/tomcat/util/http/MimeHeaders.java.orig 2020-04-24 14:28:38.873317805 -0400
+++ ./java/org/apache/tomcat/util/http/MimeHeaders.java 2020-04-24 14:28:58.728272282 -0400
@@ -375,7 +375,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();
diff -up ./java/org/apache/tomcat/util/http/parser/HttpParser.java.orig ./java/org/apache/tomcat/util/http/parser/HttpParser.java
--- ./java/org/apache/tomcat/util/http/parser/HttpParser.java.orig 2020-04-24 14:28:58.729272279 -0400
+++ ./java/org/apache/tomcat/util/http/parser/HttpParser.java 2020-04-24 14:37:59.945050923 -0400
@@ -482,6 +482,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 return the next char
private static int skipLws(StringReader input, boolean withReset)
throws IOException {
diff -up ./test/org/apache/coyote/http11/TestInternalInputBuffer.java.orig ./test/org/apache/coyote/http11/TestInternalInputBuffer.java
--- ./test/org/apache/coyote/http11/TestInternalInputBuffer.java.orig 2020-04-24 14:57:33.385591722 -0400
+++ ./test/org/apache/coyote/http11/TestInternalInputBuffer.java 2020-04-24 14:28:58.730272277 -0400
@@ -33,6 +33,7 @@ import static org.junit.Assert.assertTru
import org.junit.Test;
import org.apache.catalina.Context;
+import org.apache.catalina.connector.Connector;
import org.apache.catalina.startup.SimpleHttpClient;
import org.apache.catalina.startup.TesterServlet;
import org.apache.catalina.startup.Tomcat;
@@ -130,6 +131,28 @@ public class TestInternalInputBuffer ext
@Test
+ public void testBug51557Valid() {
+
+ Bug51557Client client = new Bug51557Client("X-Bug51557Valid", "1234");
+
+ client.doRequest();
+ assertTrue(client.isResponse200());
+ assertEquals("1234abcd", client.getResponseBody());
+ assertTrue(client.isResponseBodyOK());
+ }
+
+
+ @Test
+ public void testBug51557Invalid() {
+
+ Bug51557Client client = new Bug51557Client("X-Bug51557=Invalid", "1234", true);
+
+ client.doRequest();
+ assertTrue(client.isResponse400());
+ }
+
+
+ @Test
public void testBug51557NoColon() {
Bug51557Client client = new Bug51557Client("X-Bug51557NoColon");
@@ -142,27 +165,52 @@ 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();
+ }
+ }
+
+
+ @Test
+ public void testBug51557CtlInName() throws Exception {
+ for (int i = 0; i < 31; i++) {
+ doTestBug51557CharInName((char) i);
tearDown();
setUp();
}
+ doTestBug51557CharInName((char) 127);
}
@Test
- public void testBug51557Ctl() throws Exception {
+ public void testBug51557CtlInValue() throws Exception {
for (int i = 0; i < 31; i++) {
- doTestBug51557Char((char) 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);
}
@@ -205,7 +253,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");
@@ -215,22 +289,53 @@ public class TestInternalInputBuffer ext
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.
*/
private class Bug51557Client extends SimpleHttpClient {
- private String headerName;
- private String headerLine;
+ private final String headerName;
+ private final String headerLine;
+ private final boolean rejectIllegalHeader;
public Bug51557Client(String headerName) {
this.headerName = headerName;
this.headerLine = headerName;
+ this.rejectIllegalHeader = false;
}
public Bug51557Client(String headerName, String headerValue) {
+ this(headerName, headerValue, false);
+ }
+
+ public Bug51557Client(String headerName, String headerValue,
+ boolean rejectIllegalHeader) {
this.headerName = headerName;
this.headerLine = headerName + ": " + headerValue;
+ this.rejectIllegalHeader = rejectIllegalHeader;
}
private Exception doRequest() {
@@ -243,8 +348,11 @@ public class TestInternalInputBuffer ext
root.addServletMapping("/test", "Bug51557");
try {
+ Connector connector = tomcat.getConnector();
+ Assert.assertTrue(connector.setProperty(
+ "rejectIllegalHeader", Boolean.toString(rejectIllegalHeader)));
tomcat.start();
- setPort(tomcat.getConnector().getLocalPort());
+ setPort(connector.getLocalPort());
// Open connection
connect();
@@ -480,6 +588,75 @@ public class TestInternalInputBuffer ext
}
+ /**
+ * 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
+
+ // 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
public void testInvalidMethod() {
diff -up webapps/docs/changelog.xml.orig webapps/docs/changelog.xml
--- webapps/docs/changelog.xml.orig 2020-04-24 14:57:33.381591731 -0400
+++ webapps/docs/changelog.xml 2020-04-24 15:44:27.588766788 -0400
@@ -77,6 +77,19 @@
</subsection>
</section>
<section name="Tomcat 7.0.76-12 (csutherl)">
+ <subsection name="Coyote">
+ <changelog>
+ <add>
+ Add an option to reject requests that contain HTTP headers with invalid
+ (non-token) header names with a 400 response. (markt)
+ </add>
+ <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="jdbc-pool">
<changelog>
<fix>
diff -up ./webapps/docs/config/http.xml.orig ./webapps/docs/config/http.xml
--- ./webapps/docs/config/http.xml.orig 2020-04-24 14:57:33.383591727 -0400
+++ ./webapps/docs/config/http.xml 2020-04-24 14:37:07.540160722 -0400
@@ -542,6 +542,20 @@
present in the value will be ignored.</p>
</attribute>
+ <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="restrictedUserAgents" required="false">
<p>The value is a regular expression (using <code>java.util.regex</code>)
matching the <code>user-agent</code> header of HTTP clients for which