httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ryan Bloom <...@covalent.net>
Subject Re: [PATCH] Rework httpd buckets/filtering
Date Sun, 23 Sep 2001 22:04:54 GMT
On Sunday 23 September 2001 02:53 pm, Greg Stein wrote:
> On Sun, Sep 23, 2001 at 08:33:07AM -0700, Ryan Bloom wrote:
> > None of this can be committed until it has passed all of the tests in the
> > perl test framework. The last time somebody (me) mucked with these
> > filters it broke all of the CGI script handling. I would also like to
> > hear from Jeff about this, because when we wrote the filters originally,
> > we had input filtering use this design, and Jeff and Greg were adamant
> > that filters needed to be able to return more than they were asked for.
>
> Euh... not sure if you typoed. Input filters should *NOT* return more than
> they were asked for. Never.
>
> If a filter did that, it could end up passing data up to a filter that is
> not responsible for that data. For example, reading past the end of a
> request, or reading past the headers yet that portion of the body was
> supposed to go into a newly-inserted unzip or dechunk filter.
>
> I can think of more examples, if called upon :-)

And I am saying, that was the original design, and Jeff Trawick and Greg
Ames posted a couple of examples that proved that design incorrect.  I 
can't remember them right now, and I don't have time to review all of the
archives, but the original design had input filters only ever returning the 
amount of data requested, and it broke their modules.  The way the current
design makes sure that we don't read past the end of the request, is by 
having a single filter that kept track of where the end of the request is.

> The current input filter code *does* return too much, though, and that is
> one of its problems. I'm not sure that this change improves upon that (need
> to review some more, me thinks).
>
> A filter can return *less* than what was asked for, but never more.
>
> >...
> >
> > > --- include/apr_buckets.h	2001/09/22 22:36:07	1.118
> > > +++ include/apr_buckets.h	2001/09/23 10:36:12
> > > @@ -689,6 +689,17 @@
> > >                                               apr_off_t *length);
> > >
> > >  /**
> > > + * create a flat buffer of the elements in a bucket_brigade.
> > > + * @param b The bucket brigade to create the buffer from
> > > + * @param c The pointer to the returned character string
> > > + * @param len The length of the returned character string
> > > + * @param p The pool to allocate the character string from.
> > > + * Note: You *must* have enough space allocated to store all of the
> > > data. + * See apr_brigade_length().
> > > + */
> > > +APU_DECLARE(apr_status_t) apr_brigade_to_buffer(apr_bucket_brigade *b,
> > > char *c); +
>
> Ryan said this should take a reading mode. I don't understand why that
> should be necessary. You have a brigade of a defined size, and you're
> flattening that much data. You're already past the point of non-blocking
> reads being used.
>
> If you had a partial read, then how would you communiate that? And don't
> say strlen(c).
>
> No... the function does not need a mode, nor an output length. You give it
> a brigade and it flattens it in its entirety. If there is a problem getting
> the whole thing flattened, then it returns an error.
>
> Personally, I would pass in a length of the output buffer so the function
> can verify that it is flattening the proper amount -- not too little, and
> not overwriting.

That would be true if this function were to go into httpd, but it isn't.  It is
going into APR-util, and there is nothing saying that you will have a
brigade of a defined size.

Ryan

______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Mime
View raw message