Re: [PATCH 2/2] connect: know that zero-ID is not a ref

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 2, 2016 at 12:56 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> On Fri, Sep 2, 2016 at 12:39 PM, Shawn Pearce <spearce@xxxxxxxxxxx> wrote:
>> On Fri, Sep 2, 2016 at 10:15 AM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> 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?

Indeed, you are correct.

I'm just stating the JGit client behavior is to look at
"capabilities^{}" in the first line as special, not the SHA-1.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]