This is an automated email from the ASF dual-hosted git repository. michaelo pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new 1c5bf7a BZ 63835: Add support for Keep-Alive header 1c5bf7a is described below commit 1c5bf7a904cffa438eb9b979f3bd32e1579e9666 Author: Michael Osipov <micha...@apache.org> AuthorDate: Wed Oct 23 15:37:42 2019 +0200 BZ 63835: Add support for Keep-Alive header This implementation slightly varies from HTTPd in way that it responds with 'Connection: keep-alive' if and only if additionally to the required request header the keepAliveTimeout has to be greater than 0. Also modified existing tests because HttpUrlConnection always sends 'Connection: keep-alive' where the tests are not aware of this -- thus fail. --- java/org/apache/coyote/http11/Constants.java | 1 + java/org/apache/coyote/http11/Http11Processor.java | 22 +++- .../apache/catalina/startup/SimpleHttpClient.java | 7 +- .../apache/coyote/http11/TestHttp11Processor.java | 141 ++++++++++++++++++--- webapps/docs/changelog.xml | 7 + 5 files changed, 160 insertions(+), 18 deletions(-) diff --git a/java/org/apache/coyote/http11/Constants.java b/java/org/apache/coyote/http11/Constants.java index 2ca4dc4..55045d4 100644 --- a/java/org/apache/coyote/http11/Constants.java +++ b/java/org/apache/coyote/http11/Constants.java @@ -117,6 +117,7 @@ public final class Constants { public static final String CHUNKED = "chunked"; public static final byte[] ACK_BYTES = ByteChunk.convertToBytes("HTTP/1.1 100 " + CRLF + CRLF); public static final String TRANSFERENCODING = "Transfer-Encoding"; + public static final String KEEP_ALIVE = "Keep-Alive"; public static final byte[] _200_BYTES = ByteChunk.convertToBytes("200"); public static final byte[] _400_BYTES = ByteChunk.convertToBytes("400"); public static final byte[] _404_BYTES = ByteChunk.convertToBytes("404"); diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java index 6ffcd16..4698174 100644 --- a/java/org/apache/coyote/http11/Http11Processor.java +++ b/java/org/apache/coyote/http11/Http11Processor.java @@ -907,8 +907,26 @@ public class Http11Processor extends AbstractProcessor { headers.addValue(Constants.CONNECTION).setString( Constants.CLOSE); } - } else if (!http11 && !getErrorState().isError()) { - headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE); + } else if (!getErrorState().isError()) { + if (!http11) { + headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE); + } + + boolean connectionKeepAlivePresent = + isConnectionToken(request.getMimeHeaders(), Constants.KEEPALIVE); + + if (connectionKeepAlivePresent) { + int keepAliveTimeout = protocol.getKeepAliveTimeout(); + + if (keepAliveTimeout > 0) { + String value = "timeout=" + keepAliveTimeout / 1000L; + headers.setValue(Constants.KEEP_ALIVE).setString(value); + + if (http11) { + headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE); + } + } + } } // Add server header diff --git a/test/org/apache/catalina/startup/SimpleHttpClient.java b/test/org/apache/catalina/startup/SimpleHttpClient.java index b53bb1a..6ff2050 100644 --- a/test/org/apache/catalina/startup/SimpleHttpClient.java +++ b/test/org/apache/catalina/startup/SimpleHttpClient.java @@ -56,6 +56,7 @@ public abstract class SimpleHttpClient { public static final String REDIRECT_302 = "HTTP/1.1 302 "; public static final String REDIRECT_303 = "HTTP/1.1 303 "; public static final String FAIL_400 = "HTTP/1.1 400 "; + public static final String FORBIDDEN_403 = "HTTP/1.1 403 "; public static final String FAIL_404 = "HTTP/1.1 404 "; public static final String FAIL_405 = "HTTP/1.1 405 "; public static final String TIMEOUT_408 = "HTTP/1.1 408 "; @@ -444,6 +445,10 @@ public abstract class SimpleHttpClient { return responseLineStartsWith(FAIL_400); } + public boolean isResponse403() { + return responseLineStartsWith(FORBIDDEN_403); + } + public boolean isResponse404() { return responseLineStartsWith(FAIL_404); } @@ -481,4 +486,4 @@ public abstract class SimpleHttpClient { } public abstract boolean isResponseBodyOK(); -} \ No newline at end of file +} diff --git a/test/org/apache/coyote/http11/TestHttp11Processor.java b/test/org/apache/coyote/http11/TestHttp11Processor.java index 8d8c777..20b6c90 100644 --- a/test/org/apache/coyote/http11/TestHttp11Processor.java +++ b/test/org/apache/coyote/http11/TestHttp11Processor.java @@ -24,6 +24,7 @@ import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.Reader; +import java.io.StringReader; import java.io.Writer; import java.net.InetSocketAddress; import java.net.Socket; @@ -33,7 +34,6 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.concurrent.CountDownLatch; @@ -58,6 +58,7 @@ import org.apache.tomcat.util.buf.B2CConverter; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.descriptor.web.SecurityCollection; import org.apache.tomcat.util.descriptor.web.SecurityConstraint; +import org.apache.tomcat.util.http.parser.TokenList; public class TestHttp11Processor extends TomcatBaseTest { @@ -570,26 +571,37 @@ public class TestHttp11Processor extends TomcatBaseTest { tomcat.start(); - byte[] requestBody = "HelloWorld".getBytes(StandardCharsets.UTF_8); - Map<String,List<String>> reqHeaders = null; + String request = + "POST /echo HTTP/1.1" + SimpleHttpClient.CRLF + + "Host: localhost:" + getPort() + SimpleHttpClient.CRLF; if (useExpectation) { - reqHeaders = new HashMap<>(); - List<String> expectation = new ArrayList<>(); - expectation.add("100-continue"); - reqHeaders.put("Expect", expectation); + request += "Expect: 100-continue" + SimpleHttpClient.CRLF; + } + request += SimpleHttpClient.CRLF + + "HelloWorld"; + + Client client = new Client(tomcat.getConnector().getLocalPort()); + client.setRequest(new String[] {request}); + + client.connect(); + client.processRequest(); + + Assert.assertTrue(client.isResponse403()); + String connectionHeaderValue = null; + for (String header : client.getResponseHeaders()) { + if (header.startsWith("Connection:")) { + connectionHeaderValue = header.substring(header.indexOf(':') + 1).trim(); + break; + } } - ByteChunk responseBody = new ByteChunk(); - Map<String,List<String>> responseHeaders = new HashMap<>(); - int rc = postUrl(requestBody, "http://localhost:"; + getPort() + "/echo", - responseBody, reqHeaders, responseHeaders); - Assert.assertEquals(HttpServletResponse.SC_FORBIDDEN, rc); - List<String> connectionHeaders = responseHeaders.get("Connection"); if (useExpectation) { + List<String> connectionHeaders = new ArrayList<>(); + TokenList.parseTokenList(new StringReader(connectionHeaderValue), connectionHeaders); Assert.assertEquals(1, connectionHeaders.size()); - Assert.assertEquals("close", connectionHeaders.get(0).toLowerCase(Locale.ENGLISH)); + Assert.assertEquals("close", connectionHeaders.get(0)); } else { - Assert.assertNull(connectionHeaders); + Assert.assertNull(connectionHeaderValue); } } @@ -1498,6 +1510,105 @@ public class TestHttp11Processor extends TomcatBaseTest { } + @Test + public void testKeepAliveHeader01() throws Exception { + doTestKeepAliveHeader(false, 3000, 10); + } + + @Test + public void testKeepAliveHeader02() throws Exception { + doTestKeepAliveHeader(true, 5000, 1); + } + + @Test + public void testKeepAliveHeader03() throws Exception { + doTestKeepAliveHeader(true, 5000, 10); + } + + @Test + public void testKeepAliveHeader04() throws Exception { + doTestKeepAliveHeader(true, -1, 10); + } + + @Test + public void testKeepAliveHeader05() throws Exception { + doTestKeepAliveHeader(true, -1, 1); + } + + @Test + public void testKeepAliveHeader06() throws Exception { + doTestKeepAliveHeader(true, -1, -1); + } + + private void doTestKeepAliveHeader(boolean sendKeepAlive, int keepAliveTimeout, + int maxKeepAliveRequests) throws Exception { + Tomcat tomcat = getTomcatInstance(); + + tomcat.getConnector().setAttribute("keepAliveTimeout", keepAliveTimeout); + tomcat.getConnector().setAttribute("maxKeepAliveRequests", maxKeepAliveRequests); + + // 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: localhost:" + getPort() + SimpleHttpClient.CRLF; + + if (sendKeepAlive) { + request += "Connection: keep-alive" + SimpleHttpClient.CRLF; + } + + request += SimpleHttpClient.CRLF; + + Client client = new Client(tomcat.getConnector().getLocalPort()); + client.setRequest(new String[] {request}); + + client.connect(); + client.processRequest(false); + + Assert.assertTrue(client.isResponse200()); + + String connectionHeaderValue = null; + String keepAliveHeaderValue = null; + for (String header : client.getResponseHeaders()) { + if (header.startsWith("Connection:")) { + connectionHeaderValue = header.substring(header.indexOf(':') + 1).trim(); + } + if (header.startsWith("Keep-Alive:")) { + keepAliveHeaderValue = header.substring(header.indexOf(':') + 1).trim(); + } + } + + if (!sendKeepAlive || keepAliveTimeout < 0 + && (maxKeepAliveRequests < 0 || maxKeepAliveRequests > 1)) { + Assert.assertNull(connectionHeaderValue); + Assert.assertNull(keepAliveHeaderValue); + } else { + List<String> connectionHeaders = new ArrayList<>(); + TokenList.parseTokenList(new StringReader(connectionHeaderValue), connectionHeaders); + + if (sendKeepAlive && keepAliveTimeout > 0 && + (maxKeepAliveRequests < 0 || maxKeepAliveRequests > 1)) { + Assert.assertEquals(1, connectionHeaders.size()); + Assert.assertEquals("keep-alive", connectionHeaders.get(0)); + Assert.assertEquals("timeout=" + keepAliveTimeout / 1000L, keepAliveHeaderValue); + } + + if (sendKeepAlive && maxKeepAliveRequests == 1) { + Assert.assertEquals(1, connectionHeaders.size()); + Assert.assertEquals("close", connectionHeaders.get(0)); + Assert.assertNull(keepAliveHeaderValue); + } + } + } + + /** * Test servlet that prints out the values of * HttpServletRequest.getServerName() and diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 3b333b5..4d9c0f7 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -52,6 +52,13 @@ </fix> </changelog> </subsection> + <subsection name="Coyote"> + <changelog> + <add> + <bug>63835</bug>: Add support for Keep-Alive header. (michaelo) + </add> + </changelog> + </subsection> <subsection name="Web applications"> <changelog> <add> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org