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