Re: Be more careful about updating refs

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 16, 2008, 01:11 UTC
Message-ID: <7vzlv6bmm3.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.1.00.0801151546560.2806@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 9 quoted lines

> This makes write_ref_sha1() more careful: it actually checks the SHA1 of 
> the ref it is updating, and refuses to update a ref with an object that it 
> cannot find.
>
> Perhaps more importantly, it also refuses to update a branch head with a 
> non-commit object. I don't quite know *how* the stable series maintainers 
> were able to corrupt their repository to have a HEAD that pointed to a tag 
> rather than a commit object, but they did. Which results in a totally 
> broken repository that cannot be cloned or committed on.

Two questions and a comment:

 - Do we want to impose the same restriction on refs/remotes/?
   I think that is a logical thing to do.

 - What should the receive-pack and git-fetch do if they trigger
   the check in this codepath while updating the refs under the
   affected hierarchies?  Fail the push and fetch?

 - I think !strcmp(refname, "HEAD") is not quite a right check
   to do here.  In order to catch the detached head case, it
   must be checked, but at the same time if the head is not
   detached, it should look at where the symref points at
   (i.e. a symref HEAD that points outside refs/heads is an
   error, and we need to catch that).

> +static int is_branch(const char *refname)
> +{
> +	return !strcmp(refname, "HEAD") || !prefixcmp(refname, "refs/heads/");
> +}