diff options
| -rwxr-xr-x | t/t5530-upload-pack-error.sh | 75 | ||||
| -rw-r--r-- | upload-pack.c | 192 |
2 files changed, 152 insertions, 115 deletions
diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh new file mode 100755 index 0000000000..cc8949e3ef --- /dev/null +++ b/t/t5530-upload-pack-error.sh @@ -0,0 +1,75 @@ +#!/bin/sh + +test_description='errors in upload-pack' + +. ./test-lib.sh + +D=`pwd` + +corrupt_repo () { + object_sha1=$(git rev-parse "$1") && + ob=$(expr "$object_sha1" : "\(..\)") && + ject=$(expr "$object_sha1" : "..\(..*\)") && + rm -f ".git/objects/$ob/$ject" +} + +test_expect_success 'setup and corrupt repository' ' + + echo file >file && + git add file && + git rev-parse :file && + git commit -a -m original && + test_tick && + echo changed >file && + git commit -a -m changed && + corrupt_repo HEAD:file + +' + +test_expect_failure 'fsck fails' ' + + git fsck +' + +test_expect_success 'upload-pack fails due to error in pack-objects' ' + + ! echo "0032want $(git rev-parse HEAD) +00000009done +0000" | git-upload-pack . > /dev/null 2> output.err && + grep "pack-objects died" output.err +' + +test_expect_success 'corrupt repo differently' ' + + git hash-object -w file && + corrupt_repo HEAD^^{tree} + +' + +test_expect_failure 'fsck fails' ' + + git fsck +' +test_expect_success 'upload-pack fails due to error in rev-list' ' + + ! echo "0032want $(git rev-parse HEAD) +00000009done +0000" | git-upload-pack . > /dev/null 2> output.err && + grep "waitpid (async) failed" output.err +' + +test_expect_success 'create empty repository' ' + + mkdir foo && + cd foo && + git init + +' + +test_expect_failure 'fetch fails' ' + + git fetch .. master + +' + +test_done diff --git a/upload-pack.c b/upload-pack.c index 67994680f2..7e04311027 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -144,6 +144,7 @@ static void create_pack_file(void) char abort_msg[] = "aborting due to possible repository " "corruption on the remote side."; int buffered = -1; + ssize_t sz; const char *argv[10]; int arg = 0; @@ -168,22 +169,15 @@ static void create_pack_file(void) pack_objects.git_cmd = 1; pack_objects.argv = argv; - if (start_command(&pack_objects)) { - /* daemon sets things up to ignore TERM */ - kill(rev_list.pid, SIGKILL); + if (start_command(&pack_objects)) die("git-upload-pack: unable to fork git-pack-objects"); - } /* We read from pack_objects.err to capture stderr output for * progress bar, and pack_objects.out to capture the pack data. */ while (1) { - const char *who; struct pollfd pfd[2]; - pid_t pid; - int status; - ssize_t sz; int pe, pu, pollsize; reset_timeout(); @@ -204,123 +198,91 @@ static void create_pack_file(void) 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 = xread(pack_objects.out, cp, - sizeof(data) - outsz); - if (0 < sz) - ; - else if (sz == 0) { - close(pack_objects.out); - pack_objects.out = -1; - } - else - goto fail; - sz += outsz; - if (1 < sz) { - buffered = data[sz-1] & 0xFF; - sz--; - } - else - buffered = -1; - sz = send_client_data(1, data, sz); - if (sz < 0) - goto fail; - } - if (0 <= pe && (pfd[pe].revents & (POLLIN|POLLHUP))) { - /* Status ready; we ship that in the side-band - * or dump to the standard error. - */ - sz = xread(pack_objects.err, progress, - sizeof(progress)); - if (0 < sz) - send_client_data(2, progress, sz); - else if (sz == 0) { - close(pack_objects.err); - pack_objects.err = -1; - } - else - goto fail; + if (!pollsize) + break; + + if (poll(pfd, pollsize, -1) < 0) { + if (errno != EINTR) { + error("poll failed, resuming: %s", + strerror(errno)); + sleep(1); } + continue; } - - /* See if the children are still there */ - if (rev_list.pid || pack_objects.pid) { - pid = waitpid(-1, &status, WNOHANG); - if (!pid) - continue; - who = ((pid == rev_list.pid) ? "git-rev-list" : - (pid == pack_objects.pid) ? "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 (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 packdata 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 = xread(pack_objects.out, cp, + sizeof(data) - outsz); + if (0 < sz) + ; + else if (sz == 0) { + close(pack_objects.out); + pack_objects.out = -1; } - if (!WIFEXITED(status) || WEXITSTATUS(status) > 0) { - error("git-upload-pack: %s died with error.", - who); + else goto fail; + sz += outsz; + if (1 < sz) { + buffered = data[sz-1] & 0xFF; + sz--; } - if (pid == rev_list.pid) - rev_list.pid = 0; - if (pid == pack_objects.pid) - pack_objects.pid = 0; - if (rev_list.pid || pack_objects.pid) - continue; - } - - /* both died happily */ - if (pollsize) - continue; - - /* flush the data */ - if (0 <= buffered) { - data[0] = buffered; - sz = send_client_data(1, data, 1); + else + buffered = -1; + sz = send_client_data(1, data, sz); if (sz < 0) goto fail; - fprintf(stderr, "flushed.\n"); } - if (use_sideband) - packet_flush(1); - return; + if (0 <= pe && (pfd[pe].revents & (POLLIN|POLLHUP))) { + /* Status ready; we ship that in the side-band + * or dump to the standard error. + */ + sz = xread(pack_objects.err, progress, + sizeof(progress)); + if (0 < sz) + send_client_data(2, progress, sz); + else if (sz == 0) { + close(pack_objects.err); + pack_objects.err = -1; + } + else + goto fail; + } + } + + if (finish_command(&pack_objects)) { + error("git-upload-pack: git-pack-objects died with error."); + goto fail; + } + if (finish_async(&rev_list)) + goto fail; /* error was already reported */ + + /* flush the data */ + if (0 <= buffered) { + data[0] = buffered; + sz = send_client_data(1, data, 1); + if (sz < 0) + goto fail; + fprintf(stderr, "flushed.\n"); } + if (use_sideband) + packet_flush(1); + return; + fail: - if (pack_objects.pid) - kill(pack_objects.pid, SIGKILL); - if (rev_list.pid) - kill(rev_list.pid, SIGKILL); send_client_data(3, abort_msg, sizeof(abort_msg)); die("git-upload-pack: %s", abort_msg); } |