tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1808766 - in /tomcat/trunk: java/org/apache/coyote/http11/Http11Processor.java java/org/apache/coyote/http11/LocalStrings.properties test/org/apache/coyote/http11/TestHttp11Processor.java webapps/docs/changelog.xml
Date Mon, 18 Sep 2017 19:33:18 GMT
Author: markt
Date: Mon Sep 18 19:33:18 2017
New Revision: 1808766

URL: http://svn.apache.org/viewvc?rev=1808766&view=rev
Log:
Implement various Host header checks required by RFC 7230
- Host header must be present for HTTP/1.1 requests
- multiple host headers are invalid
- if the request line include the host, it must match the host header

Modified:
    tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
    tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
    tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1808766&r1=1808765&r2=1808766&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Mon Sep 18 19:33:18 2017
@@ -725,6 +725,30 @@ public class Http11Processor extends Abs
             }
         }
 
+
+        // Check host header
+        MessageBytes hostValueMB = null;
+        try {
+            hostValueMB = headers.getUniqueValue("host");
+        } catch (IllegalArgumentException iae) {
+            // Multiple Host headers are not permitted
+            // 400 - Bad request
+            response.setStatus(400);
+            setErrorState(ErrorState.CLOSE_CLEAN, null);
+            if (log.isDebugEnabled()) {
+                log.debug(sm.getString("http11processor.request.multipleHosts"));
+            }
+        }
+        if (http11 && hostValueMB == null) {
+            // 400 - Bad request
+            response.setStatus(400);
+            setErrorState(ErrorState.CLOSE_CLEAN, null);
+            if (log.isDebugEnabled()) {
+                log.debug(sm.getString("http11processor.request.prepare")+
+                          " host header missing");
+            }
+        }
+
         // Check for a full URI (including protocol://host:port/)
         ByteChunk uriBC = request.requestURI().getByteChunk();
         if (uriBC.startsWithIgnoreCase("http", 0)) {
@@ -733,21 +757,44 @@ public class Http11Processor extends Abs
             int uriBCStart = uriBC.getStart();
             int slashPos = -1;
             if (pos != -1) {
+                pos += 3;
                 byte[] uriB = uriBC.getBytes();
-                slashPos = uriBC.indexOf('/', pos + 3);
+                slashPos = uriBC.indexOf('/', pos);
+                int atPos = uriBC.indexOf('@', pos);
                 if (slashPos == -1) {
                     slashPos = uriBC.getLength();
                     // Set URI as "/"
                     request.requestURI().setBytes
-                        (uriB, uriBCStart + pos + 1, 1);
+                        (uriB, uriBCStart + pos - 2, 1);
                 } else {
                     request.requestURI().setBytes
                         (uriB, uriBCStart + slashPos,
                          uriBC.getLength() - slashPos);
                 }
-                MessageBytes hostMB = headers.setValue("host");
-                hostMB.setBytes(uriB, uriBCStart + pos + 3,
-                                slashPos - pos - 3);
+                // Skip any user info
+                if (atPos != -1) {
+                    pos = atPos + 1;
+                }
+                if (http11) {
+                    // Missing host header is illegal but handled above
+                    if (hostValueMB != null) {
+                        // Any host in the request line must be consistent with
+                        // the Host header
+                        if (!hostValueMB.getByteChunk().equals(
+                                uriB, uriBCStart + pos, slashPos - pos)) {
+                            response.setStatus(400);
+                            setErrorState(ErrorState.CLOSE_CLEAN, null);
+                            if (log.isDebugEnabled()) {
+                                log.debug(sm.getString("http11processor.request.inconsistentHosts"));
+                            }
+                        }
+                    }
+                } else {
+                    // Not HTTP/1.1 - no Host header so generate one since
+                    // Tomcat internals assume it is set
+                    hostValueMB = headers.setValue("host");
+                    hostValueMB.setBytes(uriB, uriBCStart + pos, slashPos - pos);
+                }
             }
         }
 
@@ -792,20 +839,7 @@ public class Http11Processor extends Abs
             }
         }
 
-        MessageBytes valueMB = headers.getValue("host");
-
-        // Check host header
-        if (http11 && (valueMB == null)) {
-            // 400 - Bad request
-            response.setStatus(400);
-            setErrorState(ErrorState.CLOSE_CLEAN, null);
-            if (log.isDebugEnabled()) {
-                log.debug(sm.getString("http11processor.request.prepare")+
-                          " host header missing");
-            }
-        }
-
-        parseHost(valueMB);
+        parseHost(hostValueMB);
 
         if (!contentDelimitation) {
             // If there's no content length

Modified: tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties?rev=1808766&r1=1808765&r2=1808766&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties Mon Sep 18 19:33:18
2017
@@ -20,6 +20,8 @@ abstractHttp11Protocol.httpUpgradeConfig
 http11processor.fallToDebug=\n Note: further occurrences of HTTP header parsing errors will
be logged at DEBUG level.
 http11processor.header.parse=Error parsing HTTP request header
 http11processor.neverused=This method should never be used
+http11processor.request.inconsistentHosts=The host specified in the request line is not consistent
with the host header
+http11processor.request.multipleHosts=The request contained multiple host headers
 http11processor.request.prepare=Error preparing request
 http11processor.request.process=Error processing request
 http11processor.request.finish=Error finishing request

Modified: tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java?rev=1808766&r1=1808765&r2=1808766&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java Mon Sep 18 19:33:18
2017
@@ -1008,4 +1008,309 @@ public class TestHttp11Processor extends
             resp.setStatus(205);
         }
     }
+
+    /*
+     * Multiple, different Host headers
+     */
+    @Test
+    public void testMultipleHostHeader01() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request =
+                "GET /foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                "Host: a" + SimpleHttpClient.CRLF +
+                "Host: b" + SimpleHttpClient.CRLF +
+                 SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+
+        // Expected response is a 400 response.
+        assertTrue(client.isResponse400());
+    }
+
+    /*
+     * Multiple instances of the same Host header
+     */
+    @Test
+    public void testMultipleHostHeader02() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request =
+                "GET /foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                "Host: a" + SimpleHttpClient.CRLF +
+                "Host: a" + SimpleHttpClient.CRLF +
+                 SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+
+        // Expected response is a 400 response.
+        assertTrue(client.isResponse400());
+    }
+
+    @Test
+    public void testMissingHostHeader() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request =
+                "GET /foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                 SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+
+        // Expected response is a 400 response.
+        assertTrue(client.isResponse400());
+    }
+
+    @Test
+    public void testInconsistentHostHeader01() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request =
+                "GET http://a/foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                "Host: b" + SimpleHttpClient.CRLF +
+                 SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+
+        // Expected response is a 400 response.
+        assertTrue(client.isResponse400());
+    }
+
+    @Test
+    public void testInconsistentHostHeader02() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request =
+                "GET http://a:8080/foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                "Host: b:8080" + SimpleHttpClient.CRLF +
+                 SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+
+        // Expected response is a 400 response.
+        assertTrue(client.isResponse400());
+    }
+
+    @Test
+    public void testInconsistentHostHeader03() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request =
+                "GET http://user:pwd@a/foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                "Host: b" + SimpleHttpClient.CRLF +
+                 SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+
+        // Expected response is a 400 response.
+        assertTrue(client.isResponse400());
+    }
+
+    /*
+     * Request line host is an exact match for Host header (no port)
+     */
+    @Test
+    public void testConsistentHostHeader01() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request =
+                "GET http://a/foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                "Host: a" + SimpleHttpClient.CRLF +
+                 SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+
+        // Expected response is a 200 response.
+        assertTrue(client.isResponse200());
+    }
+
+    /*
+     * Request line host is an exact match for Host header (with port)
+     */
+    @Test
+    public void testConsistentHostHeader02() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request =
+                "GET http://a:8080/foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                "Host: a:8080" + SimpleHttpClient.CRLF +
+                 SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+
+        // Expected response is a 200 response.
+        assertTrue(client.isResponse200());
+    }
+
+    /*
+     * Request line host is an exact match for Host header
+     * (no port, with user info)
+     */
+    @Test
+    public void testConsistentHostHeader03() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        // Add servlet
+        Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet());
+        ctx.addServletMappingDecoded("/foo", "TesterServlet");
+
+        tomcat.start();
+
+        String request =
+                "GET http://user:pwd@a/foo HTTP/1.1" + SimpleHttpClient.CRLF +
+                "Host: a" + SimpleHttpClient.CRLF +
+                 SimpleHttpClient.CRLF;
+
+        Client client = new Client(tomcat.getConnector().getLocalPort());
+        client.setRequest(new String[] {request});
+
+        client.connect();
+        client.processRequest();
+
+        // Expected response is a 200 response.
+        assertTrue(client.isResponse200());
+    }
 }

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1808766&r1=1808765&r2=1808766&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Sep 18 19:33:18 2017
@@ -57,6 +57,21 @@
         (non-token) header names with a 400 response and reject such requests by
         default. (markt)
       </add>
+      <fix>
+        Implement the requirements of RFC 7230 (and RFC 2616) that HTTP/1.1
+        requests must include a <code>Host</code> header and any request that
+        does not must be rejected with a 400 response. (markt)
+      </fix>
+      <fix>
+        Implement the requirements of RFC 7230 that any HTTP/1.1 request that
+        specifies a host in the request line, must specify the same host in the
+        <code>Host</code> header. (markt)
+      </fix>
+      <fix>
+        Implement the requirements of RFC 7230 that any HTTP/1.1 request that
+        contains multiple <code>Host</code> headers is rejected with a 400
+        response. (markt)
+      </fix>
     </changelog>
   </subsection>
 </section>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message