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

Reply via email to