On 22 May 2013, at 3:57 PM, Plüm, Rüdiger, Vodafone Group
<[email protected]> 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
--
