Re: Bug report: stash in upstream caused remote fetch to fail

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 6, 2014, 23:22 UTC
Message-ID: <xmqq4n5ghf0z.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140106230348.GA7811@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 18 quoted lines

> Then we ask fetch_refs_via_pack to get the actual objects for us. And
> it checks our refs again, with this call chain:
>
>   do_fetch
>     fetch_refs
>       transport_fetch_refs
>         fetch_refs_via_pack
>           fetch_pack
>             do_fetch_pack
>               everything_local
>                 filter_refs
>                   check_refname_format
>
> Here, the code looks like this:
>
>   if (!memcmp(ref->name, "refs/", 5) &&
>       check_refname_format(ref->name + 5, 0))
>     ; /* trash */

I think this should feed ref->name, not ref->name + 5; an obvious alternative would be to use REFNAME_ALLOW_ONELEVEL; you are also right that && is probably a misspelt ||; this is about protecting ourselves from creating garbage in our ref namespace, so accepting anything outside refs/ makes little sense.

> It's really not clear to me what the check in filter_refs was trying to
> do. It dates all the way back to 1baaae5 (Make maximal use of the remote
> refs, 2005-10-28), but there is not much explanation. I haven't dug into
> the list around that time to see if there's any discussion.

I think the "funny refs" the log message talks about is about filtering "we know we can reach these objects via our alternates, but these are not refs we actually have".