Merge pull request #17278 from nextcloud/backport/17264/stable16

[stable16] handle moveFromStorage within the same storage even when storage wrap…
This commit is contained in:
Roeland Jago Douma 2019-09-27 12:28:27 +02:00 committed by GitHub
commit 0ecc8ed904
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 134 additions and 21 deletions

View File

@ -46,6 +46,8 @@ use OC\Files\Cache\Scanner;
use OC\Files\Cache\Updater; use OC\Files\Cache\Updater;
use OC\Files\Filesystem; use OC\Files\Filesystem;
use OC\Files\Cache\Watcher; use OC\Files\Cache\Watcher;
use OC\Files\Storage\Wrapper\Jail;
use OC\Files\Storage\Wrapper\Wrapper;
use OCP\Files\EmptyFileNameException; use OCP\Files\EmptyFileNameException;
use OCP\Files\FileNameTooLongException; use OCP\Files\FileNameTooLongException;
use OCP\Files\InvalidCharacterInPathException; use OCP\Files\InvalidCharacterInPathException;
@ -635,6 +637,23 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
return (bool)$result; 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 IStorage $sourceStorage
* @param string $sourceInternalPath * @param string $sourceInternalPath
@ -642,7 +661,16 @@ abstract class Common implements Storage, ILockingStorage, IWriteStreamStorage {
* @return bool * @return bool
*/ */
public function moveFromStorage(IStorage $sourceStorage, $sourceInternalPath, $targetInternalPath) { 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); return $this->rename($sourceInternalPath, $targetInternalPath);
} }

View File

@ -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) { public function getJailedPath($path) {
$root = rtrim($this->rootPath, '/') . '/'; $root = rtrim($this->rootPath, '/') . '/';

View File

@ -22,6 +22,10 @@
namespace Test\Files\Storage; namespace Test\Files\Storage;
use OC\Files\Storage\Wrapper\Jail;
use OC\Files\Storage\Wrapper\Wrapper;
use PHPUnit\Framework\MockObject\MockObject;
/** /**
* Class CommonTest * Class CommonTest
* *
@ -34,15 +38,88 @@ class CommonTest extends Storage {
* @var string tmpDir * @var string tmpDir
*/ */
private $tmpDir; private $tmpDir;
protected function setUp() { protected function setUp() {
parent::setUp(); parent::setUp();
$this->tmpDir = \OC::$server->getTempManager()->getTemporaryFolder(); $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() { protected function tearDown() {
\OC_Helper::rmdirr($this->tmpDir); \OC_Helper::rmdirr($this->tmpDir);
parent::tearDown(); 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'));
}
} }