From b1c71b72815cb82a8bad14020a047320b88a04eb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 20 Jun 2006 18:26:34 -0700 Subject: [PATCH] upload-pack: avoid sending an incomplete pack upon failure When the repository on the remote side is corrupted, rev-list spawned from upload-pack would die with error, but pack-objects that reads from the rev-list happily created a packfile that can be unpacked by the downloader. When this happens, the resulting packfile is not corrupted and unpacks cleanly, but the list of the objects contained in it is not what the protocol exchange computed. This update makes upload-pack to monitor its subprocesses, and when either of them dies with error, sends an incomplete pack data to the downloader to cause it to fail. Signed-off-by: Junio C Hamano --- upload-pack.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 163 insertions(+), 14 deletions(-) diff --git a/upload-pack.c b/upload-pack.c index 979e58306..a9a8f2ed1 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -5,6 +5,9 @@ #include "object.h" #include "commit.h" #include "exec_cmd.h" +#include +#include +#include static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] "; @@ -33,17 +36,20 @@ static int strip(char *line, int len) static void create_pack_file(void) { - int fd[2]; - pid_t pid; + /* Pipes between rev-list to pack-objects and pack-objects to us. */ + int lp_pipe[2], pu_pipe[2]; + pid_t pid_rev_list, pid_pack_objects; int create_full_pack = (nr_our_refs == nr_needs && !nr_has); + char data[8193]; + int buffered = -1; - if (pipe(fd) < 0) + if (pipe(lp_pipe) < 0) die("git-upload-pack: unable to create pipe"); - pid = fork(); - if (pid < 0) + pid_rev_list = fork(); + if (pid_rev_list < 0) die("git-upload-pack: unable to fork git-rev-list"); - if (!pid) { + if (!pid_rev_list) { int i; int args; const char **argv; @@ -60,10 +66,10 @@ static void create_pack_file(void) argv = (const char **) p; buf = xmalloc(args * 45); - dup2(fd[1], 1); + dup2(lp_pipe[1], 1); close(0); - close(fd[0]); - close(fd[1]); + close(lp_pipe[0]); + close(lp_pipe[1]); *p++ = "rev-list"; *p++ = use_thin_pack ? "--objects-edge" : "--objects"; if (create_full_pack || MAX_NEEDS <= nr_needs) @@ -86,11 +92,154 @@ static void create_pack_file(void) execv_git_cmd(argv); die("git-upload-pack: unable to exec git-rev-list"); } - dup2(fd[0], 0); - close(fd[0]); - close(fd[1]); - execl_git_cmd("pack-objects", "--stdout", NULL); - die("git-upload-pack: unable to exec git-pack-objects"); + + if (pipe(pu_pipe) < 0) + die("git-upload-pack: unable to create pipe"); + pid_pack_objects = fork(); + if (pid_pack_objects < 0) { + /* daemon sets things up to ignore TERM */ + kill(pid_rev_list, SIGKILL); + die("git-upload-pack: unable to fork git-pack-objects"); + } + if (!pid_pack_objects) { + dup2(lp_pipe[0], 0); + dup2(pu_pipe[1], 1); + + close(lp_pipe[0]); + close(lp_pipe[1]); + close(pu_pipe[0]); + close(pu_pipe[1]); + execl_git_cmd("pack-objects", "--stdout", NULL); + kill(pid_rev_list, SIGKILL); + die("git-upload-pack: unable to exec git-pack-objects"); + } + + close(lp_pipe[0]); + close(lp_pipe[1]); + + /* We read from pu_pipe[0] to capture the pack data. + */ + close(pu_pipe[1]); + + while (1) { + const char *who; + struct pollfd pfd[2]; + pid_t pid; + int status; + ssize_t sz; + int pu, pollsize; + + pollsize = 0; + pu = -1; + + if (0 <= pu_pipe[0]) { + pfd[pollsize].fd = pu_pipe[0]; + pfd[pollsize].events = POLLIN; + pu = pollsize; + pollsize++; + } + + if (pollsize) { + if (poll(pfd, pollsize, -1) < 0) { + if (errno != EINTR) { + error("poll failed, resuming: %s", + strerror(errno)); + sleep(1); + } + continue; + } + if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) { + /* Data ready; we keep the last byte + * to ourselves in case we detect + * broken rev-list, so that we can + * leave the stream corrupted. This + * is unfortunate -- unpack-objects + * would happily accept a valid pack + * data with trailing garbage, so + * appending garbage after we pass all + * the pack data is not good enough to + * signal breakage to downstream. + */ + char *cp = data; + ssize_t outsz = 0; + if (0 <= buffered) { + *cp++ = buffered; + outsz++; + } + sz = read(pu_pipe[0], cp, + sizeof(data) - outsz); + if (0 < sz) + ; + else if (sz == 0) { + close(pu_pipe[0]); + pu_pipe[0] = -1; + } + else + goto fail; + sz += outsz; + if (1 < sz) { + buffered = data[sz-1] & 0xFF; + sz--; + } + else + buffered = -1; + sz = xwrite(1, data, sz); + if (sz < 0) + goto fail; + } + } + + /* See if the children are still there */ + if (pid_rev_list || pid_pack_objects) { + pid = waitpid(-1, &status, WNOHANG); + if (!pid) + continue; + who = ((pid == pid_rev_list) ? "git-rev-list" : + (pid == pid_pack_objects) ? "git-pack-objects" : + NULL); + if (!who) { + if (pid < 0) { + error("git-upload-pack: %s", + strerror(errno)); + goto fail; + } + error("git-upload-pack: we weren't " + "waiting for %d", pid); + continue; + } + if (!WIFEXITED(status) || WEXITSTATUS(status) > 0) { + error("git-upload-pack: %s died with error.", + who); + goto fail; + } + if (pid == pid_rev_list) + pid_rev_list = 0; + if (pid == pid_pack_objects) + pid_pack_objects = 0; + if (pid_rev_list || pid_pack_objects) + continue; + } + + /* both died happily */ + if (pollsize) + continue; + + /* flush the data */ + if (0 <= buffered) { + data[0] = buffered; + sz = xwrite(1, data, 1); + if (sz < 0) + goto fail; + fprintf(stderr, "flushed.\n"); + } + return; + } + fail: + if (pid_pack_objects) + kill(pid_pack_objects, SIGKILL); + if (pid_rev_list) + kill(pid_rev_list, SIGKILL); + die("git-upload-pack: aborting due to possible repository corruption on the remote side."); } static int got_sha1(char *hex, unsigned char *sha1) -- 2.30.2