Re: [PATCH 4/8] interpret_branch_name: allow callers to restrict expansions

[Jeff King <peff@peff.net>]

On Tue, Feb 28, 2017 at 07:23:38AM -0500, Jeff King wrote:

> > -int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
> > +int interpret_branch_name(const char *name, int namelen, struct strbuf *buf,
> > +			  unsigned allowed)
> >  {
> >  	char *at;
> >  	const char *start;
> > @@ -1254,24 +1275,29 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf)
> >  		if (len == namelen)
> >  			return len; /* consumed all */
> >  		else
> > -			return reinterpret(name, namelen, len, buf);
> > +			return reinterpret(name, namelen, len, buf, allowed);
> >  	}
> 
> It's hard to see from this context, but a careful reader may note that
> we do not check "allowed" at all before calling
> interpret_nth_prior_checkout(). This is looking for branch names via
> HEAD, so I don't think it can ever return anything but a local name.
> 
> Which, hmm. I guess was valid when the flag was "only_branches", but
> would not be valid under INTERPRET_BRANCH_REMOTE. I wonder if
> 
>   git branch -r -D @{-1}
> 
> incorrectly deletes refs/remotes/origin/master if you previously had
> refs/heads/origin/master checked out.

The answer is "yes", it's broken. So interpret_branch_name() should do
an "allowed" check before expanding the nth-prior. The fix should be
easy, especially on top of the earlier 426f76595 (which, incidentally, I
already based this series on).

I'll hold off on re-rolling to see if it collects any other review.

-Peff