TextEditor: Checksum not invalidated on file change

[dragotin]

Steps to reproduce

1. Create a text file with the sync client
2. Wait until it is synced.
3. Check that it has a checksum in the database, which was sent from the client.
4. Edit the file from the built in texteditor, let it save.
5. See that the checksum of the file has not changed.

Expected behaviour

When a file is changed, the checksum on the server needs to be recalculated. If that can not be done, it needs at least to be invalidated.

Having a wrong checksum for a file is a critical problem as the client will complain about a corrupted file.

Server configuration

Operating system:
Linux
Web server:
apache
Database:
mysql
PHP version:
5.5.14
ownCloud version: (see ownCloud admin page)
9.0.x, master branch, 15833d9
Updated from an older ownCloud or fresh install:

Where did you install ownCloud from:
git
Are you using external storage, if yes which one: local/smb/sftp/...
no external storage
Are you using encryption: yes/no
no encryption
Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/...
no user-backend.

[MorrisJobke]

sev3 because there is not yet any client out there that uses the checksums.

[rullzer]

mmmm I don't get this. Doesn't the texteditor use dav?

[rullzer]

Ah no it seems it uses internal calls. So when we modify a file we must just reset the checksum to the empty string. @icewind1991 can you help with that?

I think it only has to be handled in the view.php::file_put_contents function right?

[rullzer]

Raised to sev2 as this has to be fixed for 9.0.2 from my POV. @cmonteroluque

[dragotin]

@MorrisJobke ...exept the few desktop clients (version 2.1.1) that some enthusiasts might use ;-). The client uses the checksum to validate the download from the server by comparing the server stored checksum to one that the client calculates after download.

With this bug, the client will refuse the download of the file once it was saved by the text editor because the checksums do not match. I wonder which other apps use "internal calls" to store data and have the same problem.

Well, morris, the fix is easy once the users call because of this: Just update the filecache and set all checksums to empty. 😢

[MorrisJobke]

exept the few desktop clients (version 2.1.1) that some enthusiasts might use ;-)

Oh. Really? I thought it will be present in the next major release 🙈 Sorry

[MorrisJobke]

Raised to sev2 as this has to be fixed for 9.0.2 from my POV. @cmonteroluque

Yep.

[ghost]

I agree

[rullzer]

Ok let me dive into this.

[rullzer]

Ok there are actually a few scenarios here.

There is the scenario where somebody calls file_put_content. I have that fixed locally I think. We then just clear the checksum.

But what to do if somebody calls fopen. I think it is safest just to also clear the checksum if the file is opened with write. We may look into the future into a wrapper or something that only clears is actually something is writen/deleted. But for now clearing seems best. CC: @PVince81 @dragotin

[rullzer]

PR 1 of 2 in #24098

[PVince81]

@rullzer yes, I guess you meant only for fopen in one of the write modes, not read.
You might also be able to do it at close time, see https://github.com/owncloud/core/blob/v9.0.1/lib/private/files/view.php#L1092 (this code is run after the file was closed).

It's probably best to clear the checksum in one of the existing database calls instead of adding new ones though (which makes the "close time" approach less interesting)

[rullzer]

Yes I agree. But the current calls don't allow me to easily inject it there.. without doing it all the time. For example on move and copy we don't want to clear the checksum etc.

[rullzer]

PR that fixes both cases in #24098 now

[rullzer]

Stable 9 backport in #24129

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.