John Boero <[EMAIL PROTECTED]> wrote Wed, Jun 06, 2007:
> Hi. I distribute sabayon isos through my shared hosting account. It's
> quickest for me to ssh into the host and download from my source
> directly (thanks to links). This time I noticed quirky estimates on my
> larger files. It looks like there's a signed int for tracking bytes
> received.
Could you tell us about what version of ELinks you are using?
> Could this be fixed by making it a long?
The off_t type should be used for this kind of thing; it seems Witold
already made such preparations.
> No big deal, just thought I'd make it known.
Thank you for reporting this.
... and now some comments to the patch
> From 6e43bca585238183c6a3f04695b48b755b1b52b6 Mon Sep 17 00:00:00 2001
> From: Witold Filipczyk <[EMAIL PROTECTED]>
> Date: Thu, 7 Jun 2007 16:16:53 +0200
> Subject: [PATCH] http: used off_t instead of int.
>
> The progress of files bigger than 2GB is shown properly.
> ---
> src/protocol/http/http.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/protocol/http/http.c b/src/protocol/http/http.c
> index 62f7c3f..71086c3 100644
> --- a/src/protocol/http/http.c
> +++ b/src/protocol/http/http.c
> @@ -74,13 +74,13 @@ struct http_connection_info {
>
> #define LEN_CHUNKED -2 /* == we get data in unknown number of chunks */
> #define LEN_FINISHED 0
> - int length;
> + off_t length;
>
> /* Either bytes coming in this chunk yet or "parser state". */
> #define CHUNK_DATA_END -3
> #define CHUNK_ZERO_SIZE -2
> #define CHUNK_SIZE -1
> - int chunk_remaining;
> + off_t chunk_remaining;
This change should have some consequences for decompress_data calling
also I think. At least we need to check for overflows before trunkating
and assigning the value of this to the len parameter passed to
decompress_data. The code in question:
...
int len;
int zero = (http->chunk_remaining == CHUNK_ZERO_SIZE);
if (zero) http->chunk_remaining = 0;
len = http->chunk_remaining;
/* Maybe everything necessary didn't come yet.. */
int_upper_bound(&len, rb->length);
conn->received += len;
data = decompress_data(conn, rb->data, len, &data_len);
...
I am not sure if this should also be updated to use strtoll:
unsigned char *de;
int n = 0;
if (l != -1) {
errno = 0;
n = strtol(rb->data, (char **) &de, 16);
if (errno || !*de) {
return -1;
}
}
if (l == -1 || de == rb->data) {
return -1;
}
/* Remove everything to the EOLN. */
kill_buffer_data(rb, l);
http->chunk_remaining = n;
... or whether chunk_remaining should even be off_t, but it clearly
results in fewer changes needed to decompress_data.
The problem with potential overflows should also be investigated with
respect the http_connection_info->length member. The only code where
this could be a problem seems to be this. Since the check already
ensures that http->length is smaller than an already known-good value,
it should be OK, I think:
read_normal_http_data(struct connection *conn, struct read_buffer *rb)
{
struct http_connection_info *http = conn->info;
unsigned char *data;
int data_len;
int len = rb->length;
if (http->length >= 0 && http->length < len) {
/* We won't read more than we have to go. */
len = http->length;
}
> int code;
> };
> @@ -1007,7 +1007,7 @@ decompress_data(struct connection *conn, unsigned char
> *data, int len,
> * early otherwise)). */
> enum { NORMAL, FINISHING } state = NORMAL;
> int did_read = 0;
> - int *length_of_block;
> + off_t *length_of_block;
> unsigned char *output = NULL;
>
> length_of_block = (http->length == LEN_CHUNKED ?
> &http->chunk_remaining
> @@ -1761,10 +1761,10 @@ again:
> d = parse_header(conn->cached->head, "Content-Length", NULL);
> if (d) {
> unsigned char *ep;
> - int l;
> + off_t l;
>
> errno = 0;
> - l = strtol(d, (char **) &ep, 10);
> + l = strtoll(d, (char **) &ep, 10);
>
> if (!errno && !*ep && l >= 0) {
> if (!http->close || POST_HTTP_1_0(version))
> --
> 1.5.2.1.851.g432c-dirty
--
Jonas Fonseca
_______________________________________________
elinks-dev mailing list
[email protected]
http://linuxfromscratch.org/mailman/listinfo/elinks-dev