Goal
~~~~

In https://public-inbox.org/git/20150612182045.GA23698%40peff.net/,
Peff sent a patch that is used by GitHub to abort `git receive-pack`
when the size of the pack we receive is bigger than a configured
limit.

GitLab is interested in using the same approach and in standardizing
the error messages the user could get back.

Comments
~~~~~~~~

I kept Peff as the author of the patches that are made mostly from his
patch, but I added my Signed-off-by to them.

Changes from previous RFC version
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  - added documentation to all the 3 patches,

  - changed strtoul() to strtoumax() in the first 2 patches, as
    suggested by Peff,

  - changed git_config_ulong() to git_config_int64() and used PRIuMAX
    and uintmax_t in the last patch, as suggested by Peff,

  - reorganized the tests in the last patch, as suggested by Peff

  - improved commit message of the last patch with information from
    Peff

Links
~~~~~

This patch series is available here:

https://github.com/chriscool/git/commits/max-receive

The previous RFC version is here on GitHub:

https://github.com/chriscool/git/commits/max-receive2

and here on the list:

https://public-inbox.org/git/20160815195729.16826-1-chrisc...@tuxfamily.org/

Peff's initial patch is:

https://public-inbox.org/git/20150612182045.GA23698%40peff.net/

Diff with previous RFC version
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..f5b6061 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2517,6 +2517,11 @@ receive.unpackLimit::
        especially on slow filesystems.  If not set, the value of
        `transfer.unpackLimit` is used instead.
 
+receive.maxsize::
+       If the size of a pack file 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-index-pack.txt b/Documentation/git-index-pack.txt
index 7a4e055..1b4b65d 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -87,6 +87,8 @@ OPTIONS
        Specifying 0 will cause Git to auto-detect the number of CPU's
        and use maximum 3 threads.
 
+--max-input-size=<size>::
+       Die, if the pack is larger than <size>.
 
 Note
 ----
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
 -------
 <directory>::
diff --git a/Documentation/git-unpack-objects.txt 
b/Documentation/git-unpack-objects.txt
index 3e887d1..b3de50d 100644
--- a/Documentation/git-unpack-objects.txt
+++ b/Documentation/git-unpack-objects.txt
@@ -44,6 +44,9 @@ OPTIONS
 --strict::
        Don't write objects with broken content or links.
 
+--max-input-size=<size>::
+       Die, if the pack is larger than <size>.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 1fd60bd..4a8b4ae 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1718,7 +1718,7 @@ int cmd_index_pack(int argc, const char **argv, const 
char *prefix)
                                if (*c || opts.off32_limit & 0x80000000)
                                        die(_("bad %s"), arg);
                        } else if (skip_prefix(arg, "--max-input-size=", &arg)) 
{
-                               max_input_size = strtoul(arg, NULL, 10);
+                               max_input_size = strtoumax(arg, NULL, 10);
                        } else
                                usage(index_pack_usage);
                        continue;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 7627f7f..8c2943d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -214,7 +214,7 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
        }
 
        if (strcmp(var, "receive.maxsize") == 0) {
-               max_input_size = git_config_ulong(var, value);
+               max_input_size = git_config_int64(var, value);
                return 0;
        }
 
@@ -1657,8 +1657,8 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
                        argv_array_pushf(&child.args, "--strict%s",
                                fsck_msg_types.buf);
                if (max_input_size)
-                       argv_array_pushf(&child.args, "--max-input-size=%lu",
-                               max_input_size);
+                       argv_array_pushf(&child.args, 
"--max-input-size=%"PRIuMAX,
+                               (uintmax_t)max_input_size);
                child.no_stdout = 1;
                child.err = err_fd;
                child.git_cmd = 1;
@@ -1688,8 +1688,8 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
                if (!reject_thin)
                        argv_array_push(&child.args, "--fix-thin");
                if (max_input_size)
-                       argv_array_pushf(&child.args, "--max-input-size=%lu",
-                               max_input_size);
+                       argv_array_pushf(&child.args, 
"--max-input-size=%"PRIuMAX,
+                               (uintmax_t)max_input_size);
                child.out = -1;
                child.err = err_fd;
                child.git_cmd = 1;
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 59b1f39..4532aa0 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -554,7 +554,7 @@ int cmd_unpack_objects(int argc, const char **argv, const 
char *prefix)
                                continue;
                        }
                        if (skip_prefix(arg, "--max-input-size=", &arg)) {
-                               max_input_size = strtoul(arg, NULL, 10);
+                               max_input_size = strtoumax(arg, NULL, 10);
                                continue;
                        }
                        usage(unpack_usage);
diff --git a/t/t5546-push-limits.sh b/t/t5546-push-limits.sh
index d3a4d1a..b38d508 100755
--- a/t/t5546-push-limits.sh
+++ b/t/t5546-push-limits.sh
@@ -3,45 +3,40 @@
 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
+       git init --bare dest
 '
 
-test_expect_success 'receive.maxsize rejects push' '
-       git --git-dir=dest config receive.maxsize 512 &&
-       test_must_fail git push dest HEAD
-'
+# Let's run tests with different unpack limits: 1 and 10
+# When the limit is 1, `git receive-pack` will call `git index-pack`.
+# When the limit is 10, `git receive-pack` will call `git unpack-objects`.
 
-test_expect_success 'bumping limit allows push' '
-       git --git-dir=dest config receive.maxsize 4k &&
-       git push dest HEAD
-'
+while read unpacklimit filesize filename
+do
 
-test_expect_success 'create another known-size commit' '
-       test-genrandom bar 2048 >file2 &&
-       git add file2 &&
-       test_commit two-k
-'
+       test_expect_success "create known-size ($filesize bytes) commit 
'$filename'" '
+               test-genrandom foo "$filesize" >"$filename" &&
+               git add "$filename" &&
+               test_commit "$filename"
+       '
 
-test_expect_success 'change unpacklimit' '
-       git --git-dir=dest config receive.unpacklimit 10
-'
+       test_expect_success "set unpacklimit to $unpacklimit" '
+               git --git-dir=dest config receive.unpacklimit "$unpacklimit"
+       '
 
-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 'setting receive.maxsize to 512 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_expect_success 'bumping limit to 4k allows push' '
+               git --git-dir=dest config receive.maxsize 4k &&
+               git push dest HEAD
+       '
+
+done <<\EOF
+1 1024 one-k-file
+10 2048 two-k-file
+EOF
 
 test_done


Christian Couder (1):
  unpack-objects: add --max-input-size=<size> option

Jeff King (2):
  index-pack: add --max-input-size=<size> option
  receive-pack: allow a maximum input size to be specified

 Documentation/config.txt             |  5 +++++
 Documentation/git-index-pack.txt     |  2 ++
 Documentation/git-receive-pack.txt   |  3 +++
 Documentation/git-unpack-objects.txt |  3 +++
 builtin/index-pack.c                 |  5 +++++
 builtin/receive-pack.c               | 12 +++++++++++
 builtin/unpack-objects.c             |  7 ++++++
 t/t5546-push-limits.sh               | 42 ++++++++++++++++++++++++++++++++++++
 8 files changed, 79 insertions(+)
 create mode 100755 t/t5546-push-limits.sh

-- 
2.10.0.rc0.3.g8535b4c

--
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

Reply via email to