Fix error logs due to deletion of keys#28899
Conversation
83841b7 to
c93784c
Compare
apps/files_trashbin/lib/Trashbin.php
Outdated
| self::copy_recursive($source, $target, $view); | ||
|
|
||
| self::retainVersions($targetFilename, $owner, $ownerPath, $timestamp, true); | ||
| self::retainVersions($targetFilename, $owner, $ownerPath, $timestamp, false,true); |
apps/files_trashbin/lib/Trashbin.php
Outdated
| * @param bool $forceCopy true to only make a copy of the versions into the trashbin | ||
| */ | ||
| private static function retainVersions($filename, $owner, $ownerPath, $timestamp, $forceCopy = false) { | ||
| private static function retainVersions($filename, $owner, $ownerPath, $timestamp, $sourceStorage= false, $forceCopy = false) { |
There was a problem hiding this comment.
can we use null instead of false ?
There was a problem hiding this comment.
and there is missing space between $sourceStorage and =
apps/files_trashbin/lib/Trashbin.php
Outdated
|
|
||
| $copyKeysResult = false; | ||
|
|
||
| if (\OC::$server->getEncryptionManager()->isEnabled()) { |
There was a problem hiding this comment.
From what I see, in the past we never had encryption code directly in the trashbin.
There must be logic elsewhere already about moving/copying encryption keys to trash, it is likely that this logic exists directly in the encryption app. Can you find it ?
There was a problem hiding this comment.
@PVince81 So here is where the change triggers for deletion of keys to trash:
https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/Wrapper/Encryption.php#L644 which leads to https://github.com/owncloud/core/blob/master/apps/files_trashbin/lib/Storage.php#L119 and then the call goes down to https://github.com/owncloud/core/blob/master/apps/files_trashbin/lib/Storage.php#L163. From there finally the call reaches https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/Wrapper/Encryption.php#L250.
So should we move the code somewhere around https://github.com/owncloud/core/blob/master/apps/files_trashbin/lib/Storage.php ?
f80efd1 to
45ab3ec
Compare
When files are deleted and moved to trashbin, the keys are deleted. This results in exception due to file not exist. And hence this change helps to fix the issue. The issue is caused when the version of the files are tried to retain in the trashbin. Signed-off-by: Sujith H <sharidasan@owncloud.com>
45ab3ec to
11338d3
Compare
|
Can you make sure the following still works correctly:
I see your test steps above but they don't seem to cover checking that the restored versions still work. |
|
@PVince81 Below are the tests done Single Storage
Cross Storage
|
|
Please backport to stable10, set milestone to "planned" for the next release there |
|
Back port PR: #28934 |
|
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. |
When files are deleted and moved to trashbin,
the keys are deleted. This results in exception
due to file not exist. And hence this change
helps to fix the issue. The issue is caused
when the version of the files are tried to
retain in the trashbin.
Signed-off-by: Sujith H sharidasan@owncloud.com
Description
When the files are moved to trashbin, the retain version causes problem. Which means the retainVersion in the trashbin tries to access the file keys which are already deleted. So this causes exception.
Related Issue
#28883
Motivation and Context
This change tries to solve the issue caused by accessing file keys which are already deleted during the transfer of files to trashbin.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: