You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2018/10/05 10:07:49 UTC
svn commit: r1842878 - in /tomcat/trunk: java/org/apache/coyote/
java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/
test/org/apache/coyote/http11/ webapps/docs/
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org