Re: "git symbolic-ref" doesn't do a very good job

From: Jeff King <peff@peff.net>
Date: Aug 1, 2022, 17:43 UTC
Message-ID: <YugQqp4oN26OFOpt@coredump.intra.peff.net>
In-Reply-To: <CAHk-=wi5pfUcuaAUz=rifon9d51mshE7k6bkpMXddog0On9jow@mail.gmail.com>

On Sat, Jul 30, 2022 at 05:44:25PM -0700, Linus Torvalds wrote:

Show 7 quoted lines

> Put another way: I think my patch is likely the right thing to do (and
> I'd personally prefer the stricter check without the ALLOW_ONELEVEL
> flag), but you and Junio are right about it being a bigger change than
> I in my naivete thought it was.
> 
> So I won't really push for this, I suspect this needs very much to be
> a judgement call by you guys.

Just to lay out the options, I think we have:

  1. Do nothing. This breaks nothing. ;)

  2. Your patch, but with ALLOW_ONELEVEL. This fixes nonsense like
     "foo..bar", but doesn't break "FETCH_HEAD". Requires fixing t4202's
     ".lock" example. Replaces the HEAD starts_with("refs/") check.

  3. Your patch as-is. Same as (2), but also breaks FETCH_HEAD.

  4. Your patch, plus any extra tightening of HEAD to refs/heads/. I
     think this is probably breaking too much (I put more details
     elsewhere in the thread).

I'd be in favor of (2), which is really just catching syntactically invalid crap, and shouldn't break anyone. Technically it's possible somebody could be using a symref pointing at arbitrary data for who-knows-what reason, and extracting it with "symbolic-ref", but that is getting beyond far-fetched, I think.

I'm also tempted by (3), but we should be prepared for obscure breakage reports.

-Peff