Transferid collisions

[PVince81]

See owncloud/core#17956 (comment)

This was found by running "test_basicSync.py" on my environment (see initial steps).
It looks like two instances of sync client from the test are uploading the same file at the same time. Since the start time is the same, the transfer id is the same.
I suspect that the transfer id is based on the timestamp.

@LukasReschke suggested using a low secure random instead.

This was with:

• owncloud-client-1.8.4-2.5.x86_64
• openSUSE Factory
• laptop has SSD and smashdir is a ramdisk (makes the whole thing faster)
• server is ownCloud 8.1 (stable 8.1 branch)

@ogoffart @guruz

[LukasReschke]

@LukasReschke suggested using a low secure random instead.

That only in case https://github.com/owncloud/core/blob/c700f42b68f430e9c89ce97b92ea91323c9f6ed5/lib/private/filechunking.php#L151 is indeed used for that. No idea how that chunking really works… Looked at it soooooooo long ago 🙊

[PVince81]

signature_split isn't used anywhere. It looks like it's very old code.

The transfer id is actually parsed from the file name sent by the client: https://github.com/owncloud/core/blob/c700f42b68f430e9c89ce97b92ea91323c9f6ed5/lib/private/filechunking.php#L36

[PVince81]

I grepped the sync logs I saw that the transfer id 3657486019 appears in both the following logs:

• creator-ocsync.step01.cnt000.log
• loser-ocsync.step04.cnt000.log

@guruz pointed out at https://github.com/owncloud/client/blob/master/src/libsync/propagateupload.cpp#L256 and

client/src/gui/folder.cpp

Line 73 in 7fc7925

qsrand(QTime::currentTime().msec());

Here is my Qt version in case it would affect the algo:
libQt5Core5-5.4.2-2.3.x86_64

[PVince81]

@guruz advised me to add this in cmd.php's main:

qsrand(QTime::currentTime().msec() * QCoreApplication::applicationPid());

Now when run the smashbox test the collision is gone and all tests pass!

[PVince81]

Fixed through 79d895e

@guruz backport ?

[guruz]

With the 2.0 beta coming out next week (hopefully) I don't think it's needed :)
thanks for spotting!

[PVince81]

@guruz if we don't it means smashbox tests will randomly fail in CI as it does now. Unless we switch CI directly to 2.0 once it's out.
@DeepDiver1975 what do you think ?

[guruz]

Cherry-picked into 1.8
65933d5

[DeepDiver1975]

Cherry-picked into 1.8

THX - so I'll run ci with 1.8 branch build