httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jus...@erenkrantz.com>
Subject Re: bug in ap_get_client_block (wrong handling of EOS)
Date Wed, 14 May 2003 06:08:26 GMT
--On Wednesday, May 14, 2003 3:08 PM +1000 Stas Bekman <stas@stason.org> wrote:

>> The ap_get_client_block() API can't support that semantic of a
>> partial-read given its interface.  ap_get_client_block is a kluge that
>> only exists to make people who want partial source compatibility with
>> 1.3 modules.  ap_get_brigade should be used instead...
>
> The public API documentation doesn't say anything about this.

The public API documentation doesn't have lots of things.  ;-)

> a similar patch attached to reply to Jeff's post. Your other proposal sounds
> good too (faster).

The issue is that r->remaining isn't used anywhere, but it's set by 
ap_setup_client_block.  So, I think we can use it for this purpose. 
r->remaining==0 used to mean EOS a long time ago (and was how 
ap_get_client_block knew when it was EOS and always returned 0 if that was 0).

I'm partial to my approach rather than Jeff's because it relies on EOS being 
last and doesn't add yet another field somewhere.  (IIRC, it's always been a 
point of contention whether core_request_rec is subject to MMN.  I've thought 
it shouldn't be, but I think some people said it is.  I've lost track...)

> That's cool with me. Mark those old broken functions as deprecated.

I think we should just remove them in 2.1.  Mark the ap_*_client_block API as 
deprecated (but supported) in 2.0 and outright toss 'em in 2.1.

>> No.  There is a global r->eos_sent though.
>
> but it's not the same as r->seen_eos, and it's used for output filters.

Right, but the point was that output_filters have that semantic of not sending 
EOS twice because of the r->eos_sent safeguard.  However, the r->eos_sent is a 
big giant kluge as well.  It's needed for HTTP pipelining.  It'd be nice to 
remove that if possible.

> The problem is that users will come complaining to the author of a good
> module, which may fail and not to the author of the faulty module. Hence
> it'd be nice to have a methodology for protecting ourselves from faulty
> situations.

No, I'm saying that ap_get_client_block should be fixed in 2.0 (via stated 
suggestions).  I'd just rather toss the whole darn thing in 2.1, so that we 
don't have this API headache moving forward.

A normal client of ap_get_brigade shouldn't have this problem.  If they are 
ignoring EOS, then it's their own fault and they get what they deserve. 
ap_get_client_block is a special case because it's lame.

> I suppose I can set a flag in my filters that identifies whether eos has
> been sent already, and if they get called again, simply die pointing the
> finger to the faulty filter. The problem: there is no f->prev, but only
> f->next, so there is no (easy) way to communicate this information.

The stacktrace is a good way to find out what f->prev was.  =)  -- justin

Mime
View raw message