httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: svn commit: r1484852 - in /httpd/httpd/trunk: CHANGES modules/http/http_filters.c
Date Wed, 22 May 2013 14:16:43 GMT
On 22 May 2013, at 3:57 PM, Plüm, Rüdiger, Vodafone Group <ruediger.pluem@vodafone.com>
wrote:

> I guess you missed a special case:
> 
> If the reverse proxy backend sends a response without a Content-Length header and without
a Transfer-Encoding,
> which is IMHO valid for HTTP 1.0 responses if the connection is closed after the response.
> The following patch would fix this:
> 
> Index: modules/http/http_filters.c
> ===================================================================
> --- modules/http/http_filters.c (revision 1485192)
> +++ modules/http/http_filters.c (working copy)
> @@ -394,8 +394,15 @@
>        case BODY_LENGTH:
>        case BODY_CHUNK_DATA: {
> 
> -            /* Ensure that the caller can not go over our boundary point. */
> -            if (ctx->remaining < readbytes) {
> +            /*
> +             * Ensure that the caller can not go over our boundary point,
> +             * BUT only if this is not a response by reverse proxy backend
> +             * that sent no Content-Length header and has no transfer encoding
> +             * which is valid for a non keep-alive HTTP 1.0 response.
> +             */
> +            if ((ctx->remaining < readbytes) && !((ctx->remaining ==
0) &&
> +                  (ctx->state == BODY_NONE) &&
> +                  (f->r->proxyreq == PROXYREQ_RESPONSE))) {
>                readbytes = ctx->remaining;
>            }
>            if (readbytes > 0) {
> 
> Should I commit it?

Hmmm… the check for f->r->proxyreq == PROXYREQ_RESPONSE shouldn't be necessary at
this point, as we already check for this case here:

       /* If we don't have a request entity indicated by the headers, EOS.
        * (BODY_NONE is a valid intermediate state due to trailers,
        *  but it isn't a valid starting state.)
        *
        * RFC 2616 Section 4.4 note 5 states that connection-close
        * is invalid for a request entity - request bodies must be
        * denoted by C-L or T-E: chunked.
        *
        * Note that since the proxy uses this filter to handle the
        * proxied *response*, proxy responses MUST be exempt.
        */
       if (ctx->state == BODY_NONE && f->r->proxyreq != PROXYREQ_RESPONSE)
{
           e = apr_bucket_eos_create(f->c->bucket_alloc);
           APR_BRIGADE_INSERT_TAIL(b, e);
           ctx->eos_sent = 1;
           return APR_SUCCESS;
       }

The BODY_NONE case however shouldn't be constrained as you've said, something like this?

Index: modules/http/http_filters.c
===================================================================
--- modules/http/http_filters.c	(revision 1484926)
+++ modules/http/http_filters.c	(working copy)
@@ -395,7 +395,7 @@
        case BODY_CHUNK_DATA: {

            /* Ensure that the caller can not go over our boundary point. */
-            if (ctx->remaining < readbytes) {
+            if (ctx->state != BODY_NONE && ctx->remaining < readbytes) {
                readbytes = ctx->remaining;
            }
            if (readbytes > 0) {

Regards,
Graham
--


Mime
View raw message