This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 1f5951ad32 Hard-code rejectIllegalHeader and allowHostHeaderMismatch
to defaults
1f5951ad32 is described below
commit 1f5951ad3224009b98662c8d6b0bbdfcffd2c3b1
Author: Mark Thomas <[email protected]>
AuthorDate: Tue May 2 18:06:44 2023 +0100
Hard-code rejectIllegalHeader and allowHostHeaderMismatch to defaults
---
.../coyote/http11/AbstractHttp11Protocol.java | 63 ------------
.../apache/coyote/http11/Http11InputBuffer.java | 50 ++--------
java/org/apache/coyote/http11/Http11Processor.java | 20 +---
.../coyote/http11/TestHttp11InputBuffer.java | 109 +++------------------
webapps/docs/changelog.xml | 6 ++
webapps/docs/config/http.xml | 21 ----
6 files changed, 36 insertions(+), 233 deletions(-)
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
index e31fbb25a2..9cdd084198 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -172,69 +172,6 @@ public abstract class AbstractHttp11Protocol<S> extends
AbstractProtocol<S> {
}
- private boolean allowHostHeaderMismatch = false;
-
- /**
- * Will Tomcat accept an HTTP 1.1 request where the host header does not
agree with the host specified (if any) in
- * the request line?
- *
- * @return {@code true} if Tomcat will allow such requests, otherwise
{@code false}
- *
- * @deprecated This will removed in Tomcat 11 onwards where {@code
allowHostHeaderMismatch} will be hard-coded to
- * {@code false}.
- */
- @Deprecated
- public boolean getAllowHostHeaderMismatch() {
- return allowHostHeaderMismatch;
- }
-
- /**
- * Will Tomcat accept an HTTP 1.1 request where the host header does not
agree with the host specified (if any) in
- * the request line?
- *
- * @param allowHostHeaderMismatch {@code true} to allow such requests,
{@code false} to reject them with a 400
- *
- * @deprecated This will removed in Tomcat 11 onwards where {@code
allowHostHeaderMismatch} will be hard-coded to
- * {@code false}.
- */
- @Deprecated
- public void setAllowHostHeaderMismatch(boolean allowHostHeaderMismatch) {
- this.allowHostHeaderMismatch = allowHostHeaderMismatch;
- }
-
-
- private boolean rejectIllegalHeader = true;
-
- /**
- * 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 This will removed in Tomcat 11 onwards where {@code
allowHostHeaderMismatch} will be hard-coded to
- * {@code true}.
- */
- @Deprecated
- 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
- *
- * @deprecated This will removed in Tomcat 11 onwards where {@code
allowHostHeaderMismatch} will be hard-coded to
- * {@code true}.
- */
- @Deprecated
- public void setRejectIllegalHeader(boolean rejectIllegalHeader) {
- this.rejectIllegalHeader = rejectIllegalHeader;
- }
-
-
private int maxSavePostSize = 4 * 1024;
/**
diff --git a/java/org/apache/coyote/http11/Http11InputBuffer.java
b/java/org/apache/coyote/http11/Http11InputBuffer.java
index e6b1595803..36a1bb4f1c 100644
--- a/java/org/apache/coyote/http11/Http11InputBuffer.java
+++ b/java/org/apache/coyote/http11/Http11InputBuffer.java
@@ -65,8 +65,6 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
private final MimeHeaders headers;
- private final boolean rejectIllegalHeader;
-
/**
* State.
*/
@@ -148,14 +146,12 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
// ----------------------------------------------------------- Constructors
- public Http11InputBuffer(Request request, int headerBufferSize, boolean
rejectIllegalHeader,
- HttpParser httpParser) {
+ public Http11InputBuffer(Request request, int headerBufferSize, HttpParser
httpParser) {
this.request = request;
headers = request.getMimeHeaders();
this.headerBufferSize = headerBufferSize;
- this.rejectIllegalHeader = rejectIllegalHeader;
this.httpParser = httpParser;
filterLibrary = new InputFilter[0];
@@ -886,7 +882,7 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
if (headerData.start == pos) {
// Zero length header name - not valid.
// skipLine() will handle the error
- return skipLine(false);
+ return skipLine();
}
headerParsePos = HeaderParsePosition.HEADER_VALUE_START;
headerData.headerValue = headers.addValue(byteBuffer.array(),
headerData.start, pos - headerData.start);
@@ -902,7 +898,7 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
headerData.lastSignificantChar = pos;
byteBuffer.position(byteBuffer.position() - 1);
// skipLine() will handle the error
- return skipLine(false);
+ return skipLine();
}
// chr is next byte of header name. Convert to lowercase.
@@ -913,7 +909,7 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
// Skip the line and ignore the header
if (headerParsePos == HeaderParsePosition.HEADER_SKIPLINE) {
- return skipLine(false);
+ return skipLine();
}
//
@@ -971,10 +967,10 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
eol = true;
} else if (prevChr == Constants.CR) {
// Invalid value - also need to delete header
- return skipLine(true);
+ return skipLine();
} else if (chr != Constants.HT &&
HttpParser.isControl(chr)) {
// Invalid value - also need to delete header
- return skipLine(true);
+ return skipLine();
} else if (chr == Constants.SP || chr == Constants.HT) {
byteBuffer.put(headerData.realPos, chr);
headerData.realPos++;
@@ -1022,25 +1018,7 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
}
- private HeaderParseStatus skipLine(boolean deleteHeader) throws
IOException {
- boolean rejectThisHeader = rejectIllegalHeader;
- // Check if rejectIllegalHeader is disabled and needs to be overridden
- // for this header. The header name is required to determine if this
- // override is required. The header name is only available once the
- // header has been created. If the header has been created then
- // deleteHeader will be true.
- if (!rejectThisHeader && deleteHeader) {
- if (headers.getName(headers.size() -
1).equalsIgnoreCase("content-length")) {
- // Malformed content-length headers must always be rejected
- // RFC 9112, section 6.3, bullet 5.
- rejectThisHeader = true;
- } else {
- // Only need to delete the header if the request isn't going to
- // be rejected (it will be the most recent one)
- headers.removeHeader(headers.size() - 1);
- }
- }
-
+ private HeaderParseStatus skipLine() throws IOException {
// Parse the rest of the invalid header so we can construct a useful
// exception and/or debug message.
headerParsePos = HeaderParsePosition.HEADER_SKIPLINE;
@@ -1068,18 +1046,10 @@ public class Http11InputBuffer implements InputBuffer,
ApplicationBufferHandler
headerData.lastSignificantChar = pos;
}
}
- if (rejectThisHeader || log.isDebugEnabled()) {
- if (rejectThisHeader) {
- throw new IllegalArgumentException(
- sm.getString("iib.invalidheader.reject",
HeaderUtil.toPrintableString(byteBuffer.array(),
- headerData.lineStart,
headerData.lastSignificantChar - headerData.lineStart + 1)));
- }
- log.debug(sm.getString("iib.invalidheader",
HeaderUtil.toPrintableString(byteBuffer.array(),
- headerData.lineStart, headerData.lastSignificantChar -
headerData.lineStart + 1)));
- }
- headerParsePos = HeaderParsePosition.HEADER_START;
- return HeaderParseStatus.HAVE_MORE_HEADERS;
+ throw new IllegalArgumentException(
+ sm.getString("iib.invalidheader.reject",
HeaderUtil.toPrintableString(byteBuffer.array(),
+ headerData.lineStart, headerData.lastSignificantChar -
headerData.lineStart + 1)));
}
diff --git a/java/org/apache/coyote/http11/Http11Processor.java
b/java/org/apache/coyote/http11/Http11Processor.java
index bad930c98c..9e2e74914c 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -155,8 +155,7 @@ public class Http11Processor extends AbstractProcessor {
httpParser = new HttpParser(protocol.getRelaxedPathChars(),
protocol.getRelaxedQueryChars());
- inputBuffer = new Http11InputBuffer(request,
protocol.getMaxHttpRequestHeaderSize(),
- protocol.getRejectIllegalHeader(), httpParser);
+ inputBuffer = new Http11InputBuffer(request,
protocol.getMaxHttpRequestHeaderSize(), httpParser);
request.setInputBuffer(inputBuffer);
outputBuffer = new Http11OutputBuffer(response,
protocol.getMaxHttpResponseHeaderSize());
@@ -733,19 +732,10 @@ public class Http11Processor extends AbstractProcessor {
// Any host in the request line must be consistent with
// the Host header
if (!hostValueMB.getByteChunk().equals(uriB,
uriBCStart + pos, slashPos - pos)) {
- if (protocol.getAllowHostHeaderMismatch()) {
- // The requirements of RFC 2616 are being
- // applied. If the host header and the request
- // line do not agree, the request line takes
- // precedence
- hostValueMB = headers.setValue("host");
- hostValueMB.setBytes(uriB, uriBCStart + pos,
slashPos - pos);
- } else {
- // The requirements of RFC 7230 are being
- // applied. If the host header and the request
- // line do not agree, trigger a 400 response.
-
badRequest("http11processor.request.inconsistentHosts");
- }
+ // The requirements of RFC 7230 are being
+ // applied. If the host header and the request
+ // line do not agree, trigger a 400 response.
+
badRequest("http11processor.request.inconsistentHosts");
}
}
} else {
diff --git a/test/org/apache/coyote/http11/TestHttp11InputBuffer.java
b/test/org/apache/coyote/http11/TestHttp11InputBuffer.java
index 95fefd82dd..8bee3e1b2e 100644
--- a/test/org/apache/coyote/http11/TestHttp11InputBuffer.java
+++ b/test/org/apache/coyote/http11/TestHttp11InputBuffer.java
@@ -140,7 +140,7 @@ public class TestHttp11InputBuffer extends TomcatBaseTest {
@Test
public void testBug51557Invalid() {
- Bug51557Client client = new Bug51557Client("X-Bug51557=Invalid",
"1234", true);
+ Bug51557Client client = new Bug51557Client("X-Bug51557=Invalid",
"1234");
client.doRequest();
Assert.assertTrue(client.isResponse400());
@@ -153,9 +153,7 @@ public class TestHttp11InputBuffer extends TomcatBaseTest {
Bug51557Client client = new Bug51557Client("X-Bug51557NoColon");
client.doRequest();
- Assert.assertTrue(client.isResponse200());
- Assert.assertEquals("abcd", client.getResponseBody());
- Assert.assertTrue(client.isResponseBodyOK());
+ Assert.assertTrue(client.isResponse400());
}
@@ -218,9 +216,7 @@ public class TestHttp11InputBuffer extends TomcatBaseTest {
Bug51557Client client = new Bug51557Client("X-Bug=51557NoColon", "foo"
+ SimpleHttpClient.CRLF + " bar");
client.doRequest();
- Assert.assertTrue(client.isResponse200());
- Assert.assertEquals("abcd", client.getResponseBody());
- Assert.assertTrue(client.isResponseBodyOK());
+ Assert.assertTrue(client.isResponse400());
}
@@ -230,9 +226,7 @@ public class TestHttp11InputBuffer extends TomcatBaseTest {
Bug51557Client client = new Bug51557Client("=X-Bug51557", "invalid");
client.doRequest();
- Assert.assertTrue(client.isResponse200());
- Assert.assertEquals("abcd", client.getResponseBody());
- Assert.assertTrue(client.isResponseBodyOK());
+ Assert.assertTrue(client.isResponse400());
}
@@ -242,9 +236,7 @@ public class TestHttp11InputBuffer extends TomcatBaseTest {
Bug51557Client client = new Bug51557Client("X-Bug51557=", "invalid");
client.doRequest();
- Assert.assertTrue(client.isResponse200());
- Assert.assertEquals("abcd", client.getResponseBody());
- Assert.assertTrue(client.isResponseBodyOK());
+ Assert.assertTrue(client.isResponse400());
}
@@ -252,9 +244,15 @@ public class TestHttp11InputBuffer extends TomcatBaseTest {
Bug51557Client client = new Bug51557Client("X-Bug" + s + "51557",
"invalid");
client.doRequest();
- Assert.assertTrue(client.isResponse200());
- Assert.assertEquals("abcd", client.getResponseBody());
- Assert.assertTrue(client.isResponseBodyOK());
+ if (s == ':') {
+ // Shortens name to 'X-Bug' and makes value '51557:invalid'
+ Assert.assertTrue(client.isResponse200());
+ Assert.assertEquals("abcd", client.getResponseBody());
+ Assert.assertTrue(client.isResponseBodyOK());
+ } else {
+ Assert.assertTrue("" + s, client.isResponse400());
+ }
+
}
@@ -262,9 +260,7 @@ public class TestHttp11InputBuffer extends TomcatBaseTest {
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());
+ Assert.assertTrue("Testing [" + (int) s + "]", client.isResponse400());
}
@@ -285,22 +281,15 @@ public class TestHttp11InputBuffer extends TomcatBaseTest
{
private final String headerName;
private final String headerLine;
- private final boolean rejectIllegalHeader;
Bug51557Client(String headerName) {
this.headerName = headerName;
this.headerLine = headerName;
- this.rejectIllegalHeader = false;
}
Bug51557Client(String headerName, String headerValue) {
- this(headerName, headerValue, false);
- }
-
- Bug51557Client(String headerName, String headerValue, boolean
rejectIllegalHeader) {
this.headerName = headerName;
this.headerLine = headerName + ": " + headerValue;
- this.rejectIllegalHeader = rejectIllegalHeader;
}
private Exception doRequest() {
@@ -313,7 +302,6 @@ public class TestHttp11InputBuffer extends TomcatBaseTest {
try {
Connector connector = tomcat.getConnector();
- Assert.assertTrue(connector.setProperty("rejectIllegalHeader",
Boolean.toString(rejectIllegalHeader)));
tomcat.start();
setPort(connector.getLocalPort());
@@ -538,73 +526,6 @@ public class TestHttp11InputBuffer 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.addServletMappingDecoded("/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 bfe823104d..0de19ecbcd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -146,6 +146,12 @@
Fix an edge case in HTTP header parsing and ensure that HTTP headers
without names are treated as invalid. (markt)
</fix>
+ <update>
+ Remove support for the HTTP Connector settings
+ <code>rejectIllegalHeader</code> and
+ <code>allowHostHeaderMismatch</code>. These are now hard-coded to the
+ previous defaults. (markt)
+ </update>
</changelog>
</subsection>
<subsection name="Jasper">
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index 236cde03a4..047959c754 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -339,16 +339,6 @@
with either <code>0.0.0.0</code> or <code>::</code>.</p>
</attribute>
- <attribute name="allowHostHeaderMismatch" required="false">
- <p>By default Tomcat will reject requests that specify a host in the
- request line but specify a different host in the host header. This
- check can be disabled by setting this attribute to <code>true</code>. If
- not specified, the default is <code>false</code>.
- <br/>
- This setting will be removed in Tomcat 11 onwards where it will be
- hard-coded to <code>false</code>.</p>
- </attribute>
-
<attribute name="allowedTrailerHeaders" required="false">
<p>By default Tomcat will ignore all trailer headers when processing
chunked input. For a header to be processed, it must be added to this
@@ -615,17 +605,6 @@
expected concurrent requests (synchronous and asynchronous).</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>true</code> which will cause the request to be rejected.
- <br/>
- This setting will be removed in Tomcat 11 onwards where it will be
- hard-coded to <code>true</code>.</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: [email protected]
For additional commands, e-mail: [email protected]
