double eos buckets in request input filters ap_http_filter

[Stas Bekman <st...@stason.org>]

Though these things aren't documented, it's been repeated here many times 
that there should be only one EOS bucket seen by filters. I'm working on 
several filter examples and to protect my filters from the bad behavior of 
other filters I see no other choice but maintaining my own eos_sent flag 
in the filter context. Unfortunately this adds clutter to the code, but 
what can I do.

The latest cvs version of ap_http_filter sends the EOS bucket twice. I see 
the following in my input request (resource) filter that happens to read 
some POSTed data.

bb1: [data] [eos]
bb2: [eos]

We are now in http_protocol.c: ap_http_filter.

The first eos is tacked here:

     /* If we have no more bytes remaining on a C-L request,
      * save the caller a roundtrip to discover EOS.
      */
     if (ctx->state == BODY_LENGTH && ctx->remaining == 0) {
         e = apr_bucket_eos_create(f->c->bucket_alloc);
         APR_BRIGADE_INSERT_TAIL(b, e);
     }

notice that ctx->eos_sent is *not* set, which IMHO is wrong. Moreover all 
other code in ap_http_filter never checks whether ctx->eos_sent is set.

I've tried to add the missing checks, but it breaks, because the roundtrip 
happens all the time, even though there is no more data and if you don't 
send EOS down the stream but just return APR_SUCCESS in:

     if (!ctx->remaining) {
         switch (ctx->state) {
         case BODY_NONE:
             break;
         case BODY_LENGTH:
             if (ctx->eos_sent == 0) {
                 ctx->eos_sent = 1;
                 e = apr_bucket_eos_create(f->c->bucket_alloc);
                 APR_BRIGADE_INSERT_TAIL(b, e);
             }
             return APR_SUCCESS;
      ...

the whole mechanism breaks. So currently the only simple fix I see is not 
to try to save the round trip:

     if (ctx->state == BODY_LENGTH && ctx->remaining == 0) {
        /* e = apr_bucket_eos_create(f->c->bucket_alloc); */
        /* APR_BRIGADE_INSERT_TAIL(b, e); */
     }

Why the round trip happens again? After all there is no more data left in 
the pipe.

You can easily see the too many EOS sends by breaking at 
apr_bucket_eos_create, at least that's how I was debugging it.

So unless I've missed something that situation should be fixed. Also it'd 
be nice to add if (!ctx->eos_sent) checks when eos buckets are sent.

Moreover there is this seemingly illogical piece of code:

     if (ctx->eos_sent) {
         e = apr_bucket_eos_create(f->c->bucket_alloc);
         APR_BRIGADE_INSERT_TAIL(b, e);
         return APR_SUCCESS;
     }

why would you want to send EOS when the flag indicates that it has already 
been sent? I did a few tests and it seems that killing this code changes 
nothing. I could be wrong.

Thanks.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com