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 2012/05/30 16:11:49 UTC

svn commit: r1344266 - in /tomcat/trunk: java/org/apache/coyote/http11/AbstractHttp11Processor.java test/org/apache/coyote/http11/TestAbstractHttp11Processor.java

Author: markt
Date: Wed May 30 14:11:49 2012
New Revision: 1344266

URL: http://svn.apache.org/viewvc?rev=1344266&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53169
Allow servlets to opt to avoid chunked encoding with a response of unknown length by setting the Connection: close header.
Based on a patch suggested by Philippe Marschall.

Modified:
    tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
    tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java

Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1344266&r1=1344265&r2=1344266&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Wed May 30 14:11:49 2012
@@ -1131,7 +1131,7 @@ public abstract class AbstractHttp11Proc
         MimeHeaders headers = request.getMimeHeaders();
 
         // Check connection header
-        MessageBytes connectionValueMB = headers.getValue("connection");
+        MessageBytes connectionValueMB = headers.getValue(Constants.CONNECTION);
         if (connectionValueMB != null) {
             ByteChunk connectionValueBC = connectionValueMB.getByteChunk();
             if (findBytes(connectionValueBC, Constants.CLOSE_BYTES) != -1) {
@@ -1370,13 +1370,17 @@ public abstract class AbstractHttp11Proc
         }
 
         long contentLength = response.getContentLengthLong();
+        boolean connectionClosePresent = false;
         if (contentLength != -1) {
             headers.setValue("Content-Length").setLong(contentLength);
             getOutputBuffer().addActiveFilter
                 (outputFilters[Constants.IDENTITY_FILTER]);
             contentDelimitation = true;
         } else {
-            if (entityBody && http11) {
+            // If the response code supports an entity body and we're on
+            // HTTP 1.1 then we chunk unless we have a Connection: close header
+            connectionClosePresent = isConnectionClose(headers);
+            if (entityBody && http11 && !connectionClosePresent) {
                 getOutputBuffer().addActiveFilter
                     (outputFilters[Constants.CHUNKED_FILTER]);
                 contentDelimitation = true;
@@ -1422,7 +1426,11 @@ public abstract class AbstractHttp11Proc
         // Connection: close header.
         keepAlive = keepAlive && !statusDropsConnection(statusCode);
         if (!keepAlive) {
-            headers.addValue(Constants.CONNECTION).setString(Constants.CLOSE);
+            // Avoid adding the close header twice
+            if (!connectionClosePresent) {
+                headers.addValue(Constants.CONNECTION).setString(
+                        Constants.CLOSE);
+            }
         } else if (!http11 && !error) {
             headers.addValue(Constants.CONNECTION).setString(Constants.KEEPALIVE);
         }
@@ -1447,6 +1455,14 @@ public abstract class AbstractHttp11Proc
 
     }
 
+    private boolean isConnectionClose(MimeHeaders headers) {
+        MessageBytes connection = headers.getValue(Constants.CONNECTION);
+        if (connection == null) {
+            return false;
+        }
+        return connection.equals(Constants.CLOSE);
+    }
+
     abstract boolean prepareSendfile(OutputFilter[] outputFilters);
 
     /**

Modified: tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java?rev=1344266&r1=1344265&r2=1344266&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java (original)
+++ tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java Wed May 30 14:11:49 2012
@@ -16,20 +16,28 @@
  */
 package org.apache.coyote.http11;
 
-import java.io.File;
-import java.io.IOException;
-
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
-import org.junit.Test;
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.apache.catalina.Context;
 import org.apache.catalina.startup.SimpleHttpClient;
 import org.apache.catalina.startup.TesterServlet;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+import org.junit.Test;
 
 public class TestAbstractHttp11Processor extends TomcatBaseTest {
 
@@ -239,6 +247,105 @@ public class TestAbstractHttp11Processor
         assertEquals("OK", client.getResponseBody());
     }
 
+
+    @Test
+    public void testChunking11NoContentLength() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // Must have a real docBase - just use temp
+        Context ctxt = tomcat.addContext("",
+                System.getProperty("java.io.tmpdir"));
+
+        Tomcat.addServlet(ctxt, "NoContentLengthFlushingServlet",
+                new NoContentLengthFlushingServlet());
+        ctxt.addServletMapping("/test", "NoContentLengthFlushingServlet");
+
+        tomcat.start();
+
+        ByteChunk responseBody = new ByteChunk();
+        Map<String,List<String>> responseHeaders =
+                new HashMap<String,List<String>>();
+        int rc = getUrl("http://localhost:" + getPort() + "/test", responseBody,
+                responseHeaders);
+
+        assertEquals(HttpServletResponse.SC_OK, rc);
+        assertTrue(responseHeaders.containsKey("Transfer-Encoding"));
+        List<String> encodings = responseHeaders.get("Transfer-Encoding");
+        assertEquals(1, encodings.size());
+        assertEquals("chunked", encodings.get(0));
+    }
+
+    @Test
+    public void testNoChunking11NoContentLengthConnectionClose()
+            throws Exception {
+
+        Tomcat tomcat = getTomcatInstance();
+
+        // Must have a real docBase - just use temp
+        Context ctxt = tomcat.addContext("",
+                System.getProperty("java.io.tmpdir"));
+
+        Tomcat.addServlet(ctxt, "NoContentLengthConnectionCloseFlushingServlet",
+                new NoContentLengthConnectionCloseFlushingServlet());
+        ctxt.addServletMapping("/test",
+                "NoContentLengthConnectionCloseFlushingServlet");
+
+        tomcat.start();
+
+        ByteChunk responseBody = new ByteChunk();
+        Map<String,List<String>> responseHeaders =
+                new HashMap<String,List<String>>();
+        int rc = getUrl("http://localhost:" + getPort() + "/test", responseBody,
+                responseHeaders);
+
+        assertEquals(HttpServletResponse.SC_OK, rc);
+
+        assertTrue(responseHeaders.containsKey("Connection"));
+        List<String> connections = responseHeaders.get("Connection");
+        assertEquals(1, connections.size());
+        assertEquals("close", connections.get(0));
+
+        assertFalse(responseHeaders.containsKey("Transfer-Encoding"));
+
+        assertEquals("OK", responseBody.toString());
+    }
+
+    // flushes with no content-length set
+    // should result in chunking on HTTP 1.1
+    private static final class NoContentLengthFlushingServlet
+            extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            resp.setStatus(HttpServletResponse.SC_OK);
+            resp.setContentType("text/plain");
+            resp.getWriter().write("OK");
+            resp.flushBuffer();
+        }
+    }
+
+    // flushes with no content-length set but sets Connection: close header
+    // should no result in chunking on HTTP 1.1
+    private static final class NoContentLengthConnectionCloseFlushingServlet
+            extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            resp.setStatus(HttpServletResponse.SC_OK);
+            resp.setContentType("text/event-stream");
+            resp.addHeader("Connection", "close");
+            resp.flushBuffer();
+            resp.getWriter().write("OK");
+            resp.flushBuffer();
+        }
+    }
+
     private static final class Client extends SimpleHttpClient {
 
         public Client(int port) {



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