diff --git a/lib/private/Files/Storage/Common.php b/lib/private/Files/Storage/Common.php index 4e95c594cf..2dbaf619c0 100644 --- a/lib/private/Files/Storage/Common.php +++ b/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); } diff --git a/lib/private/Files/Storage/Wrapper/Jail.php b/lib/private/Files/Storage/Wrapper/Jail.php index f21b571646..89f6df4983 100644 --- a/lib/private/Files/Storage/Wrapper/Jail.php +++ b/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, '/') . '/'; diff --git a/tests/lib/Files/Storage/CommonTest.php b/tests/lib/Files/Storage/CommonTest.php index 38faa9b0b2..f7be996e5e 100644 --- a/tests/lib/Files/Storage/CommonTest.php +++ b/tests/lib/Files/Storage/CommonTest.php @@ -1,27 +1,31 @@ . -* -*/ + * ownCloud + * + * @author Robin Appelman + * @copyright 2012 Robin Appelman icewind@owncloud.com + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE + * License as published by the Free Software Foundation; either + * version 3 of the License, or any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU Affero General Public + * License along with this library. If not, see . + * + */ 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')); + } }