httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <trawi...@bellsouth.net>
Subject Re: cvs commit: apache-2.0/src/main http_protocol.c
Date Thu, 12 Oct 2000 11:55:32 GMT
rbb@covalent.net writes:

> > Don't you want to hold onto remaining buckets between invocations?
> > Here is the set of changes I was soon to commit to get POSTs working
> > again :) 
> 
> No, there is no reason to keep c->remaining intact between
> invocations.  The whole point of c->remaining is to say "grab no more than
> this much data from the bucket brigade and return it to me."  Once we have
> returned that much data, the remaining field is invalid.

I wasn't talking about c->remaining...  I was talking about any extra
data delivered by http_filter().

I realize that ap_get_client_block() *tries* to get http_filter() not
to deliver any more data than can be stored in the handler's buffer,
using the following line:

    len_to_read = (r->remaining > bufsiz) ? bufsiz : r->remaining;

But a filter can sit between ap_get_client_block() and http_filter(),
such that the number of bytes delivered to ap_get_client_block() isn't
so close to the number of bytes shipped on the wire.  Consider a
gunzip-type transformation.

So currently, if caller passes 8K buffer and posted data was 16K, we
set c->remaining to 8K and call the next filter.  Eventually we get to
http_filter() which returns up to 8K.  It then gets gunziped, turning
in to 27K.  Thus ap_get_client_block() can't store everything in its
caller's buffer and thus has data remaining to hold onto until the
next invocation. 

Now of course r->remaining can't be safely decremented by
ap_get_client_block() because ap_get_client_block() sees byte counts
*after* any filtering has been done.  All that r->remaining can be
used for by ap_(get|set)_client_block() is to initialize c->remaining.
Then c->remaining is further set/decremented only by http_filter().
ap_get_client_block() stops calling the next filter when c->remaining
reaches zero.  BUT...   This doesn't work because
ap_get_client_block() doesn't know whether or not there is some data
held onto by filters between it and http_filter(). It would seem that
http_filter() needs to know to return an empty brigade (or an EOS
bucket or something) if it is called (in)directly by
ap_get_client_block() when c->remaining is zero.  That allows any
filters above http_filter() to do the right thing and return their
final buffers up the chain, eventually to ap_get_client_block(). 

> > Index: main/http_protocol.c
> > ===================================================================
> > RCS file: /home/cvspublic/apache-2.0/src/main/http_protocol.c,v
> > retrieving revision 1.160
> > diff -u -r1.160 http_protocol.c
> > --- main/http_protocol.c        2000/10/12 02:54:38     1.160
> > +++ main/http_protocol.c        2000/10/12 04:18:32
> > @@ -2398,10 +2398,23 @@
> >      apr_status_t rv;
> >      apr_int32_t timeout;
> >  
> > +    if (!r->connection->input_data) {
> > +        /* XXX used only by ap_get_client_block(), lifetime is request;
> > +         * move from c to r and fix the pool
> > +         */
> > +        r->connection->input_data = ap_brigade_create(r->connection->pool);
> > +    }
> 
> I stopped using r->connection->input_data, because it isn't this filter's
> to use.  In fact, this can actually go away now.  The whole point of this
> brigade, was to give the core filter a place to store information.  It is
> incorrect for any filter to have access to a brigade in the
> conn_rec.

I certainly agree that these code shouldn't be messing around in the
conn_rec...  (See the XXX comment above.)  I think the interesting
issue is the one of left-over data mentioned above.

-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...

Mime
View raw message