Re: [PATCH 0/9] reducing memory allocations for v2 servers

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 14, 2021, 18:00 UTC
Message-ID: <xmqqy27z11gn.fsf@gitster.g>
In-Reply-To: <YUC/6n1hhUbMJiLw@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> While looking at [1], I noticed that v2 servers will read a few bits of
> client input into strvecs. Even though we expect these to be small-ish,
> there's nothing preventing a client from sending us a bunch of junk and
> wasting memory.

The title of the topic says "reducing", but it smells more to me like this is about "limiting" allocations to protect ourselves. Am I reading the series correctly?

Show 23 quoted lines

> This series changes that, putting a cap on how much data we'll receive.
> The two spots are the "capabilities" list we receive (before we even
> dispatch to a particular command like ls-refs or fetch), and the
> ref-prefix list we receive for ls-refs.
>
> To deal with the capabilities issue, we'll just handle each capability
> line as it comes in, rather than storing a list. This requires a bit of
> cleanup, which is why it takes up the first 6 patches. It also needs a
> few other cleanups from ab/serve-cleanup (and so this series is based on
> that). The dependencies there are both textual (e.g., using designated
> initializers in the capabilities table) and functional (dropping the
> "keys" parameter from v2 command() functions).
>
> Patch 7 fixes the ls-refs issue by degrading when we see too many
> prefixes (which works because the capability is optional in the first
> place).
>
> The final two patches aren't strictly necessary. They're a tightening of
> the protocol against some bogus input that I noticed while doing the
> earlier cleanups. That bogus input isn't really hurting anything, but I
> think it makes sense to reject nonsense from the client rather than
> ignoring it. The obvious risk is that some client for some reason would
> send us nonsense, but I don't think Git ever has.

Your cover letters (as proposed log messages for individual commits) are always to the point and very pleasant to read. Very much appreciated.

Thanks.