Re: Handling AP_FILTER_ERROR

2008-12-01 Thread Eric Covener
On Mon, Dec 1, 2008 at 7:33 AM, Eric Covener <[EMAIL PROTECTED]> wrote: > On Mon, Dec 1, 2008 at 6:44 AM, Nick Kew <[EMAIL PROTECTED]> wrote: >> >> On 1 Dec 2008, at 10:19, Ruediger Pluem wrote: >> >>> if (eb) { >>> -ap_die(eb->status, r); >>> +int status; >>> + >>> +sta

Re: Handling AP_FILTER_ERROR

2008-12-01 Thread Eric Covener
On Mon, Dec 1, 2008 at 6:44 AM, Nick Kew <[EMAIL PROTECTED]> wrote: > > On 1 Dec 2008, at 10:19, Ruediger Pluem wrote: > >> if (eb) { >> -ap_die(eb->status, r); >> +int status; >> + >> +status = eb->status; >> +apr_brigade_cleanup(b); >> +ap_die(status, r

Re: Handling AP_FILTER_ERROR

2008-12-01 Thread Nick Kew
On 1 Dec 2008, at 10:19, Ruediger Pluem wrote: if (eb) { -ap_die(eb->status, r); +int status; + +status = eb->status; +apr_brigade_cleanup(b); +ap_die(status, r); return AP_FILTER_ERROR; } Good call. +1 to that. -- Nick Kew

Re: Handling AP_FILTER_ERROR

2008-12-01 Thread Ruediger Pluem
On 12/01/2008 11:01 AM, Nick Kew wrote: > > On 1 Dec 2008, at 09:31, Ruediger Pluem wrote: >>> >>> But do you see any objection to the (IMHO simpler) fix of >>> removing error buckets as we detect them? >> >> Not directly. Could you please propose a concrete patch? >> It makes discussion easier

Re: Handling AP_FILTER_ERROR

2008-12-01 Thread Nick Kew
On 1 Dec 2008, at 09:31, Ruediger Pluem wrote: But do you see any objection to the (IMHO simpler) fix of removing error buckets as we detect them? Not directly. Could you please propose a concrete patch? It makes discussion easier :-). I think the one-liner should work, since we don't re-us

Re: Handling AP_FILTER_ERROR

2008-12-01 Thread Ruediger Pluem
On 12/01/2008 10:02 AM, Nick Kew wrote: > > On 1 Dec 2008, at 08:52, Ruediger Pluem wrote: >> >> 2. IMHO ap_invoke_handler is only called in three locations: > > It's a public API, so could be called by any module. Correct. > >> So ap_die maybe should do the following: >> >> if (type ==

Re: Handling AP_FILTER_ERROR

2008-12-01 Thread Nick Kew
On 1 Dec 2008, at 08:52, Ruediger Pluem wrote: 2. IMHO ap_invoke_handler is only called in three locations: It's a public API, so could be called by any module. So ap_die maybe should do the following: if (type == AP_FILTER_ERROR) { if [ap_http_header_filter is still in chain]

Re: Handling AP_FILTER_ERROR

2008-12-01 Thread Ruediger Pluem
On 12/01/2008 12:42 AM, Nick Kew wrote: > On Sun, 30 Nov 2008 18:22:39 -0500 > "Eric Covener" <[EMAIL PROTECTED]> wrote: > >> On Sun, Nov 30, 2008 at 5:20 PM, Nick Kew <[EMAIL PROTECTED]> wrote: >>> In practice, the proposed fix looks good, and I want to >>> vote +1. I'm just a little concerned

Re: Handling AP_FILTER_ERROR

2008-11-30 Thread Nick Kew
On Sun, 30 Nov 2008 18:22:39 -0500 "Eric Covener" <[EMAIL PROTECTED]> wrote: > On Sun, Nov 30, 2008 at 5:20 PM, Nick Kew <[EMAIL PROTECTED]> wrote: > > In practice, the proposed fix looks good, and I want to > > vote +1. I'm just a little concerned over whether we're > > in danger of an infinite

Re: Handling AP_FILTER_ERROR

2008-11-30 Thread Eric Covener
On Sun, Nov 30, 2008 at 5:20 PM, Nick Kew <[EMAIL PROTECTED]> wrote: > In practice, the proposed fix looks good, and I want to > vote +1. I'm just a little concerned over whether we're > in danger of an infinite loop if we feed an AP_FILTER_ERROR > into ap_http_header_filter. The filter will just

Handling AP_FILTER_ERROR

2008-11-30 Thread Nick Kew
There's a proposal in STATUS to fix a bug which causes a filter error manifesting in AP_FILTER_ERROR to be treated as an error in the content generator. There's a deeper and nastier bug underlying this: filter functions are defined as returning apr_status_t, but commonly return int. Practice is i