From 135cfd2c5573596545332787c2a77e112e98d4a3 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 28 May 2019 13:52:32 +0200 Subject: [PATCH 1/2] dont use part files for dav writes when the target folder doesn't have create permissions Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/File.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 3c42f66a04..9815059c59 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -147,7 +147,12 @@ class File extends Node implements IFile { if ($needsPartFile) { // mark file as partial while uploading (ignored by the scanner) $partFilePath = $this->getPartFileBasePath($this->path) . '.ocTransferId' . rand() . '.part'; - } else { + + if (!$view->isCreatable($partFilePath) && $view->isUpdatable($this->path)) { + $needsPartFile = false; + } + } + if (!$needsPartFile) { // upload file directly as the final path $partFilePath = $this->path; @@ -178,7 +183,7 @@ class File extends Node implements IFile { } $isEOF = false; - $wrappedData = CallbackWrapper::wrap($data, null, null, null, null, function($stream) use (&$isEOF) { + $wrappedData = CallbackWrapper::wrap($data, null, null, null, null, function ($stream) use (&$isEOF) { $isEOF = feof($stream); }); From edb6df219950a1aac795005ea4dcbc42332f45a9 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 13 Jun 2019 17:04:35 +0200 Subject: [PATCH 2/2] add test for overwriting in non creatable directory Signed-off-by: Robin Appelman --- .../tests/unit/Connector/Sabre/FileTest.php | 54 ++++++++++++++++--- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index edb61edc6e..3e3e80f752 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -28,13 +28,20 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre; use OC\Files\Storage\Local; +use OC\Files\Storage\Temporary; +use OC\Files\Storage\Wrapper\PermissionsMask; use OC\Files\View; +use OCA\DAV\Connector\Sabre\File; +use OCP\Constants; use OCP\Files\ForbiddenException; use OCP\Files\Storage; use OCP\IConfig; use Test\HookHelper; use OC\Files\Filesystem; use OCP\Lock\ILockingProvider; +use Test\TestCase; +use Test\Traits\MountProviderTrait; +use Test\Traits\UserTrait; /** * Class File @@ -43,7 +50,9 @@ use OCP\Lock\ILockingProvider; * * @package OCA\DAV\Tests\unit\Connector\Sabre */ -class FileTest extends \Test\TestCase { +class FileTest extends TestCase { + use MountProviderTrait; + use UserTrait; /** * @var string @@ -61,9 +70,8 @@ class FileTest extends \Test\TestCase { \OC_Hook::clear(); - $this->user = $this->getUniqueID('user_'); - $userManager = \OC::$server->getUserManager(); - $userManager->createUser($this->user, 'pass'); + $this->user = 'test_user'; + $this->createUser($this->user, 'pass'); $this->loginAsUser($this->user); @@ -79,15 +87,14 @@ class FileTest extends \Test\TestCase { } /** - * @return \PHPUnit_Framework_MockObject_MockObject | Storage + * @return \PHPUnit_Framework_MockObject_MockObject|Storage */ private function getMockStorage() { $storage = $this->getMockBuilder(Storage::class) ->disableOriginalConstructor() ->getMock(); - $storage->expects($this->any()) - ->method('getId') - ->will($this->returnValue('home::someuser')); + $storage->method('getId') + ->willReturn('home::someuser'); return $storage; } @@ -1163,4 +1170,35 @@ class FileTest extends \Test\TestCase { $file->get(); } + + public function testSimplePutNoCreatePermissions() { + $this->logout(); + + $storage = new Temporary([]); + $storage->file_put_contents('file.txt', 'old content'); + $noCreateStorage = new PermissionsMask([ + 'storage'=> $storage, + 'mask' => Constants::PERMISSION_ALL - Constants::PERMISSION_CREATE + ]); + + $this->registerMount($this->user, $noCreateStorage, '/' . $this->user . '/files/root'); + + $this->loginAsUser($this->user); + + $view = new View('/' . $this->user . '/files'); + + $info = $view->getFileInfo('root/file.txt'); + + $file = new File($view, $info); + + // beforeMethod locks + $view->lockFile('root/file.txt', ILockingProvider::LOCK_SHARED); + + $file->put($this->getStream('new content')); + + // afterMethod unlocks + $view->unlockFile('root/file.txt', ILockingProvider::LOCK_SHARED); + + $this->assertEquals('new content', $view->file_get_contents('root/file.txt')); + } }