tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1344267 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/coyote/http11/AbstractHttp11Processor.java test/org/apache/coyote/http11/TestAbstractHttp11Processor.java webapps/docs/changelog.xml
Date Mon, 04 Jun 2012 06:14:57 GMT
2012/5/30  <markt@apache.org>:
> Author: markt
> Date: Wed May 30 14:13:47 2012
> New Revision: 1344267
>
> URL: http://svn.apache.org/viewvc?rev=1344267&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/tc7.0.x/trunk/   (props changed)
>    tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
>    tomcat/tc7.0.x/trunk/test/org/apache/coyote/http11/TestAbstractHttp11Processor.java
>    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>
> Propchange: tomcat/tc7.0.x/trunk/
> ------------------------------------------------------------------------------
>  Merged /tomcat/trunk:r1344266
>
> Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
> URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1344267&r1=1344266&r2=1344267&view=diff
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
> +++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Wed
May 30 14:13:47 2012
> @@ -1135,7 +1135,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) {
> @@ -1374,13 +1374,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;
> @@ -1426,7 +1430,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);
>         }
> @@ -1451,6 +1459,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);
> +    }

Just a note: The HTTP1.1 spec allows Connection header as

Connection = "Connection" ":" 1#(connection-token)

thus allowing it to have several values, separated by commas, or
several headers.

I agree though that for a response it does not make much sense to
expect something else if it is the "close" connection header.

In AbstractHttp11Processor#prepareRequest() the code that checks the
value of Constants.CONNECTION header is also not prepared to handle
several values or several headers. Though nobody complained and it
works a bit faster this way.

>  public class TestAbstractHttp11Processor extends TomcatBaseTest {
> (...)
>+    // 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");

It could be resp.setHeader(..) in this test servlet here.

>+            resp.flushBuffer();
>+            resp.getWriter().write("OK");
>+            resp.flushBuffer();
>+        }
>+    }

Best regards,
Konstantin Kolinko

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


Mime
View raw message