From 835c4d3689972e616bd109cec3dd8cd4aa4d4c0d Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 2 Nov 2015 16:39:58 -0500 Subject: [PATCH 1/2] http.c: use CURLOPT_RANGE for range requests 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. Signed-off-by: David Turner Reviewed-by: Jeff King Signed-off-by: Junio C Hamano --- http.c | 33 ++++++++++++--------------------- http.h | 1 - 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/http.c b/http.c index 7da76edda1..5f368464e8 100644 --- a/http.c +++ b/http.c @@ -30,7 +30,6 @@ static CURL *curl_default; #endif #define PREV_BUF_SIZE 4096 -#define RANGE_HEADER_SIZE 30 char curl_errorstr[CURL_ERROR_SIZE]; @@ -681,6 +680,7 @@ struct active_request_slot *get_active_slot(void) curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0); curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1); + curl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL); #ifdef LIBCURL_CAN_HANDLE_AUTH_ANY curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods); #endif @@ -1173,6 +1173,13 @@ static const char *get_accept_language(void) return cached_accept_language; } +static void http_opt_request_remainder(CURL *curl, off_t pos) +{ + char buf[128]; + xsnprintf(buf, sizeof(buf), "%"PRIuMAX"-", (uintmax_t)pos); + curl_easy_setopt(curl, CURLOPT_RANGE, buf); +} + /* http_request() targets */ #define HTTP_REQUEST_STRBUF 0 #define HTTP_REQUEST_FILE 1 @@ -1201,11 +1208,8 @@ static int http_request(const char *url, long posn = ftell(result); 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_reset(&buf); - } + if (posn > 0) + http_opt_request_remainder(slot->curl, posn); } else curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer); @@ -1515,10 +1519,6 @@ void release_http_pack_request(struct http_pack_request *preq) fclose(preq->packfile); preq->packfile = NULL; } - if (preq->range_header != NULL) { - curl_slist_free_all(preq->range_header); - preq->range_header = NULL; - } preq->slot = NULL; free(preq->url); free(preq); @@ -1582,7 +1582,6 @@ struct http_pack_request *new_http_pack_request( struct packed_git *target, const char *base_url) { long prev_posn = 0; - char range[RANGE_HEADER_SIZE]; struct strbuf buf = STRBUF_INIT; struct http_pack_request *preq; @@ -1620,10 +1619,7 @@ struct http_pack_request *new_http_pack_request( fprintf(stderr, "Resuming fetch of pack %s at byte %ld\n", sha1_to_hex(target->sha1), prev_posn); - xsnprintf(range, sizeof(range), "Range: bytes=%ld-", prev_posn); - preq->range_header = curl_slist_append(NULL, range); - curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, - preq->range_header); + http_opt_request_remainder(preq->slot->curl, prev_posn); } return preq; @@ -1673,8 +1669,6 @@ struct http_object_request *new_http_object_request(const char *base_url, char prev_buf[PREV_BUF_SIZE]; ssize_t prev_read = 0; long prev_posn = 0; - char range[RANGE_HEADER_SIZE]; - struct curl_slist *range_header = NULL; struct http_object_request *freq; freq = xcalloc(1, sizeof(*freq)); @@ -1780,10 +1774,7 @@ struct http_object_request *new_http_object_request(const char *base_url, fprintf(stderr, "Resuming fetch of object %s at byte %ld\n", hex, prev_posn); - xsnprintf(range, sizeof(range), "Range: bytes=%ld-", prev_posn); - range_header = curl_slist_append(range_header, range); - curl_easy_setopt(freq->slot->curl, - CURLOPT_HTTPHEADER, range_header); + http_opt_request_remainder(freq->slot->curl, prev_posn); } return freq; diff --git a/http.h b/http.h index 49afe39279..4f97b60b5c 100644 --- a/http.h +++ b/http.h @@ -190,7 +190,6 @@ struct http_pack_request { struct packed_git **lst; FILE *packfile; char tmpfile[PATH_MAX]; - struct curl_slist *range_header; struct active_request_slot *slot; }; From f8117f550b900b0bc93364ecadbed237b0ffe25e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 2 Nov 2015 17:10:27 -0500 Subject: [PATCH 2/2] http: use off_t to store partial file size When we try to resume transfer of a partially-downloaded object or pack, we fopen() the existing file for append, then use ftell() to get the current position. We use a "long", which can hold only 2GB on a 32-bit system, even though packfiles may be larger than that. Let's switch to using off_t, which should hold any file size our system is capable of storing. We need to use ftello() to get the off_t. This is in POSIX and hopefully available everywhere; if not, we should be able to wrap it by falling back to ftell(), which would presumably return "-1" on such a large file (and we would simply skip resuming in that case). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/http.c b/http.c index 5f368464e8..42f29ce0aa 100644 --- a/http.c +++ b/http.c @@ -1205,7 +1205,7 @@ static int http_request(const char *url, curl_easy_setopt(slot->curl, CURLOPT_FILE, result); if (target == HTTP_REQUEST_FILE) { - long posn = ftell(result); + off_t posn = ftello(result); curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); if (posn > 0) @@ -1581,7 +1581,7 @@ int finish_http_pack_request(struct http_pack_request *preq) struct http_pack_request *new_http_pack_request( struct packed_git *target, const char *base_url) { - long prev_posn = 0; + off_t prev_posn = 0; struct strbuf buf = STRBUF_INIT; struct http_pack_request *preq; @@ -1613,7 +1613,7 @@ struct http_pack_request *new_http_pack_request( * If there is data present from a previous transfer attempt, * resume where it left off */ - prev_posn = ftell(preq->packfile); + prev_posn = ftello(preq->packfile); if (prev_posn>0) { if (http_is_verbose) fprintf(stderr, @@ -1668,7 +1668,7 @@ struct http_object_request *new_http_object_request(const char *base_url, int prevlocal; char prev_buf[PREV_BUF_SIZE]; ssize_t prev_read = 0; - long prev_posn = 0; + off_t prev_posn = 0; struct http_object_request *freq; freq = xcalloc(1, sizeof(*freq));