Re: [PATCH] symbolic-ref: check format of given reference

From: Jeff King <peff@peff.net>
Date: Jun 19, 2012, 14:52 UTC
Message-ID: <20120619145238.GC12085@sigill.intra.peff.net>
In-Reply-To: <20120619144712.GB12085@sigill.intra.peff.net>

On Tue, Jun 19, 2012 at 10:47:12AM -0400, Jeff King wrote:

Show 25 quoted lines

> On Mon, Jun 18, 2012 at 10:10:14AM -0700, Junio C Hamano wrote:
> 
> > For that matter, shouldn't symbolic-ref be forbidden to point
> > outside refs/heads/, not just restricted in refs/ like the current
> > code does?
> 
> We tried that already but reverted it due to topgit. See:
> 
>     commit e9cc02f0e41fd5d2f51e3c3f2b4f8cfa9e434432
>     Author: Jeff King <peff@peff.net>
>     Date:   Fri Feb 13 13:26:09 2009 -0500
> 
>         symbolic-ref: allow refs/<whatever> in HEAD
> 
>         Commit afe5d3d5 introduced a safety valve to symbolic-ref to
>         disallow installing an invalid HEAD. It was accompanied by
>         b229d18a, which changed validate_headref to require that
>         HEAD contain a pointer to refs/heads/ instead of just refs/.
>         Therefore, the safety valve also checked for refs/heads/.
> 
>         As it turns out, topgit is using refs/top-bases/ in HEAD,
>         leading us to re-loosen (at least temporarily) the
>         validate_headref check made in b229d18a. This patch does the
>         corresponding loosening for the symbolic-ref safety valve,
>         so that the two are in agreement once more.

The "at least temporarily" in that commit message merited a little investigation. There was some discussion of changing topgit to record its information using a different scheme, but there was no clear outcome:

  http://thread.gmane.org/gmane.comp.version-control.git/109581

So if somebody wanted to re-tighten this check, they would want to at least check topgit's current behavior, and see which versions of it we would be breaking. I tend to think it is not worth the effort.

-Peff