[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Check SHA vs Content (was: RE: svn commit: r1759233 - /subversion/trunk/subversion/libsvn_wc/questions.c)

From: Jacek Materna <jacek_at_assembla.com>
Date: Tue, 23 May 2017 13:35:19 +0200

On Mon, May 22, 2017 at 10:56 PM, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
> On Tue, May 9, 2017 at 1:21 PM, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
>> On Tue, Apr 4, 2017 at 11:33 AM, Stefan Sperling <stsp_at_elego.de> wrote:
>>> On Mon, Feb 20, 2017 at 09:05:25AM +0100, Bert Huijben wrote:
>>>> This code is still in trunk without any of the discussed improvements, so
>>>> this change is currently part of 1.10.0-alpha1.
>>>>
>>>> If we don't implement the improvements I think we should check if we want
>>>> to revert to the 1.0-1.9 behavior before we really look at releasing 1.10.
>>>>
>>>> See discussion below
>>>>
>>>> Bert
>>>
>>> I think the proposed approach as implemented on trunk can no longer be
>>> considered viable, unfortunately, because of this step:
>>>
>>>> > >>> 4. Calculate SHA-1 checksum of detranslated contents of working file
>>>> > >>> and compare it with pristine's checksum stored in wc.db.
>>>
>>> Given that the SHA1 collision problem is real, we are now trying to stop
>>> relying on hashes to compare content. So it does not make sense to add
>>> new code which relies on hashes in this way, in my opinion.
>>>
>>> It seems that using SHA1 to compare content is key to the proposed approach.
>>> If that is correct, then I don't agree with releasing 1.10 with this feature
>>> and I would be in favour of reverting this change.
>>>
>>> Ivan, do you have any further comments on this thread? You have remained
>>> silent for quite some time now :(
>>
>> Where are we with this? Seems the consensus is to revert r1759233 to
>> not further increase our reliance on sha1? Or is there still a way to
>> keep r1759233 in some way, and improve it to make the sha1 test
>> "sensitive but not specific", like danielsh proposed?
>>
>> Ivan?
>
> Hmmm, I'm wondering, now we decided to disallow sha1 collisions to
> enter the repository, how does this reflect on this discussion?
>
> Okay, it's another "collision-sensitive-dependency" on sha1, but there
> are others in the working copy (pristines) and ra_serf, and since
> we're assuming a non-sha1-collision world (by blocking them from the
> repository), it makes little sense to me to simply revert this, and
> not fix other sha1-dependencies. OTOH, if we want to gradually reduce
> our dependence on sha1, this is an easy one to remove, since it's not
> in production yet ... Dunno.
>
> However, there were also other, performance-related, objections to the
> new implementation. Ivan proposed improvements [1] but they were never
> implemented.
>
> So, what to do?
>
> Unless someone objects in the coming couple of days, I'd say we revert r1759233.
>
> [1] https://svn.haxx.se/dev/archive-2016-09/0016.shtml
>
> --
> Johan

I agree in not adding more new logs to the fire. I also agree we
should attack the already-in-prod deps on SHA1 as well in the
background-
Received on 2017-05-23 13:36:05 CEST

This is an archived mail posted to the Subversion Dev mailing list.