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

[Jeff King <peff@peff.net>]

On Tue, Aug 16, 2016 at 10:17:01AM +0200, Christian Couder wrote:

> From: Jeff King <peff@peff.net>
> 
> 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.
> 
> The problem is that git-gc can clean up tmp_pack_* files
> after their grace time expired, but that may not be
> enough if someone tries to "git push" in a loop.
> 
> A simple fix is to call register_tempfile() in
> open_pack_file(), and just have index-pack clean up the
> file on its way out.
> 
> But there are harder cases. For instance, if somebody
> pushes a 500MB file, and there is a pre-receive hook that
> says "too big; I won't accept this". And then they push in
> a loop, the incoming pack has already been accepted into
> the repository by the time the pre-receive hook runs.
> It's not possible to just delete it, because we don't know
> if other simultaneous processes have started to depend on
> the objects.

IMHO, everything after "not addressed by this patch" doesn't really add
anything. This commit has value on its own, and the discussion about the
next steps is already documented on the list (and hopefully will be
fixed with other patches!).

> +test_expect_success 'create remote repository' '
> +	git init --bare dest
> +'
> +
> +# 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`.
> +
> +while read unpacklimit filesize filename
> +do
> [...]
> +done <<\EOF
> +1 1024 one-k-file
> +10 2048 two-k-file
> +EOF

Is there any reason to use different filenames and sizes for the two
runs? They should behave identically, so it would make more sense to me
to subject them to identical inputs.

I know you need different files and filenames to continue pushing into
the same "dest", but the problem is easily solved by bumping the "git
init" into the loop.

-Peff