Re: git push keeps writing after server failure
- From
Jeff King <peff@peff.net>
- Date
- Jun 12, 2015, 18:20 UTC
- Message-ID
- <20150612182045.GA23698@peff.net>
- In-Reply-To
- <20150612181256.GB9242@peff.net>
On Fri, Jun 12, 2015 at 02:12:56PM -0400, Jeff King wrote:
Show 18 quoted lines
> > $ git push --all git@github.com:spearce/wk.git > > Counting objects: 2752427, done. > > Delta compression using up to 12 threads. > > Compressing objects: 100% (442684/442684), done. > > remote: fatal: pack exceeds maximum allowed size > > Writing objects: 100% (2752427/2752427), 5.28 GiB | 8.86 MiB/s, done. > > Total 2752427 (delta 2225007), reused 2752427 (delta 2225007) > > fatal: The remote end hung up unexpectedly > > fatal: The remote end hung up unexpectedly > > > > Notice GitHub prints "remote: fatal: pack exceeds maximum allowed > > size". That interrupted my "Writing objects" progress meter, and then > > git push just kept going and wrote really really fast (170 MiB/s!) > > until the entire pack was sent. > > Sounds like it's writing to a closed fd, then. Which makes sense; I > think we should hang up the socket after writing the "fatal" message > above.
For reference, here's the patch implementing the max-size check on the server. It's on my long list of patches to clean up and send to the list; I never did this one because of the unpack-objects caveat mentioned below.
The death happens in index-pack. I think receive-pack should notice that and bail (closing the socket), but it's possible that it doesn't.
There's also a git-aware proxying layer at GitHub between clients and git-receive-pack these days. It's possible that it is not properly hanging up until it sees EOF from the client.
If any of those things are true, then it is a GitHub-specific problem (which I still want to fix, but it is not something git upstream needs to care about).
-- >8 -- Date: Tue, 4 Sep 2012 12:01:06 -0400 Subject: [PATCH] receive-pack: allow a maximum input size to be specified
Receive-pack feeds its input to index-pack, which will happily accept as many bytes as a sender is willing to provide. Let's allow an arbitrary cutoff point where we will stop writing bytes to disk (they're still left in the tmp_pack, but cleaning that can easily be done outside of receive-pack).
Note that this doesn't handle the git-unpack-objects code path at all (because GitHub sets transfer.unpacklimit to 1, so we never unpack pushed objects anyway).
Signed-off-by: Jeff King <peff@peff.net> --- builtin/index-pack.c | 5 +++++ builtin/receive-pack.c | 14 +++++++++++++- t/t5528-push-limits.sh | 27 +++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100755 t/t5528-push-limits.sh
diff --git a/builtin/index-pack.c b/builtin/index-pack.c index dd1c5c9..054a144 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -70,6 +70,7 @@ static struct progress *progress; static unsigned char input_buffer[4096]; static unsigned int input_offset, input_len; static off_t consumed_bytes; +static off_t max_input_size; static unsigned deepest_delta; static git_SHA_CTX input_ctx; static uint32_t input_crc32; @@ -167,6 +168,8 @@ static void use(int bytes) if (signed_add_overflows(consumed_bytes, bytes)) die("pack too large for current definition of off_t"); consumed_bytes += bytes; + if (max_input_size && consumed_bytes > max_input_size) + die("pack exceeds maximum allowed size"); } static const char *open_pack_file(const char *pack_name) @@ -1148,6 +1151,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) opts.off32_limit = strtoul(c+1, &c, 0); if (*c || opts.off32_limit & 0x80000000) die("bad %s", arg); + } else if (!prefixcmp(arg, "--max-input-size=")) { + max_input_size = strtoul(arg + 17, NULL, 10); } else usage(index_pack_usage); continue; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 0afb8b2..e64b8d2 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -31,6 +31,7 @@ static int transfer_fsck_objects = -1; static int receive_unpack_limit = -1; static int transfer_unpack_limit = -1; static int unpack_limit = 100; +static off_t max_input_size; static int report_status; static int use_sideband; static int quiet; @@ -113,6 +114,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb) return 0; } + if (strcmp(var, "receive.maxsize") == 0) { + max_input_size = git_config_ulong(var, value); + return 0; + } + return git_default_config(var, value, cb); } @@ -832,9 +838,10 @@ static const char *unpack(void) return NULL; return "unpack-objects abnormal exit"; } else { - const char *keeper[7]; + const char *keeper[8]; int s, status, i = 0; char keep_arg[256]; + char max_input_arg[256]; struct child_process ip; s = sprintf(keep_arg, "--keep=receive-pack %"PRIuMAX" on ", (uintmax_t) getpid()); @@ -848,6 +855,11 @@ static const char *unpack(void) keeper[i++] = "--fix-thin"; keeper[i++] = hdr_arg; keeper[i++] = keep_arg; + if (max_input_size) { + snprintf(max_input_arg, sizeof(max_input_arg), + "--max-input-size=%lu", max_input_size); + keeper[i++] = max_input_arg; + } keeper[i++] = NULL; memset(&ip, 0, sizeof(ip)); ip.argv = keeper; diff --git a/t/t5528-push-limits.sh b/t/t5528-push-limits.sh new file mode 100755 index 0000000..c2f6d77 --- /dev/null +++ b/t/t5528-push-limits.sh @@ -0,0 +1,27 @@ +#!/bin/sh + +test_description='check input limits for pushing' +. ./test-lib.sh + +test_expect_success 'create known-size commit' ' + test-genrandom foo 1024 >file && + git add file && + test_commit one-k +' + +test_expect_success 'create remote repository' ' + git init --bare dest && + git --git-dir=dest config receive.unpacklimit 1 +' + +test_expect_success 'receive.maxsize rejects push' ' + git --git-dir=dest config receive.maxsize 512 && + test_must_fail git push dest HEAD +' + +test_expect_success 'bumping limit allows push' ' + git --git-dir=dest config receive.maxsize 4k && + git push dest HEAD +' + +test_done
-- 2.4.2.752.geeb594a