Author: markt
Date: Thu Oct 30 15:45:21 2014
New Revision: 1635524

URL: http://svn.apache.org/r1635524
Log:
Make the disabling of chunked encoding on connection close optional and disable 
by default since automatically disabling it causes a regression (see this 
discussion http://tomcat.markmail.org/thread/zvrrtvwme6liefng for the details).

Modified:
    tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
    tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
    tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java
    tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
    tomcat/trunk/webapps/docs/changelog.xml
    tomcat/trunk/webapps/docs/config/http.xml

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=1635524&r1=1635523&r2=1635524&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Thu 
Oct 30 15:45:21 2014
@@ -1478,7 +1478,8 @@ public abstract class AbstractHttp11Proc
             // HTTP 1.1 and we are using keep-alive then we chunk unless we 
have
             // a Connection: close header
             connectionClosePresent = isConnectionClose(headers);
-            if (entityBody && http11 && keepAlive && !connectionClosePresent) {
+            if (entityBody && http11 && (keepAlive || 
!endpoint.getDisableChunkingOnClose()) &&
+                    !connectionClosePresent) {
                 getOutputBuffer().addActiveFilter
                     (outputFilters[Constants.CHUNKED_FILTER]);
                 contentDelimitation = true;

Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java?rev=1635524&r1=1635523&r2=1635524&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java Thu 
Oct 30 15:45:21 2014
@@ -207,6 +207,13 @@ public abstract class AbstractHttp11Prot
         endpoint.setMaxKeepAliveRequests(mkar);
     }
 
+    public boolean getDisableChunkingOnClose() {
+        return endpoint.getDisableChunkingOnClose();
+    }
+    public void setDisableChunkingOnClose(boolean disableChunkingOnClose) {
+        endpoint.setDisableChunkingOnClose(disableChunkingOnClose);
+    }
+
     protected NpnHandler<S> npnHandler;
     @SuppressWarnings("unchecked")
     public void setNpnHandler(String impl) {

Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java?rev=1635524&r1=1635523&r2=1635524&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java 
(original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java Thu Oct 
30 15:45:21 2014
@@ -45,6 +45,7 @@ import org.apache.tomcat.util.threads.Th
  *
  * @author Mladen Turk
  * @author Remy Maucherat
+ * @param <S> The type of socket the endpoint is associated with.
  */
 public abstract class AbstractEndpoint<S> {
 
@@ -423,6 +424,21 @@ public abstract class AbstractEndpoint<S
     }
 
     /**
+     * Should the option of using chunked transfer encoding be disabled when it
+     * is known that the connection is going to be closed at the end of the
+     * response. Disabling chunking in this case is marginally more efficient
+     * but makes it impossible for the user agent to determine if the whole
+     * response was received or if it was truncated due to an error.
+     */
+    private boolean disableChunkingOnClose = false;
+    public boolean getDisableChunkingOnClose() {
+        return disableChunkingOnClose;
+    }
+    public void setDisableChunkingOnClose(boolean disableChunkingOnClose) {
+        this.disableChunkingOnClose = disableChunkingOnClose;
+    }
+
+    /**
      * The maximum number of headers in a request that are allowed.
      * 100 by default. A value of less than 0 means no limit.
      */

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=1635524&r1=1635523&r2=1635524&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java 
(original)
+++ tomcat/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java 
Thu Oct 30 15:45:21 2014
@@ -55,6 +55,16 @@ public class TestAbstractHttp11Processor
 
     @Test
     public void testResponseWithErrorChunked() throws Exception {
+        doTestResponseWithErrorChunked(false);
+    }
+
+    @Test
+    public void testResponseWithErrorChunkedDisabled() throws Exception {
+        doTestResponseWithErrorChunked(true);
+    }
+
+
+    private void doTestResponseWithErrorChunked(boolean disabled) throws 
Exception {
         Tomcat tomcat = getTomcatInstance();
 
         // No file system docBase required
@@ -65,6 +75,14 @@ public class TestAbstractHttp11Processor
                 new ResponseWithErrorServlet(true));
         ctx.addServletMapping("/*", "ChunkedResponseWithErrorServlet");
 
+        // This setting means the connection will be closed at the end of the
+        // request
+        tomcat.getConnector().setAttribute("maxKeepAliveRequests", "1");
+
+        if (disabled) {
+            tomcat.getConnector().setAttribute("disableChunkingOnClose", 
"true");
+        }
+
         tomcat.start();
 
         String request =
@@ -81,9 +99,22 @@ public class TestAbstractHttp11Processor
         // Expected response is a 200 response followed by an incomplete 
chunked
         // body.
         assertTrue(client.isResponse200());
-        // There should not be an end chunk
+        // Should use chunked encoding
+        String transferEncoding = null;
+        for (String header : client.getResponseHeaders()) {
+            if (header.startsWith("Transfer-Encoding:")) {
+                transferEncoding = header.substring(18).trim();
+            }
+        }
+        if (disabled) {
+            Assert.assertNull(transferEncoding);
+        } else {
+            Assert.assertEquals("chunked", transferEncoding);
+        }
+        // In both cases:
+        // - there should be no end chunk
+        // - the response should end with the last text written before the 
error
         assertFalse(client.getResponseBody().endsWith("0"));
-        // The last portion of text should be there
         assertTrue(client.getResponseBody().endsWith("line03"));
     }
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1635524&r1=1635523&r2=1635524&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Oct 30 15:45:21 2014
@@ -239,8 +239,10 @@
         Async state MUST_COMPLETE should still be started. (remm)
       </fix>
       <fix>
-        Disable chunking when there is no keepalive (connection close is added
-        at the end of the method). (remm)
+        Provide a new option on the connector (disableChunkingOnClose) to
+        disable chunking when there is no keepalive (connection close is added
+        at the end of the method). This option is disabled by default.
+        (remm/markt)
       </fix>
     </changelog>
   </subsection>

Modified: tomcat/trunk/webapps/docs/config/http.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/config/http.xml?rev=1635524&r1=1635523&r2=1635524&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/config/http.xml (original)
+++ tomcat/trunk/webapps/docs/config/http.xml Thu Oct 30 15:45:21 2014
@@ -363,6 +363,18 @@
       </p>
     </attribute>
 
+    <attribute name="disableChunkingOnClose" required="false">
+      <p>Should the option of using chunked transfer encoding be disabled when
+      it is known that the connection is going to be closed at the end of the
+      response. The connection will be closed at the end of the response if the
+      client requested connection closure or if
+      <strong>maxKeepAliveRequests</strong> has been reached. Disabling 
chunking
+      is marginally more efficient but makes it impossible for the user agent 
to
+      determine if the whole response was received or if it was truncated due 
to
+      an error. If not specified, the default value of <code>false</code> will
+      be used.</p>
+    </attribute>
+
     <attribute name="disableUploadTimeout" required="false">
       <p>This flag allows the servlet container to use a different, usually
       longer connection timeout during data upload. If not specified, this



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

Reply via email to