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.
> +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.
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).
> +static const char *log_bytes_sent_all(request_rec *r, char *a)
> +{
> + long sent;
> + char *date = apr_palloc(r->pool, APR_RFC822_DATE_LEN);
> +
> + ap_recent_rfc822_date(date, r->request_time);
> +
> + /* Headers that don't make it into headers tables
> + * "HTTP/x.x " + CRLF = 11 bytes
> + * "Server: " + CRLF = 10 bytes
> + * "Date: " + CRLF = 8 bytes
> + * CRLF after headers = 2 bytes --> Total = 31 bytes */
> + sent = strlen(r->status_line) + strlen(ap_get_server_version()) +
> + strlen(date) + 31;
> +
> + /* Add regular and error headers */
> + apr_table_do(get_header_len, &sent, r->headers_out, NULL);
> + apr_table_do(get_header_len, &sent, r->err_headers_out, NULL);
> +
> + /* Is the browser bug being avoided?
> + * If so, header "X-Pad: avoid browser bug" will be added and a CRLF
> + * 26 bytes in total */
> + if (sent >= 255 && sent <= 257)
> + sent += 26;
> +
> + if (!r->header_only)
> + sent += r->bytes_sent;
> +
> + return apr_psprintf(r->pool, "%ld", sent);
> +}
> +
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
