diff --git a/Documentation/git-http-fetch.adoc b/Documentation/git-http-fetch.adoc index 12036e65e9..45e0d3d07c 100644 --- a/Documentation/git-http-fetch.adoc +++ b/Documentation/git-http-fetch.adoc @@ -48,8 +48,9 @@ commit-id:: line (which is not expected in this case), 'git http-fetch' fetches the packfile directly at the given URL and uses index-pack to generate corresponding .idx and .keep files. - The hash is used to determine the name of the temporary file and is - arbitrary. The output of index-pack is printed to stdout. Requires + The hash is used to determine the name of the temporary file. It need + not be the pack hash, but it must uniquely identify the pack contents + for resumption. The output of index-pack is printed to stdout. Requires one or more --index-pack-arg options. --index-pack-arg=:: diff --git a/http.c b/http.c index 930e0d227f..4965e89248 100644 --- a/http.c +++ b/http.c @@ -2688,10 +2688,13 @@ int finish_http_pack_request(struct http_pack_request *preq) int tmpfile_fd; int ret = 0; + /* Another downloader may unlink the staging path while we index it. */ + tmpfile_fd = xdup(fileno(preq->packfile)); fclose(preq->packfile); preq->packfile = NULL; - - tmpfile_fd = xopen(preq->tmpfile.buf, O_RDONLY); + if (lseek(tmpfile_fd, 0, SEEK_SET) < 0) + die_errno("unable to seek local file %s for pack", + preq->tmpfile.buf); ip.git_cmd = 1; ip.in = tmpfile_fd; @@ -2733,22 +2736,30 @@ struct http_pack_request *new_http_pack_request( struct http_pack_request *new_direct_http_pack_request( const unsigned char *packed_git_hash, char *url) { - off_t prev_posn = 0; + off_t prev_posn; struct http_pack_request *preq; + int fd; CALLOC_ARRAY(preq, 1); strbuf_init(&preq->tmpfile, 0); - preq->url = url; odb_pack_name(the_repository, &preq->tmpfile, packed_git_hash, "pack"); strbuf_addstr(&preq->tmpfile, ".temp"); - preq->packfile = fopen(preq->tmpfile.buf, "a"); - if (!preq->packfile) { - error("Unable to open local file %s for pack", - preq->tmpfile.buf); + fd = open(preq->tmpfile.buf, O_RDWR | O_CREAT, 0666); + if (fd < 0) { + error_errno("unable to open local file %s for pack", + preq->tmpfile.buf); goto abort; } + prev_posn = lseek(fd, 0, SEEK_END); + if (prev_posn < 0) { + error_errno("unable to seek local file %s for pack", + preq->tmpfile.buf); + close(fd); + goto abort; + } + preq->packfile = xfdopen(fd, "w"); preq->slot = get_active_slot(); preq->headers = object_request_headers(); @@ -2757,12 +2768,7 @@ struct http_pack_request *new_direct_http_pack_request( curl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url); curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, preq->headers); - /* - * If there is data present from a previous transfer attempt, - * resume where it left off - */ - prev_posn = ftello(preq->packfile); - if (prev_posn>0) { + if (prev_posn > 0) { if (http_is_verbose) fprintf(stderr, "Resuming fetch of pack %s at byte %"PRIuMAX"\n", diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 621f2fbf73..76cfcb8006 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -312,6 +312,170 @@ test_expect_success 'http-fetch --packfile accepts an already complete partial' git -C packfileclient-complete cat-file -e "$HASH" ' +test_expect_success 'http-fetch --packfile resumes a partial download' ' + git init packfileclient-resume && + p=$(cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git && + ls objects/pack/pack-*.pack) && + tmpfile="packfileclient-resume/.git/objects/pack/pack-$ARBITRARY.pack.temp" && + test_copy_bytes 64 <"$HTTPD_DOCUMENT_ROOT_PATH/repo_pack.git/$p" >"$tmpfile" && + GIT_TRACE_CURL="$TRASH_DIRECTORY/resume.trace" \ + git -C packfileclient-resume http-fetch --packfile="$ARBITRARY" \ + --index-pack-arg=index-pack --index-pack-arg=--stdin \ + --index-pack-arg=--keep \ + "$HTTPD_URL/dumb/repo_pack.git/$p" >out && + test_grep "Range: bytes=64-" resume.trace && + test_path_is_missing "$tmpfile" && + git -C packfileclient-resume cat-file -e "$HASH" +' + +test_expect_success PERL,PIPE 'concurrent http-fetch --packfile cannot corrupt an overlapping download' ' + git init packfileclient-overlap && + blob=$(test-tool genrandom pack-overlap 2m | + git -C "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git \ + hash-object -w --stdin) && + packhash=$(printf "%s\n" "$blob" | + git -C "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git \ + pack-objects "$TRASH_DIRECTORY/overlap-pack") && + pack="$TRASH_DIRECTORY/overlap-pack-$packhash.pack" && + tmpfile="packfileclient-overlap/.git/objects/pack/pack-$packhash.pack.temp" && + mkfifo server-ready first-ready && + exec 7<>server-ready && + exec 8<>first-ready && + write_script slow-pack-server "$PERL_PATH" <<-\EOF && + use strict; + use warnings; + use IO::Socket::INET; + + my ($packfile, $server_ready, $first_ready) = @ARGV; + my $completed = 0; + END { + if (!$completed) { + signal_ready($server_ready, "failed"); + signal_ready($first_ready, "failed"); + } + } + + $SIG{ALRM} = sub { die "timed out serving concurrent pack requests\n" }; + alarm 60; + + open(my $in, "<:raw", $packfile) or die "open $packfile: $!"; + my $pack = do { local $/; <$in> }; + close($in) or die "close $packfile: $!"; + my $server = IO::Socket::INET->new(LocalAddr => "127.0.0.1", + LocalPort => 0, Proto => "tcp", Listen => 2, ReuseAddr => 1) + or die "listen: $!"; + + sub signal_ready { + my ($file, $value) = @_; + open(my $out, ">", $file) or die "open $file: $!"; + print $out "$value\n" or die "write $file: $!"; + close($out) or die "close $file: $!"; + } + + sub write_all { + my ($out, $data) = @_; + my $offset = 0; + while ($offset < length($data)) { + my $written = syswrite($out, $data, + length($data) - $offset, $offset); + defined($written) && $written or die "write response: $!"; + $offset += $written; + } + } + + sub start_response { + my $out = $server->accept() or die "accept: $!"; + <$out> or die "read request: $!"; + my $start = 0; + while (<$out>) { + last if /^\r?\n$/; + $start = $1 if /^Range: bytes=(\d+)-/i; + } + $start < length($pack) or die "invalid range $start"; + my $length = length($pack) - $start; + my $middle = int($length / 2); + my $status = $start ? "206 Partial Content" : "200 OK"; + my $headers = "HTTP/1.1 $status\r\n" . + "Content-Length: $length\r\n" . + ($start ? "Content-Range: bytes $start-" . + (length($pack) - 1) . "/" . length($pack) . "\r\n" : "") . + "Connection: close\r\n\r\n"; + write_all($out, $headers); + write_all($out, substr($pack, $start, $middle)); + return ($out, $start + $middle); + } + + signal_ready($server_ready, $server->sockport()); + my ($first, $first_pos) = start_response(); + signal_ready($first_ready, "ready"); + my ($second, $second_pos) = start_response(); + write_all($first, substr($pack, $first_pos)); + write_all($second, substr($pack, $second_pos)); + close($first) or die "close first response: $!"; + close($second) or die "close second response: $!"; + $completed = 1; + alarm 0; + EOF + { + "$TRASH_DIRECTORY/slow-pack-server" "$pack" \ + "$TRASH_DIRECTORY/server-ready" \ + "$TRASH_DIRECTORY/first-ready" >server.log 2>&1 & + server_pid=$! + } && + test_when_finished " + kill $server_pid 2>/dev/null || : + wait $server_pid 2>/dev/null || : + exec 7>&- + exec 8>&- + rm -f server-ready first-ready slow-pack-server + " && + read port <&7 && + url="http://127.0.0.1:$port/pack" && + { + ( + if ! GIT_TRACE_CURL="$TRASH_DIRECTORY/overlap-first.trace" \ + GIT_TRACE_CURL_NO_DATA=1 \ + git -C packfileclient-overlap http-fetch --packfile="$packhash" \ + --index-pack-arg=index-pack \ + --index-pack-arg=--stdin --index-pack-arg=--keep \ + "$url" >first.out + then + echo failed >"$TRASH_DIRECTORY/first-ready" && + exit 1 + fi + ) & + first_pid=$! + } && + test_when_finished " + kill $first_pid 2>/dev/null || : + wait $first_pid 2>/dev/null || : + " && + read ready <&8 && + test "$ready" = ready && + test_path_is_file "$tmpfile" && + { + GIT_TRACE_CURL="$TRASH_DIRECTORY/overlap-second.trace" \ + GIT_TRACE_CURL_NO_DATA=1 \ + git -C packfileclient-overlap http-fetch --packfile="$packhash" \ + --index-pack-arg=index-pack \ + --index-pack-arg=--stdin --index-pack-arg=--keep \ + "$url" >second.out & + second_pid=$! + } && + test_when_finished " + kill $second_pid 2>/dev/null || : + wait $second_pid 2>/dev/null || : + " && + wait "$second_pid" && + wait "$first_pid" && + wait "$server_pid" && + printf "keep\t%s\npack\t%s\n" "$packhash" "$packhash" | sort >expect && + sort first.out second.out >actual && + test_cmp expect actual && + test_path_is_missing "$tmpfile" && + git -C packfileclient-overlap cat-file -e "$blob" +' + test_expect_success 'fetch notices corrupt pack' ' cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git && (cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git &&