Author: markt Date: Fri Oct 5 10:07:49 2018 New Revision: 1842878 URL: http://svn.apache.org/viewvc?rev=1842878&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62739 Do not reject requests with an empty HTTP Host header. Such requests are unusual but not invalid. Patch provided by Michael Orr. This closes #124.
Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1842878&r1=1842877&r2=1842878&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Fri Oct 5 10:07:49 2018 @@ -266,6 +266,12 @@ public abstract class AbstractProcessor protected void parseHost(MessageBytes valueMB) { if (valueMB == null || valueMB.isNull()) { populateHost(); + populatePort(); + return; + } else if (valueMB.getLength() == 0) { + // Empty Host header so set sever name to empty string + request.serverName().setString(""); + populatePort(); return; } @@ -329,9 +335,9 @@ public abstract class AbstractProcessor /** - * Called when a host name is not present in the request (e.g. HTTP/1.0). - * It populates the server name and port with appropriate information. The - * source is expected to vary by protocol. + * Called when a host header is not present in the request (e.g. HTTP/1.0). + * It populates the server name with appropriate information. The source is + * expected to vary by protocol. * <p> * The default implementation is a NO-OP. */ @@ -339,6 +345,18 @@ public abstract class AbstractProcessor // NO-OP } + + /** + * Called when a host header is not present or is empty in the request (e.g. + * HTTP/1.0). It populates the server port with appropriate information. The + * source is expected to vary by protocol. + * <p> + * The default implementation is a NO-OP. + */ + protected void populatePort() { + // NO-OP + } + @Override public final void action(ActionCode actionCode, Object param) { Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1842878&r1=1842877&r2=1842878&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Fri Oct 5 10:07:49 2018 @@ -858,13 +858,11 @@ public class AjpProcessor extends Abstra /** * {@inheritDoc} * <p> - * This implementation populates the server name and port from the local - * name and port provided by the AJP message. + * This implementation populates the server name from the local name + * provided by the AJP message. */ @Override protected void populateHost() { - // No host information (HTTP/1.0) - request.setServerPort(request.getLocalPort()); try { request.serverName().duplicate(request.localName()); } catch (IOException e) { @@ -874,6 +872,19 @@ public class AjpProcessor extends Abstra } + /** + * {@inheritDoc} + * <p> + * This implementation populates the server port from the local port + * provided by the AJP message. + */ + @Override + protected void populatePort() { + // No host information (HTTP/1.0) + request.setServerPort(request.getLocalPort()); + } + + /** * When committing the response, we have to validate the set of headers, as * well as setup the response filters. 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=1842878&r1=1842877&r2=1842878&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Fri Oct 5 10:07:49 2018 @@ -1011,21 +1011,23 @@ public class Http11Processor extends Abs } + /* + * Note: populateHost() is not over-ridden. + * request.serverName() will be set to return the default host name by + * the Mapper. + */ + + /** * {@inheritDoc} * <p> - * This implementation provides the server name from the default host and - * the server port from the local port. + * This implementation provides the server port from the local port. */ @Override - protected void populateHost() { - // No host information (HTTP/1.0) + protected void populatePort() { // Ensure the local port field is populated before using it. request.action(ActionCode.REQ_LOCALPORT_ATTRIBUTE, request); request.setServerPort(request.getLocalPort()); - - // request.serverName() will be set to the default host name by the - // mapper } 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=1842878&r1=1842877&r2=1842878&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java (original) +++ tomcat/trunk/test/org/apache/coyote/http11/TestHttp11Processor.java Fri Oct 5 10:07:49 2018 @@ -1204,6 +1204,118 @@ public class TestHttp11Processor extends } /* + * Hostname (no port) is included in the request line, but Host header + * is empty. + * Added for bug 62739. + */ + @Test + public void testInconsistentHostHeader04() 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: " + 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. + Assert.assertTrue(client.isResponse400()); + } + + /* + * Hostname (with port) is included in the request line, but Host header + * is empty. + * Added for bug 62739. + */ + @Test + public void testInconsistentHostHeader05() 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: " + 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. + Assert.assertTrue(client.isResponse400()); + } + + /* + * Hostname (with port and user) is included in the request line, but Host + * header is empty. + * Added for bug 62739. + */ + @Test + public void testInconsistentHostHeader06() 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: " + 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. + Assert.assertTrue(client.isResponse400()); + } + + + /* * Request line host is an exact match for Host header (no port) */ @Test @@ -1218,7 +1330,7 @@ public class TestHttp11Processor extends Context ctx = tomcat.addContext("", null); // Add servlet - Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet()); + Tomcat.addServlet(ctx, "TesterServlet", new ServerNameTesterServlet()); ctx.addServletMappingDecoded("/foo", "TesterServlet"); tomcat.start(); @@ -1236,6 +1348,7 @@ public class TestHttp11Processor extends // Expected response is a 200 response. Assert.assertTrue(client.isResponse200()); + Assert.assertEquals("request.getServerName() is [a] and request.getServerPort() is 80", client.getResponseBody()); } /* @@ -1253,7 +1366,7 @@ public class TestHttp11Processor extends Context ctx = tomcat.addContext("", null); // Add servlet - Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet()); + Tomcat.addServlet(ctx, "TesterServlet", new ServerNameTesterServlet()); ctx.addServletMappingDecoded("/foo", "TesterServlet"); tomcat.start(); @@ -1271,6 +1384,8 @@ public class TestHttp11Processor extends // Expected response is a 200 response. Assert.assertTrue(client.isResponse200()); + Assert.assertEquals("request.getServerName() is [a] and request.getServerPort() is 8080", client.getResponseBody()); + } /* @@ -1289,7 +1404,7 @@ public class TestHttp11Processor extends Context ctx = tomcat.addContext("", null); // Add servlet - Tomcat.addServlet(ctx, "TesterServlet", new TesterServlet()); + Tomcat.addServlet(ctx, "TesterServlet", new ServerNameTesterServlet()); ctx.addServletMappingDecoded("/foo", "TesterServlet"); tomcat.start(); @@ -1307,5 +1422,119 @@ public class TestHttp11Processor extends // Expected response is a 200 response. Assert.assertTrue(client.isResponse200()); + Assert.assertEquals("request.getServerName() is [a] and request.getServerPort() is 80", client.getResponseBody()); + } + + /* + * Host header exists but its value is an empty string. This is valid if + * the request line does not include a hostname/port. + * Added for bug 62739. + */ + @Test + public void testBlankHostHeader01() 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 ServerNameTesterServlet()); + ctx.addServletMappingDecoded("/foo", "TesterServlet"); + + tomcat.start(); + + String request = + "GET /foo HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: " + 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. + Assert.assertTrue(client.isResponse200()); + Assert.assertEquals("request.getServerName() is [] and request.getServerPort() is " + getPort(), client.getResponseBody()); + } + + /* + * Host header exists but has its value is empty (and there are multiple + * spaces after the ':'. This is valid if the request line does not + * include a hostname/port. + * Added for bug 62739. + */ + @Test + public void testBlankHostHeader02() 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 ServerNameTesterServlet()); + ctx.addServletMappingDecoded("/foo", "TesterServlet"); + + tomcat.start(); + + String request = + "GET /foo HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: " + 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. + Assert.assertTrue(client.isResponse200()); + Assert.assertEquals("request.getServerName() is [] and request.getServerPort() is " + getPort(), client.getResponseBody()); + } + + + /** + * Test servlet that prints out the values of + * HttpServletRequest.getServerName() and + * HttpServletRequest.getServerPort() in the response body, e.g.: + * + * "request.getServerName() is [foo] and request.getServerPort() is 8080" + * + * or: + * + * "request.getServerName() is null and request.getServerPort() is 8080" + */ + private static class ServerNameTesterServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + + resp.setContentType("text/plain"); + PrintWriter out = resp.getWriter(); + + if (null == req.getServerName()) + { + out.print("request.getServerName() is null"); + } + else + { + out.print("request.getServerName() is [" + req.getServerName() + "]"); + } + + out.print(" and request.getServerPort() is " + req.getServerPort()); + } } } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1842878&r1=1842877&r2=1842878&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Fri Oct 5 10:07:49 2018 @@ -109,6 +109,11 @@ <fix> Make PEM file parser a public utility class. (remm) </fix> + <fix> + <bug>62739</bug>: Do not reject requests with an empty HTTP Host header. + Such requests are unusual but not invalid. Patch provided by Michael + Orr. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org