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/02/04 18:43:02 UTC
[tomcat] branch 7.0.x updated: Stricter header value parsing
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 702bf15 Stricter header value parsing
702bf15 is described below
commit 702bf15bea292915684d931526d95d4990b2e73d
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Jan 6 20:53:25 2020 +0000
Stricter header value parsing
---
.../coyote/http11/AbstractHttp11Protocol.java | 51 +++++--
.../apache/coyote/http11/AbstractInputBuffer.java | 5 +
.../apache/coyote/http11/Http11AprProcessor.java | 4 +-
.../apache/coyote/http11/Http11AprProtocol.java | 2 +-
.../apache/coyote/http11/Http11NioProcessor.java | 4 +-
.../apache/coyote/http11/Http11NioProtocol.java | 2 +-
java/org/apache/coyote/http11/Http11Processor.java | 4 +-
java/org/apache/coyote/http11/Http11Protocol.java | 2 +-
.../coyote/http11/InternalAprInputBuffer.java | 50 ++++--
.../apache/coyote/http11/InternalInputBuffer.java | 54 +++++--
.../coyote/http11/InternalNioInputBuffer.java | 43 ++++--
java/org/apache/tomcat/util/http/MimeHeaders.java | 2 +-
.../apache/tomcat/util/http/parser/HttpParser.java | 11 ++
.../coyote/http11/TestInternalInputBuffer.java | 167 +++++++++++++++++++--
webapps/docs/changelog.xml | 5 +
webapps/docs/config/http.xml | 11 +-
16 files changed, 345 insertions(+), 72 deletions(-)
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
index 8009380..632760c 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -83,27 +83,56 @@ public abstract class AbstractHttp11Protocol<S> extends AbstractProtocol<S> {
}
- 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;
}
diff --git a/java/org/apache/coyote/http11/AbstractInputBuffer.java b/java/org/apache/coyote/http11/AbstractInputBuffer.java
index 5ac9f11..379fab5 100644
--- a/java/org/apache/coyote/http11/AbstractInputBuffer.java
+++ b/java/org/apache/coyote/http11/AbstractInputBuffer.java
@@ -108,6 +108,11 @@ public abstract class AbstractInputBuffer<S> implements InputBuffer{
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;
diff --git a/java/org/apache/coyote/http11/Http11AprProcessor.java b/java/org/apache/coyote/http11/Http11AprProcessor.java
index cda37e1..2281bc1 100644
--- a/java/org/apache/coyote/http11/Http11AprProcessor.java
+++ b/java/org/apache/coyote/http11/Http11AprProcessor.java
@@ -61,7 +61,7 @@ public class Http11AprProcessor extends AbstractHttp11Processor<Long> {
// ----------------------------------------------------------- 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 AbstractHttp11Processor<Long> {
httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
- inputBuffer = new InternalAprInputBuffer(request, headerBufferSize, rejectIllegalHeaderName,
+ inputBuffer = new InternalAprInputBuffer(request, headerBufferSize, rejectIllegalHeader,
httpParser);
request.setInputBuffer(inputBuffer);
diff --git a/java/org/apache/coyote/http11/Http11AprProtocol.java b/java/org/apache/coyote/http11/Http11AprProtocol.java
index a5495d8..4afeac5 100644
--- a/java/org/apache/coyote/http11/Http11AprProtocol.java
+++ b/java/org/apache/coyote/http11/Http11AprProtocol.java
@@ -299,7 +299,7 @@ public class Http11AprProtocol extends AbstractHttp11Protocol<Long> {
@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(),
diff --git a/java/org/apache/coyote/http11/Http11NioProcessor.java b/java/org/apache/coyote/http11/Http11NioProcessor.java
index 7419531..8cc5648 100644
--- a/java/org/apache/coyote/http11/Http11NioProcessor.java
+++ b/java/org/apache/coyote/http11/Http11NioProcessor.java
@@ -66,7 +66,7 @@ public class Http11NioProcessor extends AbstractHttp11Processor<NioChannel> {
// ----------------------------------------------------------- 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) {
@@ -76,7 +76,7 @@ public class Http11NioProcessor extends AbstractHttp11Processor<NioChannel> {
httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
inputBuffer = new InternalNioInputBuffer(request, maxHttpHeaderSize,
- rejectIllegalHeaderName, httpParser);
+ rejectIllegalHeader, httpParser);
request.setInputBuffer(inputBuffer);
outputBuffer = new InternalNioOutputBuffer(response, maxHttpHeaderSize);
diff --git a/java/org/apache/coyote/http11/Http11NioProtocol.java b/java/org/apache/coyote/http11/Http11NioProtocol.java
index fa51a3b..c61f3a0 100644
--- a/java/org/apache/coyote/http11/Http11NioProtocol.java
+++ b/java/org/apache/coyote/http11/Http11NioProtocol.java
@@ -264,7 +264,7 @@ public class Http11NioProtocol extends AbstractHttp11JsseProtocol<NioChannel> {
@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(),
diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java
index 0ed0e1e..de6b8f3 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -51,7 +51,7 @@ public class Http11Processor extends AbstractHttp11Processor<Socket> {
// ------------------------------------------------------------ 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) {
@@ -60,7 +60,7 @@ public class Http11Processor extends AbstractHttp11Processor<Socket> {
httpParser = new HttpParser(relaxedPathChars, relaxedQueryChars);
- inputBuffer = new InternalInputBuffer(request, headerBufferSize, rejectIllegalHeaderName,
+ inputBuffer = new InternalInputBuffer(request, headerBufferSize, rejectIllegalHeader,
httpParser);
request.setInputBuffer(inputBuffer);
diff --git a/java/org/apache/coyote/http11/Http11Protocol.java b/java/org/apache/coyote/http11/Http11Protocol.java
index 54a2ee3..2e59197 100644
--- a/java/org/apache/coyote/http11/Http11Protocol.java
+++ b/java/org/apache/coyote/http11/Http11Protocol.java
@@ -163,7 +163,7 @@ public class Http11Protocol extends AbstractHttp11JsseProtocol<Socket> {
@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(),
diff --git a/java/org/apache/coyote/http11/InternalAprInputBuffer.java b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
index da2846b..72850ba 100644
--- a/java/org/apache/coyote/http11/InternalAprInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalAprInputBuffer.java
@@ -52,7 +52,7 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
* Alternate constructor.
*/
public InternalAprInputBuffer(Request request, int headerBufferSize,
- boolean rejectIllegalHeaderName, HttpParser httpParser) {
+ boolean rejectIllegalHeader, HttpParser httpParser) {
this.request = request;
headers = request.getMimeHeaders();
@@ -64,7 +64,7 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
bbuf = ByteBuffer.allocateDirect((headerBufferSize / 1500 + 1) * 1500);
}
- this.rejectIllegalHeaderName = rejectIllegalHeaderName;
+ this.rejectIllegalHeaderName = rejectIllegalHeader;
this.httpParser = httpParser;
inputStreamInputBuffer = new SocketInputBuffer();
@@ -351,6 +351,8 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
//
byte chr = 0;
+ byte prevChr = 0;
+
while (true) {
// Read new bytes if needed
@@ -359,19 +361,23 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
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
@@ -456,10 +462,24 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
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(lineStart, 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(lineStart, start);
+ return true;
} else if (buf[pos] == Constants.SP) {
buf[realPos] = buf[pos];
realPos++;
@@ -512,6 +532,9 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
lastRealByte = pos - 1;
}
+ byte chr = 0;
+ byte prevChr = 0;
+
while (!eol) {
// Read new bytes if needed
@@ -520,9 +543,12 @@ public class InternalAprInputBuffer extends AbstractInputBuffer<Long> {
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;
diff --git a/java/org/apache/coyote/http11/InternalInputBuffer.java b/java/org/apache/coyote/http11/InternalInputBuffer.java
index 3d194e2..d1f9e48 100644
--- a/java/org/apache/coyote/http11/InternalInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalInputBuffer.java
@@ -53,14 +53,14 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
* 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();
@@ -303,6 +303,8 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
//
byte chr = 0;
+ byte prevChr = 0;
+
while (true) {
// Read new bytes if needed
@@ -311,19 +313,23 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
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
@@ -409,15 +415,29 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
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(lineStart, 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(lineStart, start);
+ return true;
+ } else if (chr == Constants.SP) {
+ buf[realPos] = chr;
realPos++;
} else {
- buf[realPos] = buf[pos];
+ buf[realPos] = chr;
realPos++;
lastSignificantChar = realPos;
}
@@ -483,6 +503,9 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
lastRealByte = pos - 1;
}
+ byte chr = 0;
+ byte prevChr = 0;
+
while (!eol) {
// Read new bytes if needed
@@ -491,9 +514,12 @@ public class InternalInputBuffer extends AbstractInputBuffer<Socket> {
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;
diff --git a/java/org/apache/coyote/http11/InternalNioInputBuffer.java b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
index ee241b1..fba00a1 100644
--- a/java/org/apache/coyote/http11/InternalNioInputBuffer.java
+++ b/java/org/apache/coyote/http11/InternalNioInputBuffer.java
@@ -100,13 +100,13 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
* Alternate constructor.
*/
public InternalNioInputBuffer(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;
inputStreamInputBuffer = new SocketInputBuffer();
@@ -515,6 +515,8 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
//
byte chr = 0;
+ byte prevChr = 0;
+
while (headerParsePos == HeaderParsePosition.HEADER_START) {
// Read new bytes if needed
@@ -525,19 +527,23 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
}
}
+ 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 ) {
@@ -633,11 +639,22 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
}
}
+ 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++;
@@ -695,6 +712,9 @@ 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) {
@@ -705,9 +725,12 @@ public class InternalNioInputBuffer extends AbstractInputBuffer<NioChannel> {
}
}
- 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;
diff --git a/java/org/apache/tomcat/util/http/MimeHeaders.java b/java/org/apache/tomcat/util/http/MimeHeaders.java
index f282a4d..ca6e295 100644
--- a/java/org/apache/tomcat/util/http/MimeHeaders.java
+++ b/java/org/apache/tomcat/util/http/MimeHeaders.java
@@ -393,7 +393,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 --git a/java/org/apache/tomcat/util/http/parser/HttpParser.java b/java/org/apache/tomcat/util/http/parser/HttpParser.java
index 7c322f3..962b5f7 100644
--- a/java/org/apache/tomcat/util/http/parser/HttpParser.java
+++ b/java/org/apache/tomcat/util/http/parser/HttpParser.java
@@ -505,6 +505,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.
diff --git a/test/org/apache/coyote/http11/TestInternalInputBuffer.java b/test/org/apache/coyote/http11/TestInternalInputBuffer.java
index 401091b..24005fe 100644
--- a/test/org/apache/coyote/http11/TestInternalInputBuffer.java
+++ b/test/org/apache/coyote/http11/TestInternalInputBuffer.java
@@ -163,13 +163,13 @@ public class TestInternalInputBuffer extends TomcatBaseTest {
@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 extends TomcatBaseTest {
@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 extends TomcatBaseTest {
}
- 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 extends TomcatBaseTest {
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 extends TomcatBaseTest {
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 extends TomcatBaseTest {
}
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 extends TomcatBaseTest {
try {
Connector connector = tomcat.getConnector();
- connector.setProperty("rejectIllegalHeaderName",
- Boolean.toString(rejectIllegalHeaderName));
+ Assert.assertTrue(connector.setProperty(
+ "rejectIllegalHeader", Boolean.toString(rejectIllegalHeader)));
tomcat.start();
setPort(connector.getLocalPort());
@@ -515,6 +589,75 @@ public class TestInternalInputBuffer extends TomcatBaseTest {
}
+ /**
+ * 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 --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 99d7e83..aaf2f39 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -134,6 +134,11 @@
the <code>Transfer-Encoding</code> header were ignored rather than
treated as an error. (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">
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index f6cb6bc..9a95902 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -540,15 +540,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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org