tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From micha...@apache.org
Subject [tomcat] 01/01: BZ 63835: Add support for Keep-Alive header
Date Sun, 10 Nov 2019 00:23:20 GMT
This is an automated email from the ASF dual-hosted git repository.

michaelo pushed a commit to branch BZ-63835/9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit bf3f9515495476d480765c183a3442d8e3d5f129
Author: Michael Osipov <michaelo@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  | 140 ++++++++++++++++++---
 4 files changed, 152 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 7febf50..8923b9b 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 {
 
@@ -575,26 +576,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);
         }
     }
 
@@ -1503,6 +1515,104 @@ 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);
+    }
+
+    public 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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message