-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Cannot restore previous file version with enabled encryption #22781
Comments
@schiesbn please have a look and evaluate severity |
Trying to reproduce this locally. Still failing here but retrying… |
I will also try to re-produce it |
@shhristov would be also good if you could reduce the steps to the minmum, e.g. does it really only happen for the second user? Does it really only happen if the recovery key is enabled? Do you need the sync client to reproduce it or does it also fail if you upload the same file twice on the web interface and restore the previous version. |
OK, I can reproduce it with following steps:
(no sync client, no recovery key and now second user needed to re-produce it) |
After the step 4, the current version has the version '2' in the file cache and the file copied to files_versions has the version '1'. That's correct, so this explains while at this point both downloads work. After restoring the file from files_versions, the current file still has the version '2'. That's why the signature check fails. If I change it manually to '1' the download works again. Also the new version in files_version has the wrong encryption version after restore. It still has the version '1' but should have the version '2'. Again if I change it manually to '1' the download works. So it seems we no longer update the versions correctly in the file cache. |
Got it. I missed the fact that one has to restore the file and not only download it. Creating patch… ⏳ |
Thanks @LukasReschke, looking forward to your patch! Btw, I wonder why we have to handle this at all. Shouldn't the file cache make sure to move all the meta data if a file gets renamed/copied without handling any special cases like what is stored in the 'encrypted' column? |
Ideally it should. There are two different bugs here:
I'm at the moment at fixing 2. 1 is already done locally. |
When calling `\OC\Files\View::copy` we should also keep the version to ensure that the file will always have the correct version attached and can be successfully decrypted. To test this the following steps are necessary (from #22781 (comment)): 1. setup a new ownCloud 9.0 beta2 2. enable encryption 2. upload a docx (5.7MB large) 3. upload the same file again and overwrite the existing file 4. I can download the original file and the first version 5. I restore the first version 6. restored version can no longer be downloaded with the error described above The manual cache operation in `\OCA\Files_Versions\Storage` is unfortunately necessary since `\OCA\Files_Versions\Storage::copyFileContents` is not using `\OCP\Files\Storage::moveFromStorage` in the case when an object storage is used. Due to the workaround added in 54cea05 the stream is directly copied and thus bypassing the FS.
Proposed patch is at #22796 @schiesbn @shhristov Mind testing? THX a lot. |
@icewind1991 can you answer this ? |
Just for the record, the patch does that now 😉 |
@cmonteroluque critical regression |
Works in 8.2.2, stable8.2. I tested some other cases related to copying files, they don't seem to be affected by this bug:
|
When calling `\OC\Files\View::copy` we should also keep the version to ensure that the file will always have the correct version attached and can be successfully decrypted. To test this the following steps are necessary (from #22781 (comment)): 1. setup a new ownCloud 9.0 beta2 2. enable encryption 2. upload a docx (5.7MB large) 3. upload the same file again and overwrite the existing file 4. I can download the original file and the first version 5. I restore the first version 6. restored version can no longer be downloaded with the error described above The manual cache operation in `\OCA\Files_Versions\Storage` is unfortunately necessary since `\OCA\Files_Versions\Storage::copyFileContents` is not using `\OCP\Files\Storage::moveFromStorage` in the case when an object storage is used. Due to the workaround added in 54cea05 the stream is directly copied and thus bypassing the FS.
When calling `\OC\Files\View::copy` we should also keep the version to ensure that the file will always have the correct version attached and can be successfully decrypted. To test this the following steps are necessary (from #22781 (comment)): 1. setup a new ownCloud 9.0 beta2 2. enable encryption 2. upload a docx (5.7MB large) 3. upload the same file again and overwrite the existing file 4. I can download the original file and the first version 5. I restore the first version 6. restored version can no longer be downloaded with the error described above The manual cache operation in `\OCA\Files_Versions\Storage` is unfortunately necessary since `\OCA\Files_Versions\Storage::copyFileContents` is not using `\OCP\Files\Storage::moveFromStorage` in the case when an object storage is used. Due to the workaround added in 54cea05 the stream is directly copied and thus bypassing the FS.
Backport to 9.0: #22818 |
Thank you, guys, for the quick reaction!
Or the patch is not yet applied there? |
@shhristov can you file a new issue? THX |
Ok. |
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. |
Steps to reproduce
Expected behaviour
Show the readable contents of the file or offer to save the file on the local computer in a readable state. The desktop client should successfully sync the file and the file should be in a readable and usable form.
Actual behaviour
When I try to open the file from the web interface Firefox says:
The desktop client says:
I find this error in Owncloud's log:
Server configuration
Operating system:
Web server:
Apache/2.4.7
Database:
PostgreSQL 9.3.11
PHP version:
HP/5.5.9-1ubuntu4.14
ownCloud version: (see ownCloud admin page)
9.0.0 beta 2 (testing)
Updated from an older ownCloud or fresh install:
No
Where did you install ownCloud from:
Official repository:
deb http://download.owncloud.org/download/repositories/9.0:/testing/xUbuntu_14.04/ /
Signing status (ownCloud 9.0 and above):
List of activated apps:
The content of config/config.php:
Are you using external storage, if yes which one:
No
Are you using encryption: yes/no
Yes
Are you using an external user-backend, if yes which one:
no, but I am planning to use LDAP
Client configuration
Browser:
Firefox 44.0.2
Operating system:
Windows 10
Logs
Web server error log
ownCloud log (data/owncloud.log)
Browser log
The text was updated successfully, but these errors were encountered: