Merge pull request #15628 from owncloud/enc_fix_rename

Encryption - fix moving/copying files between storages

lib/private/files/storage/common.php

@@ -545,6 +545,11 @@ abstract class Common implements Storage {
 			}
 		} 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');
 			list(, $result) = \OC_Helper::streamCopy($source, $target);
 			if ($result and $preserveMtime) {

lib/private/files/storage/wrapper/encryption.php

@@ -61,19 +61,20 @@ class Encryption extends Wrapper {
 	/** @var IMountPoint */
 	private $mount;
 
-	/** @var \OCP\Encryption\Keys\IStorage */
+
+	/** @var IStorage */
 	private $keyStorage;
 
-	/** @var \OC\Encryption\Update */
+	/** @var Update */
 	private $update;
 
 	/**
 	 * @param array $parameters
-	 * @param \OCP\Encryption\IManager $encryptionManager
-	 * @param \OC\Encryption\Util $util
-	 * @param \OCP\ILogger $logger
-	 * @param \OCP\Encryption\IFile $fileHelper
-	 * @param string $uid user who perform the read/write operation (null for public access)
+	 * @param IManager $encryptionManager
+	 * @param Util $util
+	 * @param ILogger $logger
+	 * @param IFile $fileHelper
+	 * @param string $uid
 	 * @param IStorage $keyStorage
 	 * @param Update $update
 	 */
@@ -365,6 +366,101 @@ class Encryption extends Wrapper {
 		}
 	}
 
+	/**
+	 * @param \OCP\Files\Storage $sourceStorage
+	 * @param string $sourceInternalPath
+	 * @param string $targetInternalPath
+	 * @param bool $preserveMtime
+	 * @return bool
+	 */
+	public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = true) {
+
+		// TODO clean this up once the underlying moveFromStorage in OC\Files\Storage\Wrapper\Common is fixed:
+		// - call $this->storage->moveFromStorage() instead of $this->copyBetweenStorage
+		// - copy the file cache update from  $this->copyBetweenStorage to this method
+		// - remove $this->copyBetweenStorage
+
+		$result = $this->copyBetweenStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime, true);
+		if ($result) {
+			if ($sourceStorage->is_dir($sourceInternalPath)) {
+				$result &= $sourceStorage->rmdir($sourceInternalPath);
+			} else {
+				$result &= $sourceStorage->unlink($sourceInternalPath);
+			}
+		}
+		return $result;
+	}
+
+
+	/**
+	 * @param \OCP\Files\Storage $sourceStorage
+	 * @param string $sourceInternalPath
+	 * @param string $targetInternalPath
+	 * @param bool $preserveMtime
+	 * @return bool
+	 */
+	public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = false) {
+
+		// TODO clean this up once the underlying moveFromStorage in OC\Files\Storage\Wrapper\Common is fixed:
+		// - call $this->storage->moveFromStorage() instead of $this->copyBetweenStorage
+		// - copy the file cache update from  $this->copyBetweenStorage to this method
+		// - remove $this->copyBetweenStorage
+
+		return $this->copyBetweenStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime, false);
+	}
+
+	/**
+	 * copy file between two storages
+	 *
+	 * @param \OCP\Files\Storage $sourceStorage
+	 * @param string $sourceInternalPath
+	 * @param string $targetInternalPath
+	 * @param bool $preserveMtime
+	 * @param bool $isRename
+	 * @return bool
+	 */
+	private function copyBetweenStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime, $isRename) {
+		if ($sourceStorage->is_dir($sourceInternalPath)) {
+			$dh = $sourceStorage->opendir($sourceInternalPath);
+			$result = $this->mkdir($targetInternalPath);
+			if (is_resource($dh)) {
+				while ($result and ($file = readdir($dh)) !== false) {
+					if (!Filesystem::isIgnoredDir($file)) {
+						$result &= $this->copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file);
+					}
+				}
+			}
+		} else {
+			$source = $sourceStorage->fopen($sourceInternalPath, 'r');
+			$target = $this->fopen($targetInternalPath, 'w');
+			list(, $result) = \OC_Helper::streamCopy($source, $target);
+			fclose($source);
+			fclose($target);
+
+			if($result) {
+				if ($preserveMtime) {
+					$this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath));
+				}
+				$isEncrypted = $this->mount->getOption('encrypt', true) ? 1 : 0;
+
+				// in case of a rename we need to manipulate the source cache because
+				// this information will be kept for the new target
+				if ($isRename) {
+					$sourceStorage->getCache()->put($sourceInternalPath, ['encrypted' => $isEncrypted]);
+				} else {
+					$this->getCache()->put($targetInternalPath, ['encrypted' => $isEncrypted]);
+				}
+			} else {
+				// delete partially written target file
+				$this->unlink($targetInternalPath);
+				// delete cache entry that was created by fopen
+				$this->getCache()->remove($targetInternalPath);
+			}
+		}
+		return (bool)$result;
+
+	}
+
 	/**
 	 * get the path to a local version of the file.
 	 * The local version of the file can be temporary and doesn't have to be persistent across requests

tests/lib/files/storage/wrapper/encryption.php

@@ -106,15 +106,17 @@ class Encryption extends \Test\Files\Storage\Storage {
 			->willReturn(['encrypted' => false]);
 
 		$this->instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption')
-			->setConstructorArgs([
+			->setConstructorArgs(
 				[
-					'storage' => $this->sourceStorage,
-					'root' => 'foo',
-					'mountPoint' => '/',
-					'mount' => $mount
-				],
-				$this->encryptionManager, $this->util, $logger, $file, null, $this->keyStore, $this->update
-			])
+					[
+						'storage' => $this->sourceStorage,
+						'root' => 'foo',
+						'mountPoint' => '/',
+						'mount' => $mount
+					],
+					$this->encryptionManager, $this->util, $logger, $file, null, $this->keyStore, $this->update
+				]
+			)
 			->setMethods(['getMetaData', 'getCache'])
 			->getMock();
 
@@ -125,7 +127,6 @@ class Encryption extends \Test\Files\Storage\Storage {
 		$this->instance->expects($this->any())
 			->method('getCache')
 			->willReturn($this->cache);
-
 	}
 
 	/**
@@ -198,7 +199,7 @@ class Encryption extends \Test\Files\Storage\Storage {
 	 * @param boolean $copyKeysReturn
 	 * @param boolean $shouldUpdate
 	 */
-	public function testCopy($source,
+	public function testCopyEncryption($source,
 							 $target,
 							 $encryptionEnabled,
 							 $copyKeysReturn,