Re: [PATCH 2/2] connect: know that zero-ID is not a ref
- From
Stefan Beller <sbeller@google.com>
- Date
- Sep 2, 2016, 19:56 UTC
- Message-ID
- <CAGZ79kYWr09K2RXgSmmy4G=to65LbLfCi=GRkykhciq_vsyMZQ@mail.gmail.com>
- In-Reply-To
- <CAJo=hJssMohNq5zDU=wdb1gP4nW59m8pUmxP7eY4jnjr502z0w@mail.gmail.com>
On Fri, Sep 2, 2016 at 12:39 PM, Shawn Pearce <spearce@spearce.org> wrote:
Show 31 quoted lines
> On Fri, Sep 2, 2016 at 10:15 AM, Jonathan Tan <jonathantanmy@google.com> wrote:
>>
>> + if (is_null_oid(&old_oid)) {
>> + if (strcmp(name, "capabilities^{}"))
>
> Its not the zero ID that is special, its the "capabilities^{}" name
> that is special when its the first entry in the stream. In the wire
> protocol a "x^{}" line is a modifier to a prior "x" line to add a
> peeled object to the prior line. But if we see "^{}" on the first line
> that is non-sense, there is no prior line to modify with this
> identifier.
>
> Further ^{} is used here because its invalid in a name. A server
> really cannot have a reference that ends with the sequence ^{}. And a
> server should not have a reference named "capabilities" without a
> "refs/" prefix on it.
>
> So the entire "capabilities^{}" on the first line is a bunch of
> contradictions that violate a number of things about the protocol,
> which is why clients should ignore it.
>
> I think the test should be about:
>
> !*list && !strcmp(name, "capabilities^{}")
>
>> + warning("zero object ID received that is not accompanied by a "
>> + "capability declaration, ignoring and continuing anyway");
>
> Annoyingly a zero object ID is sort of possible; with a probability of
> 1/2^160 or something. Its just a very very unlikely value. Slightly
> stronger to test against the known invalid name.That ship has sailed long ago I would think? There are tests for null sha1 in the refs code, e.g. for deletion/creation of a branch we consider the null sha1 as the counterpart.
So while it may be possible to have SHA1 producing a "0"x40, you cannot e.g. push it?