Re: [PATCH v3 3/3] receive-pack: allow a maximum input size to be specified

[Jeff King]

On Wed, Aug 24, 2016 at 08:41:57PM +0200, Christian Couder wrote:

> +test_pack_input_limit () {
> + case "$1" in
> + index) unpack_limit=1 ;;
> + unpack) unpack_limit=1 ;;
> + esac

Nice, this is pretty self-explanatory.

> + test_expect_success 'prepare destination repository' '
> + rm -fr dest &&
> + git --bare init dest
> + '
> +
> + test_expect_success "set unpacklimit to $unpack_limit" '
> + git --git-dir=dest config receive.unpacklimit "$unpack_limit"
> + '
> +
> + test_expect_success 'setting receive.maxInputSize to 512 rejects push' '
> + git --git-dir=dest config receive.maxInputSize 512 &&
> + test_must_fail git push dest HEAD
> + '
> +
> + test_expect_success 'bumping limit to 4k allows push' '
> + git --git-dir=dest config receive.maxInputSize 4k &&
> + git push dest HEAD
> + '

Makes sense. We couldn't push, and then we could.

> + test_expect_success 'prepare destination repository (again)' '
> + rm -fr dest &&
> + git --bare init dest
> + '
> +
> + test_expect_success 'lifting the limit allows push' '
> + git --git-dir=dest config receive.maxInputSize 0 &&
> + git push dest HEAD
> + '

This is new in this iteration, I think. At first I thought "but every
_other_ test script is implicitly testing that we work without the
limit". But this is showing that setting the limit explicitly to 0 does
work, which is good.

The whole series looks fine to me.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/3] receive-pack: allow a maximum input size to be specified

[Christian Couder]

From: Jeff King 

Receive-pack feeds its input to either index-pack or
unpack-objects, 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.

Cleaning up what has already been written to disk is a
related problem that is not addressed by this patch.

Signed-off-by: Jeff King 
Signed-off-by: Christian Couder 
---
 Documentation/config.txt   |  6 +
 Documentation/git-receive-pack.txt |  3 +++
 builtin/receive-pack.c | 12 +
 t/t5546-receive-limits.sh  | 55 ++
 4 files changed, 76 insertions(+)
 create mode 100755 t/t5546-receive-limits.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..8a115b3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2517,6 +2517,12 @@ receive.unpackLimit::
especially on slow filesystems.  If not set, the value of
`transfer.unpackLimit` is used instead.
 
+receive.maxInputSize::
+   If the size of the incoming pack stream is larger than this
+   limit, then git-receive-pack will error out, instead of
+   accepting the pack file. If not set or set to 0, then the size
+   is unlimited.
+
 receive.denyDeletes::
If set to true, git-receive-pack will deny a ref update that deletes
the ref. Use this to prevent such a ref deletion via a push.
diff --git a/Documentation/git-receive-pack.txt 
b/Documentation/git-receive-pack.txt
index 000ee8d..0ccd5fb 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -33,6 +33,9 @@ post-update hooks found in the Documentation/howto directory.
 option, which tells it if updates to a ref should be denied if they
 are not fast-forwards.
 
+A number of other receive.* config options are available to tweak
+its behavior, see linkgit:git-config[1].
+
 OPTIONS
 ---
 ::
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 011db00..f1ce05c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -46,6 +46,7 @@ static int transfer_unpack_limit = -1;
 static int advertise_atomic_push = 1;
 static int advertise_push_options;
 static int unpack_limit = 100;
+static off_t max_input_size;
 static int report_status;
 static int use_sideband;
 static int use_atomic;
@@ -212,6 +213,11 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (strcmp(var, "receive.maxinputsize") == 0) {
+   max_input_size = git_config_int64(var, value);
+   return 0;
+   }
+
return git_default_config(var, value, cb);
 }
 
@@ -1648,6 +1654,9 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
if (fsck_objects)
argv_array_pushf(, "--strict%s",
fsck_msg_types.buf);
+   if (max_input_size)
+   argv_array_pushf(, 
"--max-input-size=%"PRIuMAX,
+   (uintmax_t)max_input_size);
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
@@ -1676,6 +1685,9 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
fsck_msg_types.buf);
if (!reject_thin)
argv_array_push(, "--fix-thin");
+   if (max_input_size)
+   argv_array_pushf(, 
"--max-input-size=%"PRIuMAX,
+   (uintmax_t)max_input_size);
child.out = -1;
child.err = err_fd;
child.git_cmd = 1;
diff --git a/t/t5546-receive-limits.sh b/t/t5546-receive-limits.sh
new file mode 100755
index 000..10cb0be
--- /dev/null
+++ b/t/t5546-receive-limits.sh
@@ -0,0 +1,55 @@
+#!/bin/sh
+
+test_description='check receive input limits'
+. ./test-lib.sh
+
+# Let's run tests with different unpack limits: 1 and 1
+# When the limit is 1, `git receive-pack` will call `git index-pack`.
+# When the limit is 1, `git receive-pack` will call `git unpack-objects`.
+
+test_pack_input_limit () {
+   case "$1" in
+   index) unpack_limit=1 ;;
+   unpack) unpack_limit=1 ;;
+   esac
+
+   test_expect_success 'prepare destination repository' '
+   rm -fr dest &&
+   git --bare init dest
+   '
+
+   test_expect_success "set unpacklimit to $unpack_limit" '
+   git --git-dir=dest config receive.unpacklimit "$unpack_limit"
+   '
+
+   test_expect_success 'setting receive.maxInputSize to 512 rejects push' '
+   git --git-dir=dest config receive.maxInputSize 512 &&
+   test_must_fail git push dest HEAD
+   '
+
+   test_expect_success 'bumping limit to 4k allows push' '
+   git