The proposed patch is dead anyway, just one remark though.
On Tue, Oct 20, 2015 at 10:08 AM, Plüm, Rüdiger, Vodafone Group
<[email protected]> wrote:
>
>
>> -----Original Message-----
>> From: Yann Ylavic [mailto:[email protected]]
>> Sent: Dienstag, 20. Oktober 2015 01:05
>>
>> Index: modules/http/http_request.c
>> ===================================================================
[]
>> --- modules/http/http_request.c (revision 1708095)
>> +++ modules/http/http_request.c (working copy)
>>
>> - if (!c->data_in_input_filters) {
>> - bb = apr_brigade_create(c->pool, c->bucket_alloc);
>> - b = apr_bucket_flush_create(c->bucket_alloc);
>> - APR_BRIGADE_INSERT_HEAD(bb, b);
>> - rv = ap_pass_brigade(c->output_filters, bb);
>> - if (APR_STATUS_IS_TIMEUP(rv)) {
>> - /*
>> - * Notice a timeout as an error message. This might be
>> - * valuable for detecting clients with broken network
>> - * connections or possible DoS attacks.
>> - *
>> - * It is still safe to use r / r->pool here as the eor bucket
>> - * could not have been destroyed in the event of a timeout.
>> - */
>> - ap_log_rerror(APLOG_MARK, APLOG_INFO, rv, r, APLOGNO(01581)
>> - "Timeout while writing data for URI %s to the"
>> - " client", r->unparsed_uri);
>> - }
>> - }
>> if (ap_extended_status) {
>> + conn_rec *c = r->connection;
>
> r is likely to be dead here
Correct, and I think the above code is quite risky too.
Does it really worth logging at the request level here, whereas using
something like:
ap_log_cerror(APLOG_MARK, APLOG_INFO, rv, c, APLOGNO(01581)
"Timeout flusing data to the client");
would be as much informative IMHO.
