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 <ma...@apache.org> 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: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org