Re: [PATCH] receive-pack: crash when checking with non-exist HEAD

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 22, 2015, 20:30 UTC
Message-ID: <xmqqr3o0q6k7.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CANYiYbE3Vy_gtFMKTMw1wHLDhU758nXaJnKaNy6WMNRz0fjUnw@mail.gmail.com>

Jiang Xin <worldhello.net@gmail.com> writes:

Show 13 quoted lines

> If HEAD of a repository points to a conflict reference, such as:
>
> * There exist a reference named 'refs/heads/jx/feature1', but HEAD
>   points to 'refs/heads/jx', or
>
> * There exist a reference named 'refs/heads/feature', but HEAD points
>   to 'refs/heads/feature/bad'.
>
> When we push to delete a reference for this repo, such as:
>
>         git push /path/to/bad-head-repo.git :some/good/reference
>
> The git-receive-pack process will crash.

I see a similar "if head_name is NULL, don't bother." check in is_ref_checked_out() so in that sense this is a correct fix to the immediate problem. That check came from 986e8239 (receive-pack: detect push to current branch of non-bare repo, 2008-11-08).

This is a tangent, but if HEAD points at an unborn branch that cannot be created, wouldn't all other things break?

For example, in order to "git commit" from such a state to create the root commit on that branch, existing unrelated branches whose names collide with the branch must be removed, which would mean one of two things, either (1) you end up losing many unrelated work, or (2) the command refuses to work, not letting you to record the commit. Neither is satisfactory, but we seem to choose (2), which is at least the safer of the two:

    $ git checkout master
    $ git checkout --orphan master/1
    $ git commit -m foo
    fatal: cannot lock ref 'HEAD': 'refs/heads/master' exists;
    cannot create 'refs/heads/master/1'

We may want to avoid putting us in such a situation in the first place. Giving "checkout --orphan" an extra check might be a simple small thing we can do, i.e.

    $ git checkout master
    $ git checkout --orphan master/1
    fatal: 'master' branch exists, cannot create 'master/1'

But I suspect it would not protect us from different avenues that can cause this kind of thing; e.g. to prevent this:

    $ git branch -D next
    $ git checkout --orphan next/1
    $ git fetch origin master:refs/heads/next

creation of a ref "refs/heads/next" here must notice HEAD points at "refs/heads/next/1" (that does not yet exist) and do something intelligent about it.

Show 17 quoted lines

>  builtin/receive-pack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 94d0571..04cb5a1 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -911,7 +911,7 @@ static const char *update(struct command *cmd,
> struct shallow_info *si)
>       return "deletion prohibited";
>     }
>
> -   if (!strcmp(namespaced_name, head_name)) {
> +   if (head_name && !strcmp(namespaced_name, head_name)) {
>       switch (deny_delete_current) {
>       case DENY_IGNORE:
>         break;