Re: [PATCH v2] receive-pack: not receive pack file with large object

From: Jiang Xin <worldhello.net@gmail.com>
Date: Oct 1, 2021, 02:30 UTC
Message-ID: <CANYiYbHNBcDaoF+QE_+62EXUZD_caaJDFmt7v1_BddQfpdVcvg@mail.gmail.com>
In-Reply-To: <87pmsqtb2p.fsf@evledraar.gmail.com>

On Thu, Sep 30, 2021 at 10:05 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 15 quoted lines

>
>
> On Thu, Sep 30 2021, Han Xin wrote:
>
> > From: Han Xin <hanxin.hx@alibaba-inc.com>
> >
> > In addition to using 'receive.maxInputSize' to limit the overall size
> > of the received packfile, a new config variable
> > 'receive.maxInputObjectSize' is added to limit the push of a single
> > object larger than this threshold.
>
> Maybe an unfair knee-jerk reaction: I think we should really be pushing
> this sort of thing into pre-receive hooks and/or the proc-receive hook,
> i.e. see 15d3af5e22e (receive-pack: add new proc-receive hook,
> 2020-08-27).

Last week, one user complained that he cannot push to his repo in our server, and later Han Xin discovered the user was trying to push a very big blob object over 10GB. For this case, the "pre-receive" hook had no change to execute because "git-receive-pack" died early because of OOM. The function "unpack_non_delta_entry()" in "builtin/unpack-objects.c" will try to allocate memory for the whole 10GB blob but no lucky.

Han Xin is preparing another patch to resolve the OOM issue found in "unpack_non_delta_entry()". But we think it is reasonable to prevent such a big blob in a pack to git-receive-pack, because it will be slower to check objects from pack and loose objects in the quarantine using pre-receive hook.

Show 5 quoted lines

> Anyway, I think there may be dragons here that you haven't
> considered. Is the "size" here the absolute size on disk, or the delta
> size (I'm offhand not familiar enough with unpack-objects.c to
> know). Does this have the same semantics no matter the
> transfer.unpackLimit?

Yes, according to setting of transfer.unpackLimit, may call git-index-pack to save the pack directly, or expand it by calling git-unpack-object. The "size" may be the absolute size on disk, or the delta size. But we know blob over 500MB (default value of core.bigFileThreshold) will not be deltafied, so can we assume this "size" is the absolute size on disk?

-- Jiang Xin