Re: [PATCH v3] receive-pack: optionally deny case clone refs

[David Turner <dturner@twopensource.com>]

On Fri, 2014-06-06 at 08:37 +0700, Duy Nguyen wrote:
> On Fri, Jun 6, 2014 at 7:52 AM, David Turner <dturner@twopensource.com> wrote:
> > Create the option receive.denycaseclonerefs, which checks pushed
> > refs to ensure that they are not case clones of an existing
> > ref.  This setting is turned on by default if core.ignorecase is
> > set, but not otherwise.
> 
> Just thinking out loud as I haven't had time to read this patch
> carefully, but I wonder if there's a potential performance problem
> with ref_is_denied_case_clone(). What if the receiver repo has 60k
> refs? Should we use name-hash (or a variant of it)? What if the sender
> pushes 60k refs in one go, will ref_is_denied_case_clone check against
> existing refs only, or it will cover more and more refs from the 60k
> input?

Good news!  The sender can't push 60k refs in one go because that would
require a command-line bigger than ARG_MAX.  (Well, OK, this is not
really good news, since of course someone could hack around this, and
since this is minor bug in git that we should probably fix).

I guess this does present a bit of a DOS attack.  When I discussed the
previous version of this patch with Junio, I did not think it was a huge
problem, since the the ordinary number of refs pushed is small.  But
when I think of it as a DOS, it sounds much worse.

I don't like the idea of creating a hashmap just for this, because
really we need to be storing packed refs in an better on-disk format --
one that allows O(log n) or better access (as has been discussed).  

I'll give this some more thought and see if I can come up with a better
solution.