Re: [PATCH 2/2] receive-pack: Add receive.denyObjectLimit to refuse push with too many objects

From: Johan Herland <johan@herland.net>
Date: May 14, 2011, 01:43 UTC
Message-ID: <201105140343.48741.johan@herland.net>
In-Reply-To: <7vpqnm7cim.fsf@alter.siamese.dyndns.org>

On Friday 13 May 2011, Junio C Hamano wrote:

Show 9 quoted lines

> Johan Herland <johan@herland.net> writes:
> > The new receive.denyObjectLimit config variable defines an upper limit
> > on the number of objects to accept in a single push. If the number of
> > objects in a push exceeds this limit, the entire push is immediately
> > aborted without storing the pushed objects on the server at all.
> 
> Where does the error message go?  Can clients pushing over various
> transports receive the reason without your server consuming the data from
> them?  Don't you want to "receive-in-core-and-discard" instead?

Yes. Will be fixed in the re-roll.

> For the purpose of "preventing an accidental push", I suspect that people
> would expect you to limit either by number of commits (i.e. depth of
> history) or by the total size of the data being transferred.

Yes, I agree that limiting by #commits, or by pack size would be more intuitive. However, neither of those values are available to me at the point where I have to decide what to do with the pack data (only the pack header is available, and that only contains the object count).

> The name "objectlimit" sounds as if you are doing the latter and we can
> use "200MB" there, but you are only limiting by count, so it is somewhat
> misleading.  We would want to see "count" or "number" somewhere in its
> name.

Agreed. Will be renamed to receive.objectCountLimit in the re-roll.

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net