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: Apache 2.0] mod_log_config: input/output bytes
Date Tue, 20 Aug 2002 09:38:07 GMT
On Tue, 2002-08-20 at 16:20, Justin Erenkrantz wrote:
> On Tue, Aug 20, 2002 at 03:46:20PM +1000, Bojan Smojver wrote:
> > +static int get_header_len(void *count, const char *key, const char *val) {
> > +
> > +    /* Header name, colon, space, header value, CRLF */
> > +    *((long *)count) += strlen(key) + strlen(val) + 4;
> > +            
> > +    return 1;
> > +}
> 
> This incorrectly assumes that the header has a space after the value
> name.  That isn't strictly required, so, on edge cases, you will add
> an extra count.  That isn't good - especially if you are charging
> per input byte - the client would *not* want to send the space if
> possible.

OOPS! Good point. Not that I'm worried about it since the extra cash
ends up in my pocket ;-)

> > +static const char *log_bytes_recv_all(request_rec *r, char *a)
> > +{
> > +    /* Request length, CRLF and another CRLF after headers */
> > +    long recv = strlen(r->the_request) + 4; /* Never zero */
> > +    const char *clen = apr_table_get(r->headers_in, "Content-Length" );
> > +    
> > +    /* Add length of headers */
> > +    apr_table_do(get_header_len, &recv, r->headers_in, NULL);
> > +
> > +    if (clen) /* We checked the value makes sense in http_protocol.c */
> > +        recv += atol(clen);
> > +
> > +    return apr_psprintf(r->pool, "%ld", recv);
> > +}
> 
> The problem here is that the length may not be specified via C-L,
> but rather Transfer-Encoding: chunked.  This system falls down
> in that case.  This is why I'd strongly recommend looking at
> trying to add r->bytes_read (or perhaps to a mod_log_config specific
> structure) to core_input_filter.

Yep, by all means more accurate. The headers could be lying anyway...

> If you read some of Brian Pane and rbb's recent posts, r->bytes_sent
> needs to move to core_output_filter, so I think something similar
> for input accounting is the way to go here.  It's ridiculously easy
> to compute the real length in core_input_filter as it knows how much
> it just read (or just do an apr_brigade_length call on b before
> returning it).

> And, of course, this should simply be the reworked r->bytes_sent
> once it gets moved to the correct place - i.e. core_output_filter.  
> 
> If you'd like to beat either Brian or rbb (both I'm sure would
> appreciate the help), a 2.0 patch which does the right thing for
> r->bytes_sent would be totally welcomed.  =)  -- justin

Yes, I fully agree. I started this campaign off by asking about the
field that holds the number of bytes received, since I couldn't find
anything like that in the request. Later on (by looking into
mod_accounting), I realised that bytes_sent is also very inaccurate
(actually it is totally wrong for HEAD requests).

I guess the best way to fix all this is:

- make bytes_sent actual bytes sent down the pipe, not the body length
- introduce bytes_recv
- populate both through the above machinery or whatever else that
actually knows what was received from and sent to the client

That is, if changing request_rec is allowed :-)

Can promise beating anything when it comes to writing Apache code, since
I'm a total newcomer, but I think things are moving in the right
direction.

Bojan


Mime
View raw message