httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bojan Smojver <bo...@rexursive.com>
Subject Re: [PATCH]: Update request in connection based filters
Date Wed, 18 Sep 2002 01:30:27 GMT
Quoting Justin Erenkrantz <jerenkrantz@apache.org>:

> Well, I'd do (plus the declaration of f in your later post):
> 
> for (f = conn->input_filters; f; f = f->next) {
>    f->r = r;
> }
> 
> for (f = conn->output_filters; f; f = f->next) {
>    f->r = r;
> }

What was that book again... K&R, The C Programming Language. Maybe I should find
out where it went :-)

I have changed the code of mod_logio.c significantly in the past few hours,
throwing out the request based filters altogether and just keeping the
connection based filters, now that f->r is not NULL any more. The whole thing
turned out to be much simpler and very accurate on simple requests. I'm yet to
do heavy testing, but I reckon even things like SSL should be covered.

> Now, the question is whether anyone has any major qualms with this.
> Basically, it now allows connection filters some access to the
> current request_rec (as defined by the protocol module).  
> 
> Internal redirects would be handled by the update_r_in_filters func
> in http_request.c (which is equivalent to this for statement - hmm,
> I forsee ap_update_filters in our future), so that should be fine.

I'm not sure that's the case. It only updates content and protocol filters (i.e.
the ones tied to the request_rec), but it doesn't touch connection filters. I
reckon we should do the above for internal redirects as well. That is, if I
really understand what internal redirects are in the first place :-)

> The only potential problem I forsee is whether the input bytes could
> be logged on one request, and the output on another.  Therefore, we
> might want to walk r->prev and add the totals to get our real bytes
> read and sent.  Oooh, internal_internal_redirect copies the
> r->read_length to the new request, so I think we're fine.

I'll try to test that by using some funcionality that depends on
internal_internal_redirect. From what I see in internal_internal_redirect, the
new->notes gets created afresh, so whatever I put in r->notes is going to get
lost. Big dramas! Looks like we'll *have to* introduce r->bytes_in and
r->bytes_out if we want this to work at all...

This would, of course, require yet another MMN bump... And it would require
revisiting all of the code that deals with copying request (e.g.
internal_internal_redirect) and then making sure that those fields are copied as
well. It also might make a lot of people upset... :-( On the other hand, it's
going to make mod_logio.c almost trivial and probably faster.

> For the logging case, I think this makes a lot of sense as it lets
> us simplify how we log.

I can verify that it is almost impossible to log the correct number of output
bytes (on the wire but per request) unless the requests are tied to connection
filters. Input bytes are different, because request based filters run after
connection based filters and we can always do what I was doing before (i.e.
store in c->notes and then copy into r->whatever). However, request based output
filters are already out of the way when connection based filters take over and
if f->r is NULL, it is not possible to store the number of bytes into the
request. So, the above patch and other neccessary additions would be essential
to achieve this.

Bojan

-------------------------------------------------
This mail sent through IMP: http://horde.org/imp/

Mime
View raw message