You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by mi...@apache.org on 2019/11/14 22:29:58 UTC
[tomcat] branch master updated: BZ 63835: Add support for
Keep-Alive header
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 <mi...@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org