Re: [PATCH v2] http.c: use CURLOPT_RANGE for range requests
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:07:07
David Turner [off-list ref] writes:
A HTTP server is permitted to return a non-range response to a HTTP range request (and Apache httpd in fact does this in some cases). While libcurl knows how to correctly handle this (by skipping bytes before and after the requested range), it only turns on this handling if it is aware that a range request is being made. By manually setting the range header instead of using CURLOPT_RANGE, we were hiding the fact that this was a range request from libcurl. This could cause corruption.
Wow, that looks really bad.
quoted hunk
Signed-off-by: David Turner <redacted> --- This version applies on top of pu. It also catches all of the range requests instead of just the one that I happened to notice. --- http.c | 24 ++++++++---------------- http.h | 1 - 2 files changed, 8 insertions(+), 17 deletions(-)diff --git a/http.c b/http.c index 6b89dea..16610b9 100644 --- a/http.c +++ b/http.c@@ -30,7 +30,7 @@ static CURL *curl_default; #endif #define PREV_BUF_SIZE 4096 -#define RANGE_HEADER_SIZE 30 +#define RANGE_HEADER_SIZE 17
Was this change necessary and is 17-byte string sufficient for 64-bit longs?
quoted hunk
@@ -1213,8 +1213,9 @@ static int http_request(const char *url, curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); if (posn > 0) { - strbuf_addf(&buf, "Range: bytes=%ld-", posn); - headers = curl_slist_append(headers, buf.buf); + strbuf_addf(&buf, "%ld-", posn); + curl_easy_setopt(slot->curl, CURLOPT_RANGE, + &buf.buf); strbuf_reset(&buf);
Makes me wonder if all the callers have a CURL* and a long so that a new helper range_opt_ask_remainder(CURL *, long) can make the resulting code even simpler; we only say "we know what comes before this position, give us everything from there" in all callers, right?