From dc39bda8704d54d7481b9b42d2cea44146d18a3d Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 15 Apr 2015 09:49:50 +0200 Subject: [PATCH 1/3] move/copy from storage --- .../files/storage/wrapper/encryption.php | 74 +++++++++++++++++-- .../lib/files/storage/wrapper/encryption.php | 19 ++--- 2 files changed, 77 insertions(+), 16 deletions(-) diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index 24bbef9b93..c37d3b4347 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/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,65 @@ 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 = false) { + $result = $this->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, 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) { + 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); + if ($result and $preserveMtime) { + $this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath)); + } + fclose($source); + fclose($target); + + if (!$result) { + // 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 diff --git a/tests/lib/files/storage/wrapper/encryption.php b/tests/lib/files/storage/wrapper/encryption.php index 6c593bb774..bc803f5f87 100644 --- a/tests/lib/files/storage/wrapper/encryption.php +++ b/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); - } /** From e4829a23585069aaed7260ad69c041c1848b0342 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Thu, 30 Apr 2015 14:30:02 +0200 Subject: [PATCH 2/3] update 'encrypted'-flag in file cache according to the storage settings --- lib/private/files/storage/common.php | 5 ++ .../files/storage/wrapper/encryption.php | 48 ++++++++++++++++--- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php index 06c61fe693..fc30b385e2 100644 --- a/lib/private/files/storage/common.php +++ b/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) { diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index c37d3b4347..c2a29bdb0c 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -373,8 +373,14 @@ class Encryption extends Wrapper { * @param bool $preserveMtime * @return bool */ - public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = false) { - $result = $this->copyFromStorage($sourceStorage, $sourceInternalPath, $targetInternalPath, true); + 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); @@ -394,6 +400,26 @@ class Encryption extends Wrapper { * @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 $sourceInternalPath + * @param $targetInternalPath + * @param $preserveMtime + * @param $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); @@ -408,13 +434,23 @@ class Encryption extends Wrapper { $source = $sourceStorage->fopen($sourceInternalPath, 'r'); $target = $this->fopen($targetInternalPath, 'w'); list(, $result) = \OC_Helper::streamCopy($source, $target); - if ($result and $preserveMtime) { - $this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath)); - } fclose($source); fclose($target); - if (!$result) { + 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 From aea734aaf15d38f8b1f819fc2f693ec24236cad3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 6 May 2015 12:14:18 +0200 Subject: [PATCH 3/3] Fix missing types on doc block and deduplicate the method name --- lib/private/files/storage/wrapper/encryption.php | 8 ++++---- tests/lib/files/storage/wrapper/encryption.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php index c2a29bdb0c..c0c4c6979c 100644 --- a/lib/private/files/storage/wrapper/encryption.php +++ b/lib/private/files/storage/wrapper/encryption.php @@ -413,10 +413,10 @@ class Encryption extends Wrapper { * copy file between two storages * * @param \OCP\Files\Storage $sourceStorage - * @param $sourceInternalPath - * @param $targetInternalPath - * @param $preserveMtime - * @param $isRename + * @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) { diff --git a/tests/lib/files/storage/wrapper/encryption.php b/tests/lib/files/storage/wrapper/encryption.php index bc803f5f87..d4492e0092 100644 --- a/tests/lib/files/storage/wrapper/encryption.php +++ b/tests/lib/files/storage/wrapper/encryption.php @@ -199,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,