Merge pull request #17264 from nextcloud/move-from-storage-wrappers

handle moveFromStorage within the same storage even when storage wrap…

lib/private/Files/Storage/Common.php

@@ -46,6 +46,8 @@ use OC\Files\Cache\Scanner;
 use OC\Files\Cache\Updater;
 use OC\Files\Filesystem;
 use OC\Files\Cache\Watcher;
+use OC\Files\Storage\Wrapper\Jail;
+use OC\Files\Storage\Wrapper\Wrapper;
 use OCP\Files\EmptyFileNameException;
 use OCP\Files\FileNameTooLongException;
 use OCP\Files\InvalidCharacterInPathException;
@@ -635,6 +637,23 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
 		return (bool)$result;
 	}
 
+	/**
+	 * Check if a storage is the same as the current one, including wrapped storages
+	 *
+	 * @param IStorage $storage
+	 * @return bool
+	 */
+	private function isSameStorage(IStorage $storage): bool {
+		while ($storage->instanceOfStorage(Wrapper::class)) {
+			/**
+			 * @var Wrapper $sourceStorage
+			 */
+			$storage = $storage->getWrapperStorage();
+		}
+
+		return $storage === $this;
+	}
+
 	/**
 	 * @param IStorage $sourceStorage
 	 * @param string $sourceInternalPath
@@ -642,7 +661,16 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
 	 * @return bool
 	 */
 	public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath) {
-		if ($sourceStorage === $this) {
+		if ($this->isSameStorage($sourceStorage)) {
+			// resolve any jailed paths
+			while ($sourceStorage->instanceOfStorage(Jail::class)) {
+				/**
+				 * @var Jail $sourceStorage
+				 */
+				$sourceInternalPath = $sourceStorage->getUnjailedPath($sourceInternalPath);
+				$sourceStorage = $sourceStorage->getUnjailedStorage();
+			}
+
 			return $this->rename($sourceInternalPath, $targetInternalPath);
 		}
 

lib/private/Files/Storage/Wrapper/Jail.php

@@ -62,6 +62,14 @@ class Jail extends Wrapper {
 		}
 	}
 
+	/**
+	 * This is separate from Wrapper::getWrapperStorage so we can get the jailed storage consistently even if the jail is inside another wrapper
+	 */
+	public function getUnjailedStorage() {
+		return $this->storage;
+	}
+
+
 	public function getJailedPath($path) {
 		$root = rtrim($this->rootPath, '/') . '/';
 

tests/lib/Files/Storage/CommonTest.php

@@ -22,6 +22,10 @@
 
 namespace Test\Files\Storage;
 
+use OC\Files\Storage\Wrapper\Jail;
+use OC\Files\Storage\Wrapper\Wrapper;
+use PHPUnit\Framework\MockObject\MockObject;
+
 /**
  * Class CommonTest
  *
@@ -34,15 +38,88 @@ class CommonTest extends Storage {
 	 * @var string tmpDir
 	 */
 	private $tmpDir;
+
 	protected function setUp() {
 		parent::setUp();
 
 		$this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder();
-		$this->instance=new \OC\Files\Storage\CommonTest(array('datadir'=>$this->tmpDir));
+		$this->instance = new \OC\Files\Storage\CommonTest(['datadir' => $this->tmpDir]);
 	}
 
 	protected function tearDown() {
 		\OC_Helper::rmdirr($this->tmpDir);
 		parent::tearDown();
 	}
+
+	public function testMoveFromStorageWrapped() {
+		/** @var \OC\Files\Storage\CommonTest|MockObject $instance */
+		$instance = $this->getMockBuilder(\OC\Files\Storage\CommonTest::class)
+			->setMethods(['copyFromStorage', 'rmdir', 'unlink'])
+			->setConstructorArgs([['datadir' => $this->tmpDir]])
+			->getMock();
+		$instance->method('copyFromStorage')
+			->willThrowException(new \Exception('copy'));
+
+		$source = new Wrapper([
+			'storage' => $instance,
+		]);
+
+		$instance->file_put_contents('foo.txt', 'bar');
+		$instance->moveFromStorage($source, 'foo.txt', 'bar.txt');
+		$this->assertTrue($instance->file_exists('bar.txt'));
+	}
+
+	public function testMoveFromStorageJailed() {
+		/** @var \OC\Files\Storage\CommonTest|MockObject $instance */
+		$instance = $this->getMockBuilder(\OC\Files\Storage\CommonTest::class)
+			->setMethods(['copyFromStorage', 'rmdir', 'unlink'])
+			->setConstructorArgs([['datadir' => $this->tmpDir]])
+			->getMock();
+		$instance->method('copyFromStorage')
+			->willThrowException(new \Exception('copy'));
+
+		$source = new Jail([
+			'storage' => $instance,
+			'root' => 'foo'
+		]);
+		$source = new Wrapper([
+			'storage' => $source
+		]);
+
+		$instance->mkdir('foo');
+		$instance->file_put_contents('foo/foo.txt', 'bar');
+		$instance->moveFromStorage($source, 'foo.txt', 'bar.txt');
+		$this->assertTrue($instance->file_exists('bar.txt'));
+	}
+
+	public function testMoveFromStorageNestedJail() {
+		/** @var \OC\Files\Storage\CommonTest|MockObject $instance */
+		$instance = $this->getMockBuilder(\OC\Files\Storage\CommonTest::class)
+			->setMethods(['copyFromStorage', 'rmdir', 'unlink'])
+			->setConstructorArgs([['datadir' => $this->tmpDir]])
+			->getMock();
+		$instance->method('copyFromStorage')
+			->willThrowException(new \Exception('copy'));
+
+		$source = new Jail([
+			'storage' => $instance,
+			'root' => 'foo'
+		]);
+		$source = new Wrapper([
+			'storage' => $source
+		]);
+		$source = new Jail([
+			'storage' => $source,
+			'root' => 'bar'
+		]);
+		$source = new Wrapper([
+			'storage' => $source
+		]);
+
+		$instance->mkdir('foo');
+		$instance->mkdir('foo/bar');
+		$instance->file_put_contents('foo/bar/foo.txt', 'bar');
+		$instance->moveFromStorage($source, 'foo.txt', 'bar.txt');
+		$this->assertTrue($instance->file_exists('bar.txt'));
+	}
 }