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:30:11 UTC
[tomcat] branch 8.5.x 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 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 1412fc5 BZ 63835: Add support for Keep-Alive header
1412fc5 is described below
commit 1412fc5c2db85afa195dc81f97e93b99b8cd1cae
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 | 5 +
.../apache/coyote/http11/TestHttp11Processor.java | 141 ++++++++++++++++++---
webapps/docs/changelog.xml | 3 +
5 files changed, 155 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/coyote/http11/Constants.java b/java/org/apache/coyote/http11/Constants.java
index 1383454..2b840ff 100644
--- a/java/org/apache/coyote/http11/Constants.java
+++ b/java/org/apache/coyote/http11/Constants.java
@@ -124,6 +124,7 @@ public final class Constants {
ByteChunk.convertToBytes("HTTP/1.1 100 Continue" + CRLF + CRLF);
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 fe3487f..c4210fb 100644
--- a/java/org/apache/coyote/http11/Http11Processor.java
+++ b/java/org/apache/coyote/http11/Http11Processor.java
@@ -1145,8 +1145,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 1020985..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);
}
diff --git a/test/org/apache/coyote/http11/TestHttp11Processor.java b/test/org/apache/coyote/http11/TestHttp11Processor.java
index 7759df6..f3d7d8f 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;
@@ -60,6 +60,7 @@ import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.compat.JreCompat;
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 {
@@ -574,26 +575,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);
}
}
@@ -1514,6 +1526,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 340c7ff..48a0e50 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -116,6 +116,9 @@
Ensure that only a full token is matched and that the match is case
insensitive. (markt)
</fix>
+ <add>
+ <bug>63835</bug>: Add support for Keep-Alive header. (michaelo)
+ </add>
<fix>
<bug>63864</bug>: Refactor parsing of the <code>transfer-encoding</code>
request header to use the shared parsing code and reduce duplication.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org