input filtering (was: cvs commit: ....)

[Greg Stein <gs...@lyra.org>]

On Tue, May 08, 2001 at 07:36:40AM -0700, rbb@covalent.net wrote:
> On Tue, 8 May 2001, Greg Stein wrote:
>...
> > This was premature, Ryan. I don't think you should have done this. My change
> > exposed further problems. It didn't create them.
> 
> It wasn't premature, because what you did broke the code, so that some of
> my stuff didn't work.  And, you did it without specifying any future steps
> to fix it.

Sorry about that... I didn't realize that it had broken any code. I saw it
as a step in the right direction, but didn't realize that the next step(s)
*had* to be done.

>...
> This only works if you finish what you have described.  Had I known what
> you were proposing, I probably would have finished it for you, but I
> didn't.

Well, part of my problem was that had we had a chance to talk about it,
rather than just a revision, then I would have been able to explain. This is
partly why I like to ask the person to back it out themselves. It gives a
chance for all parties to explain what is going on. i.e. presume that a
checkin had good intent, so if it broke something then figure out what the
person was trying to do.

> And, when I commented that your change was incorrect, I waited a
> day or two and didn't hear anything,

Out of town :-)

> so I reverted the code so that it would at least work.

Not sure what the right answer would have been.

>...
> > 1) the indirection on the prototype should go away (which I did)
> > 2) the C-L handling needs to move from ap_get_client_block() down to a
> >    low-level request input filter.
>...
> This is all well and good, but it only works if you explain #2 when
> committing #1, because otherwise nobody knows what your end-design is.

Right. But as I said: I didn't know it would be a problem :-)

Okay... are there any objections with re-applying my patch (1), as long as
(2) is also done? (at the same time)

I need to ponder a bit on the exact form of (2), but I'm thinking this is
the point to merge HTTP_IN and DECHUNK (as we discussed at Hackathon); the
combined filter would also perform task (2).

Note that r->remaining will *disappear*. Modules cannot reliably use it when
chunking is present, so it ought to go. I do see a few bogus uses to clear
up. (the variable would move into a filter context)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: input filtering (was: cvs commit: ....)

[Greg Stein <gs...@lyra.org>]

On Wed, May 09, 2001 at 02:02:18PM +0200, Graham Leggett wrote:
> Greg Stein wrote:
> 
> > I need to ponder a bit on the exact form of (2), but I'm thinking this is
> > the point to merge HTTP_IN and DECHUNK (as we discussed at Hackathon); the
> > combined filter would also perform task (2).
> 
> The proxy_http module uses the DECHUNK filter - but only because it has
> code to detect whether the transfer encoding was chunked on incoming
> data and add it if necessary.
> 
> If the HTTP_IN filter can worry about DECHUNK automatically then we can
> rip the logic out of proxy.

Yes, I imagine the DECHUNK goes away in favor of HTTP_IN. The HTTP_IN filter
then gets placed at the correct point in the filter stack:

    Connection Filters:    CORE_IN
                              |
                           [ TLS ]
                              |
                         user filters
                              |
                  < this is c->input_filters >
                              |
    Request Filters:       HTTP_IN
                              |
                         user filters
                              |
                  < this is r->input_filters >
                              |
                      ap_get_client_block


I don't think we have any other input filters at the moment.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: input filtering (was: cvs commit: ....)

[Graham Leggett <mi...@sharp.fm>]

Greg Stein wrote:

> I need to ponder a bit on the exact form of (2), but I'm thinking this is
> the point to merge HTTP_IN and DECHUNK (as we discussed at Hackathon); the
> combined filter would also perform task (2).

The proxy_http module uses the DECHUNK filter - but only because it has
code to detect whether the transfer encoding was chunked on incoming
data and add it if necessary.

If the HTTP_IN filter can worry about DECHUNK automatically then we can
rip the logic out of proxy.

Regards,
Graham
-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."