If one tries to fetch packs with an incorrectly formatted parameter while using the smart HTTP protocol v2, the client will block forever. This is seen with the following command which does not terminate: $ git -c protocol.version=2 clone https://github.com/git/git.git --shallow-since=20151012 Cloning into 'git'... Meanwhile, if one uses v1, the command will terminate as expected: $ git -c protocol.version=1 clone https://github.com/git/git.git --shallow-since=20151012 Cloning into 'git'... fatal: the remote end hung up unexpectedly This happens because when upload-pack detects invalid parameters, it will die(). When http-backend calls finish_command() and detects a non-zero exit code, it will simply call exit(1). Since there is no way in a CGI script to force a connection to terminate, the client keeps blocking on the read(), expecting more output. Write an error packet if the backend command fails to start or finishes with an error so that the client can terminate the connection. Note that if the included test case is run without the remainder of the patch, it will run forever and fail to terminate. Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx> --- Notes: So this is the first time I've touched the networking code of Git. There are a few problems with this patch that I'd appreciate some help addressing. First of all, is this even the right approach? I tried to find some other way to force a CGI script to terminate a connection but I don't think that it's possible so I had to get the client to do it instead. Next, with this patch applied it seems like t5539-fetch-http-shallow fails on 'no shallow lines after receiving ACK ready'. I've been trying to dig into why this happens but I can't quite figure it out. Finally, I'd like the test case I added in t5702 to be guarded by some timeout in case we regress instead of blocking the test suite forever but I'm not sure if that's even available functionality. It seems like we don't use the `timeout` program yet so I'm not sure if it's a good idea to introduce it here. Thanks, Denton http-backend.c | 10 +++++++--- t/t5702-protocol-v2.sh | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/http-backend.c b/http-backend.c index ec3144b444..7ea3a688fd 100644 --- a/http-backend.c +++ b/http-backend.c @@ -488,10 +488,11 @@ static void run_service(const char **argv, int buffer_input) cld.git_cmd = 1; cld.clean_on_exit = 1; cld.wait_after_clean = 1; - if (start_command(&cld)) + if (start_command(&cld)) { + packet_write_fmt(1, "ERR %s failed to start\n", argv[0]); exit(1); + } - close(1); if (gzipped_request) inflate_request(argv[0], cld.in, buffer_input, req_len); else if (buffer_input) @@ -501,8 +502,11 @@ static void run_service(const char **argv, int buffer_input) else close(0); - if (finish_command(&cld)) + if (finish_command(&cld)) { + packet_write_fmt(1, "ERR %s failed\n", argv[0]); exit(1); + } + close(1); } static int show_text_ref(const char *name, const struct object_id *oid, diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 5039e66dc4..69a920a34b 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@ -586,6 +586,23 @@ test_expect_success 'clone with http:// using protocol v2' ' ! grep "Send header: Transfer-Encoding: chunked" log ' +test_expect_success 'clone with http:// using protocol v2 and invalid parameters' ' + test_when_finished "rm -f log" && + + test_must_fail env GIT_TRACE_PACKET="$(pwd)/log" GIT_TRACE_CURL="$(pwd)/log" \ + git -c protocol.version=2 \ + clone --shallow-since=20151012 "$HTTPD_URL/smart/http_parent" http_child_invalid && + + # Client requested to use protocol v2 + grep "Git-Protocol: version=2" log && + # Server responded using protocol v2 + grep "git< version 2" log && + # Verify that we sucessfully errored out + grep -F "ERR upload-pack failed" log && + # Verify that the chunked encoding sending codepath is NOT exercised + ! grep "Send header: Transfer-Encoding: chunked" log +' + test_expect_success 'clone big repository with http:// using protocol v2' ' test_when_finished "rm -f log" && -- 2.26.0.159.g23e2136ad0
