Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check checksums or file contents before assuming a file was moved (instead of just mtime+inode) #2983

Closed
FlorentCoppint opened this issue Mar 18, 2015 · 14 comments
Assignees
Milestone

Comments

@FlorentCoppint
Copy link

Hi everyone,

Today I upgraded my Owncloud server to 8.0.2.

Then I upgraded my client to 1.8. I'm on Ubuntu.

I didn't open my client for a few days so Owncloud Client synced a few files with server.

And in "Activity" I can see this line:
owncloud-bug

Can someone explain me this ?! files are totally different (.cnf is a text file & .gpg is a private key).
AND .cnf file was deleted yesterday on client !!
Owncloud client didn't modify anything on client, but on server, .gpg got content of .cnf... absolutely absude :)

Is it an issue ?

Tell me if you need more information.

Thank you.

@ogoffart
Copy link
Contributor

Can you please provide the log. The corresponding entry on the .owncloud.log file would be usefull.
Do you have more infrmation about the files? What was the size in byte of each files? and what was the mtime of each files?

Here is my theory: The .cnf file was removed, and the .gpg file was created a bit later but recieved the same inode as the previously deleted file. The size of the two files happen to be the same, and also the mtime was set to the same as the old one. And therefore the client tought it was a move.

@FlorentCoppint
Copy link
Author

I have this in logs:
15:04:16|695|git/mariadb-ansible/templates/my.cnf|INST_RENAME|Up|1426236634|55031d22b339a|0|00157393522e02bad1dfe|4||0|0|0|||INST_NONE|

Nothing about .gpg file.

That would be strange they had same size & same mtime...

.gpg file is 6714 bytes. I don't known exact size of .cnf file but it's around 5000-6000 ...

@dragotin dragotin added type:bug p2-high Escalation, on top of current planning, release blocker labels Mar 23, 2015
@dragotin dragotin added this to the 1.8.1 - Bugfix milestone Mar 23, 2015
@ogoffart
Copy link
Contributor

If this is what I think, it is a quite unlikely event but that still can happen. The only way I see to solve this problem would be to use checksum in the database

@FlorentCoppint
Copy link
Author

I think this is a good solution, at least for files having same size.

@dragotin dragotin modified the milestones: 1.9 - Multi-account, 1.8.1 - Bugfix Apr 8, 2015
@dragotin
Copy link
Contributor

dragotin commented Apr 8, 2015

We work on the checksum thing, but that wont make it to 1.8.1 any more. Moving.

@dragotin dragotin removed the p2-high Escalation, on top of current planning, release blocker label Apr 8, 2015
@guruz guruz modified the milestones: 2.1-next, 2.0 - Multi-account Jun 24, 2015
@ckamm
Copy link
Contributor

ckamm commented Oct 2, 2015

@dragotin Am I understanding correctly that this ticket should be called "Check checksums before assuming a file was moved"?

@guruz guruz changed the title File moved to another (totally different) Check checksums or file contents before assuming a file was moved (instead of just mtime+inode) Oct 2, 2015
@guruz
Copy link
Contributor

guruz commented Oct 2, 2015

Note the whole move detection business is tricky
https://github.com/owncloud/client/blob/master/csync/src/csync_update.c#L277-l295

@ogoffart
Copy link
Contributor

@ckamm yes exactly.
@guruz move detection is trivial, if the file is gone and there is a new file with the same inode and mtime and size, it is a move. (the same fileid and etag on the remote). The lines you pasted has nothing to do with moves, but maybe because the link is outdated.

The problem is that same inode, mtime, and size might still not be a move and checking checksum in addition should solve the problem.

@guruz
Copy link
Contributor

guruz commented Nov 3, 2015

Status: We now compute a checksum on download/upload and could use this info.
Still moving to 2.2 because the .eml problem is more important
#3235

@guruz guruz modified the milestones: 2.2-next, 2.1-next Nov 3, 2015
@guruz
Copy link
Contributor

guruz commented Nov 3, 2015

Slightly related: #4034

@MTRichards MTRichards added p2-high Escalation, on top of current planning, release blocker and removed p2-high Escalation, on top of current planning, release blocker labels Nov 11, 2015
@ckamm ckamm assigned ckamm and unassigned dragotin Jan 20, 2016
ckamm added a commit to ckamm/owncloud-client that referenced this issue Jan 20, 2016
This currently doesn't do much because we only compute content
checksums for .eml files.
ckamm added a commit that referenced this issue Jan 20, 2016
This currently doesn't do much because we only compute content
checksums for .eml files.
@ckamm ckamm added the ReadyToTest QA, please validate the fix/enhancement label Jan 20, 2016
@ckamm
Copy link
Contributor

ckamm commented Jan 20, 2016

The check is now implemented, but only works for files that have a content checksum (.eml files that were uploaded). See #4375 for details on enabling more checksumming.

@dragotin
Copy link
Contributor

@ckamm and @ogoffart do you think this should/could be added for all files? Once we have the checksum in the database we could use it, right?

@ckamm
Copy link
Contributor

ckamm commented Feb 29, 2016

@dragotin Yes, I think we should start creating content checksums for everything on download and then make use of them. Ticket #4375 is about that topic.

@mcastroSG
Copy link

mcastroSG commented May 11, 2016

Hi hi:

Test 1

Steps Executed

1.- Connect desktop client to a server that has cksum capabilities enabled
2.- Include file1.eml file to Shared sync folder
3.- Wait till sync ends
4.- Move file1.eml to file2.eml
5.- Wait till sync ends

Results

Desktop client check cksum to discover if this case is a move.

05-11 10:57:31:644 0x7f98d489e680 csync_walker: file: /Users/XXXX/ownCloud/file2.eml [inode=1261703 size=7] 05-11 10:57:31:645 0x7f98d489e680 _csync_detect_update: Checking for rename based on inode # 1261703 05-11 10:57:31:649 0x7f98d489e680 _csync_detect_update: checking checksum of potential rename file2.eml 54312ef83663ead54b89b1ed90e935239d8b2f74 <-> 54312ef83663ead54b89b1ed90e935239d8b2f74 05-11 10:57:31:649 0x7f98d489e680 _csync_detect_update: pot rename detected based on inode # 1261703 05-11 10:57:31:649 0x7f98d489e680 _csync_detect_update: file: file2.eml, instruction: INSTRUCTION_EVAL_RENAME <<=

Test 2

Steps Executed

1.- Connect desktop client to a server that has cksum capabilities enabled
2.- Include ppt file to Shared sync folder
3.- Wait till sync ends
4.- Move ppt file to change its name
5.- Wait till sync ends

Results

Desktop client check cksum to discover if this case is a move.

05-11 10:45:51:545 0x7f98d489e680 csync_walker: file: /Users/****/ownCloud/test3.ppt [inode=1256219 size=9] 05-11 10:45:51:546 0x7f98d489e680 _csync_detect_update: Checking for rename based on inode # 1256219 05-11 10:45:51:549 0x7f98d489e680 _csync_detect_update: checking checksum of potential rename test3.ppt b847219a7bbdc39ae9a941ffcf3db2c58cdf52ff <-> b847219a7bbdc39ae9a941ffcf3db2c58cdf52ff 05-11 10:45:51:549 0x7f98d489e680 _csync_detect_update: pot rename detected based on inode # 1256219 05-11 10:45:51:549 0x7f98d489e680 _csync_detect_update: file: test3.ppt, instruction: INSTRUCTION_EVAL_RENAME <<=

ServerOS: Ubuntu
Server Version: 9.1.0 Alpha modified to enable cksum capabilities

Client OS : OS X El capitán 10.11.4
Client Version:Version 2.2.0rc1 (build 3346)

Client OS: W81
Client Version:Version 2.2.0rc1 (build 6069)

@mcastroSG mcastroSG removed the ReadyToTest QA, please validate the fix/enhancement label May 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants