fix moving files from external storage to object store trashbin

having the "cache rename" after the "storage move" caused the target
to get the fileid from the source file, without taking care that the object
is stored under the original file id.

By doing the "cache rename" first, we trigger the "update existing file"
logic while moving the file to the object store and the object gets stored for the
correct file id

Signed-off-by: Robin Appelman <robin@icewind.nl>

diff --git a/apps/files_trashbin/lib/Trashbin.php b/apps/files_trashbin/lib/Trashbin.php
index f73cc1f4aa6affe1718fdfd8ba5a1c6c213b5515..e23368f825bee2f4f40bbeed318b4012c61d8a69 100644 (file)
--- a/apps/files_trashbin/lib/Trashbin.php
+++ b/apps/files_trashbin/lib/Trashbin.php
@@ -278,6 +278,8 @@ class Trashbin {
                /** @var \OC\Files\Storage\Storage $sourceStorage */
                [$sourceStorage, $sourceInternalPath] = $ownerView->resolvePath('/files/' . $ownerPath);
 
+               $trashStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath);
+
                try {
                        $moveSuccessful = true;
                        if ($trashStorage->file_exists($trashInternalPath)) {
@@ -301,8 +303,6 @@ class Trashbin {
                        return false;
                }
 
-               $trashStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath);
-
                if ($moveSuccessful) {
                        $query = \OC_DB::prepare("INSERT INTO `*PREFIX*files_trash` (`id`,`timestamp`,`location`,`user`) VALUES (?,?,?,?)");
                        $result = $query->execute([$filename, $timestamp, $location, $owner]);

diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php
index a62b7d727fb0a26404a99f44e4eedd758f904be5..ada037768bd09b838f0fd5c033aacf7e1cd26166 100644 (file)
--- a/lib/private/Files/Storage/Common.php
+++ b/lib/private/Files/Storage/Common.php
@@ -620,18 +620,15 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
                        }
                } else {
                        $source = $sourceStorage->fopen($sourceInternalPath, 'r');
-                       // TODO: call fopen in a way that we execute again all storage wrappers
-                       // to avoid that we bypass storage wrappers which perform important actions
-                       // for this operation. Same is true for all other operations which
-                       // are not the same as the original one.Once this is fixed we also
-                       // need to adjust the encryption wrapper.
-                       $target = $this->fopen($targetInternalPath, 'w');
-                       [, $result] = \OC_Helper::streamCopy($source, $target);
+                       if ($source) {
+                               $result = $this->writeStream($targetInternalPath, $source) > 0;
+                       } else {
+                               $result = false;
+                       }
+
                        if ($result and $preserveMtime) {
                                $this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath));
                        }
-                       fclose($source);
-                       fclose($target);
 
                        if (!$result) {
                                // delete partially written target file