httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject Re: svn commit: r1482918 - in /httpd/httpd/trunk: modules/http/http_filters.c server/protocol.c
Date Thu, 25 Jul 2013 20:16:55 GMT
On 15.05.2013 17:46, minfrin@apache.org wrote:
> Author: minfrin
> Date: Wed May 15 15:46:01 2013
> New Revision: 1482918
> 
> URL: http://svn.apache.org/r1482918
> Log:
> core: Stop ap_finalize_request_protocol() and ap_get_client_block() from silently
> swallowing errors from the filter stack, create error buckets and return them
> appropriately.
> 
> Modified:
>     httpd/httpd/trunk/modules/http/http_filters.c
>     httpd/httpd/trunk/server/protocol.c
> 
> Modified: httpd/httpd/trunk/modules/http/http_filters.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=1482918&r1=1482917&r2=1482918&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/http_filters.c (original)
> +++ httpd/httpd/trunk/modules/http/http_filters.c Wed May 15 15:46:01 2013

...

> @@ -1637,6 +1643,28 @@ AP_DECLARE(long) ap_get_client_block(req
>       * not be used.
>       */
>      if (rv != APR_SUCCESS) {
> +        apr_bucket *e;
> +
> +        /* work around our silent swallowing of error messages by mapping
> +         * error codes at this point, and sending an error bucket back
> +         * upstream.
> +         */
> +        apr_brigade_cleanup(bb);
> +
> +        e = ap_bucket_error_create(
> +                ap_map_http_request_error(rv, HTTP_BAD_REQUEST), NULL, r->pool,
> +                r->connection->bucket_alloc);
> +        APR_BRIGADE_INSERT_TAIL(bb, e);
> +
> +        e = apr_bucket_eos_create(r->connection->bucket_alloc);
> +        APR_BRIGADE_INSERT_TAIL(bb, e);
> +
> +        rv = ap_pass_brigade(r->output_filters, bb);
> +        if (APR_SUCCESS != rv) {
> +            ap_log_rerror(APLOG_MARK, APLOG_INFO, rv, r, APLOGNO()
> +                          "Error while writing error response");
> +        }
> +
>          /* if we actually fail here, we want to just return and
>           * stop trying to read data from the client.
>           */
> 

I have a question about this part which turned up when debugging a
mod_deflate test failure. Bear with me and my very incomplete
understanding of bucket brigades:

Is this error bucket insertion safe? In the mod_deflate test we added
spurious data to the end of a compressed request body. When processing
the request, mod_deflate inflated the request body and mod_echo_post
returned it. If the body was big enough the echoed data appeared at the
client. Now the spurious data was seen by mod_deflate and the above
error bucket inserted. This error did not show up on the client side.

Now the test case received a connection close header directly at the
beginning of the response, but I wonder what would have happened if this
had been a keep-alive connection. Would the error bucket have been
queued and returned in front of the next response? Or would the
connection have been aborted by the server in any case? I'm a little bit
nervous here because of the potential for response mixup.

If the posted body was small enough that the spurious data was found
before the first part of the response as sent out, then the server
correctly returns the 400 error page instead of the echo response.

Regards,

Rainer

Mime
View raw message