From 9b1abd6fac90b97527c8db9d9119979c7753cb8c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 5 Apr 2022 15:27:59 +0200 Subject: [PATCH 1/4] save filesystem node in dav node Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/Directory.php | 9 ++++++++- apps/dav/lib/Connector/Sabre/File.php | 4 ++++ apps/dav/lib/Connector/Sabre/Node.php | 21 +++++++++++++++++++++ lib/private/Files/FileInfo.php | 2 +- 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index bd92b3b22a42d..54ee14fdaf306 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -38,6 +38,7 @@ use OCA\DAV\Connector\Sabre\Exception\Forbidden; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; use OCP\Files\FileInfo; +use OCP\Files\Folder; use OCP\Files\ForbiddenException; use OCP\Files\InvalidPathException; use OCP\Files\NotPermittedException; @@ -144,7 +145,9 @@ public function createFile($name, $data = null) { $info = $this->fileView->getFileInfo($this->path . '/' . $name); if (!$info) { // use a dummy FileInfo which is acceptable here since it will be refreshed after the put is complete - $info = new \OC\Files\FileInfo($path, null, null, [], null); + $info = new \OC\Files\FileInfo($path, null, null, [ + 'type' => FileInfo::TYPE_FILE + ], null); } $node = new \OCA\DAV\Connector\Sabre\File($this->fileView, $info); @@ -474,4 +477,8 @@ public function copyInto($targetName, $sourcePath, INode $sourceNode) { return false; } + + public function getNode(): Folder { + return $this->node; + } } diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 70dffbaaaed13..a46ca372be772 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -753,4 +753,8 @@ protected function header($string) { public function hash(string $type) { return $this->fileView->hash($type, $this->path); } + + public function getNode(): \OCP\Files\File { + return $this->node; + } } diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index 79b4db0e32773..79f1653c692cd 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -36,9 +36,12 @@ namespace OCA\DAV\Connector\Sabre; use OC\Files\Mount\MoveableMount; +use OC\Files\Node\File; +use OC\Files\Node\Folder; use OC\Files\View; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; use OCP\Files\FileInfo; +use OCP\Files\IRootFolder; use OCP\Files\StorageNotAvailableException; use OCP\Share\IShare; use OCP\Share\Exceptions\ShareNotFound; @@ -75,6 +78,8 @@ abstract class Node implements \Sabre\DAV\INode { */ protected $shareManager; + protected \OC\Files\Node\Node $node; + /** * Sets up the node, expects a full path name * @@ -91,10 +96,22 @@ public function __construct(View $view, FileInfo $info, IManager $shareManager = } else { $this->shareManager = \OC::$server->getShareManager(); } + $root = \OC::$server->get(IRootFolder::class); + if ($info->getType()=== FileInfo::TYPE_FOLDER) { + $this->node = new Folder($root, $view, $this->path, $info); + } else { + $this->node = new File($root, $view, $this->path, $info); + } } protected function refreshInfo() { $this->info = $this->fileView->getFileInfo($this->path); + $root = \OC::$server->get(IRootFolder::class); + if ($this->info->getType() === FileInfo::TYPE_FOLDER) { + $this->node = new Folder($root, $this->fileView, $this->path, $this->info); + } else { + $this->node = new File($root, $this->fileView, $this->path, $this->info); + } } /** @@ -403,6 +420,10 @@ public function getFileInfo() { return $this->info; } + public function getNode(): \OCP\Files\Node { + return $this->node; + } + protected function sanitizeMtime($mtimeFromRequest) { return MtimeSanitizer::sanitizeMtime($mtimeFromRequest); } diff --git a/lib/private/Files/FileInfo.php b/lib/private/Files/FileInfo.php index 2f361fc051dba..6389544184fca 100644 --- a/lib/private/Files/FileInfo.php +++ b/lib/private/Files/FileInfo.php @@ -254,7 +254,7 @@ public function getPermissions() { */ public function getType() { if (!isset($this->data['type'])) { - $this->data['type'] = ($this->getMimetype() === 'httpd/unix-directory') ? self::TYPE_FOLDER : self::TYPE_FILE; + $this->data['type'] = ($this->getMimetype() === self::MIMETYPE_FOLDER) ? self::TYPE_FOLDER : self::TYPE_FILE; } return $this->data['type']; } From 151c80039751bbe74042d7f6f5d58e9f115064e4 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 5 Apr 2022 15:29:49 +0200 Subject: [PATCH 2/4] allow reusing known folder info when getting directory contents Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/Directory.php | 4 ++-- apps/dav/lib/Connector/Sabre/Node.php | 14 +++++++++----- lib/private/Files/Node/Folder.php | 4 ++-- lib/private/Files/View.php | 21 ++++++++++++++------- tests/lib/Files/Node/FolderTest.php | 2 ++ 5 files changed, 29 insertions(+), 16 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index 54ee14fdaf306..8b616b0cb8a1b 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -236,7 +236,7 @@ public function getChild($name, $info = null) { throw new \Sabre\DAV\Exception\NotFound('File with name ' . $path . ' could not be located'); } - if ($info['mimetype'] === 'httpd/unix-directory') { + if ($info->getMimeType() === FileInfo::MIMETYPE_FOLDER) { $node = new \OCA\DAV\Connector\Sabre\Directory($this->fileView, $info, $this->tree, $this->shareManager); } else { $node = new \OCA\DAV\Connector\Sabre\File($this->fileView, $info, $this->shareManager); @@ -264,7 +264,7 @@ public function getChildren() { // the caller believe that the collection itself does not exist throw new Forbidden('No read permissions'); } - $folderContent = $this->fileView->getDirectoryContent($this->path); + $folderContent = $this->getNode()->getDirectoryListing(); } catch (LockedException $e) { throw new Locked(); } diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index 79f1653c692cd..e4517068f4248 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -78,7 +78,7 @@ abstract class Node implements \Sabre\DAV\INode { */ protected $shareManager; - protected \OC\Files\Node\Node $node; + protected \OCP\Files\Node $node; /** * Sets up the node, expects a full path name @@ -96,11 +96,15 @@ public function __construct(View $view, FileInfo $info, IManager $shareManager = } else { $this->shareManager = \OC::$server->getShareManager(); } - $root = \OC::$server->get(IRootFolder::class); - if ($info->getType()=== FileInfo::TYPE_FOLDER) { - $this->node = new Folder($root, $view, $this->path, $info); + if ($info instanceof Folder || $info instanceof File) { + $this->node = $info; } else { - $this->node = new File($root, $view, $this->path, $info); + $root = \OC::$server->get(IRootFolder::class); + if ($info->getType() === FileInfo::TYPE_FOLDER) { + $this->node = new Folder($root, $view, $this->path, $info); + } else { + $this->node = new File($root, $view, $this->path, $info); + } } } diff --git a/lib/private/Files/Node/Folder.php b/lib/private/Files/Node/Folder.php index d058805b20ec6..9c15f0edf416f 100644 --- a/lib/private/Files/Node/Folder.php +++ b/lib/private/Files/Node/Folder.php @@ -97,10 +97,10 @@ public function isSubNode($node) { * @throws \OCP\Files\NotFoundException */ public function getDirectoryListing() { - $folderContent = $this->view->getDirectoryContent($this->path); + $folderContent = $this->view->getDirectoryContent($this->path, '', $this->getFileInfo()); return array_map(function (FileInfo $info) { - if ($info->getMimetype() === 'httpd/unix-directory') { + if ($info->getMimetype() === FileInfo::MIMETYPE_FOLDER) { return new Folder($this->root, $this->view, $info->getPath(), $info); } else { return new File($this->root, $this->view, $info->getPath(), $info); diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index e23dd4312aad8..eef87cc65f4e0 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -1431,7 +1431,7 @@ public function getFileInfo($path, $includeMountPoints = true) { * @param string $mimetype_filter limit returned content to this mimetype or mimepart * @return FileInfo[] */ - public function getDirectoryContent($directory, $mimetype_filter = '') { + public function getDirectoryContent($directory, $mimetype_filter = '', \OCP\Files\FileInfo $directoryInfo = null) { $this->assertPathLength($directory); if (!Filesystem::isValidPath($directory)) { return []; @@ -1449,14 +1449,21 @@ public function getDirectoryContent($directory, $mimetype_filter = '') { $cache = $storage->getCache($internalPath); $user = \OC_User::getUser(); - $data = $this->getCacheEntry($storage, $internalPath, $directory); + if (!$directoryInfo) { + $data = $this->getCacheEntry($storage, $internalPath, $directory); + if (!$data instanceof ICacheEntry || !isset($data['fileid'])) { + return []; + } + } else { + $data = $directoryInfo; + } - if (!$data instanceof ICacheEntry || !isset($data['fileid']) || !($data->getPermissions() & Constants::PERMISSION_READ)) { - return []; - } + if (!($data->getPermissions() & Constants::PERMISSION_READ)) { + return []; + } - $folderId = $data['fileid']; - $contents = $cache->getFolderContentsById($folderId); //TODO: mimetype_filter + $folderId = $data->getId(); + $contents = $cache->getFolderContentsById($folderId); //TODO: mimetype_filter $sharingDisabled = \OCP\Util::isSharingDisabledForUser(); diff --git a/tests/lib/Files/Node/FolderTest.php b/tests/lib/Files/Node/FolderTest.php index 05f546874ef57..4cda92b6e838b 100644 --- a/tests/lib/Files/Node/FolderTest.php +++ b/tests/lib/Files/Node/FolderTest.php @@ -78,6 +78,8 @@ public function testGetDirectoryContent() { new FileInfo('/bar/foo/asd', null, 'foo/asd', ['fileid' => 2, 'path' => '/bar/foo/asd', 'name' => 'asd', 'size' => 100, 'mtime' => 50, 'mimetype' => 'text/plain'], null), new FileInfo('/bar/foo/qwerty', null, 'foo/qwerty', ['fileid' => 3, 'path' => '/bar/foo/qwerty', 'name' => 'qwerty', 'size' => 200, 'mtime' => 55, 'mimetype' => 'httpd/unix-directory'], null), ]); + $view->method('getFileInfo') + ->willReturn($this->createMock(FileInfo::class)); $node = new Folder($root, $view, '/bar/foo'); $children = $node->getDirectoryListing(); From ae7205f550d12d713b0b079eb0889c8f3bafb42b Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 5 Apr 2022 15:30:10 +0200 Subject: [PATCH 3/4] use existing node in SharesPlugin Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/SharesPlugin.php | 4 +- .../unit/Connector/Sabre/DirectoryTest.php | 36 ++++++---- .../tests/unit/Connector/Sabre/FileTest.php | 67 +++++++++++++------ .../unit/Connector/Sabre/ObjectTreeTest.php | 9 +-- .../unit/Connector/Sabre/SharesPluginTest.php | 15 +++-- 5 files changed, 82 insertions(+), 49 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index d482aa4b51091..bbfbfc04a72eb 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -165,7 +165,7 @@ private function getShares(DavNode $sabreNode): array { // if we already cached the folder this file is in we know there are no shares for this file if (array_search($parentPath, $this->cachedFolders) === false) { try { - $node = $this->userFolder->get($sabreNode->getPath()); + $node = $sabreNode->getNode(); } catch (NotFoundException $e) { return []; } @@ -202,7 +202,7 @@ public function handleGetProperties( ) ) { try { - $folderNode = $this->userFolder->get($sabreNode->getPath()); + $folderNode = $sabreNode->getNode(); } catch (NotFoundException $e) { // If the folder can't be properly found just return return; diff --git a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php index c88d2302bec09..e8297c2ac6661 100644 --- a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php @@ -29,6 +29,7 @@ namespace OCA\DAV\Tests\Unit\Connector\Sabre; use OC\Files\FileInfo; +use OC\Files\Node\Node; use OC\Files\Storage\Wrapper\Quota; use OCA\DAV\Connector\Sabre\Directory; use OCP\Files\ForbiddenException; @@ -82,9 +83,12 @@ protected function setUp(): void { $this->view = $this->createMock('OC\Files\View'); $this->info = $this->createMock('OC\Files\FileInfo'); - $this->info->expects($this->any()) - ->method('isReadable') + $this->info->method('isReadable') ->willReturn(true); + $this->info->method('getType') + ->willReturn(Node::TYPE_FOLDER); + $this->info->method('getName') + ->willReturn("folder"); } private function getDir($path = '/') { @@ -186,17 +190,17 @@ public function testGetChildren() { $info2 = $this->getMockBuilder(FileInfo::class) ->disableOriginalConstructor() ->getMock(); - $info1->expects($this->any()) - ->method('getName') + $info1->method('getName') ->willReturn('first'); - $info1->expects($this->any()) - ->method('getEtag') + $info1->method('getPath') + ->willReturn('folder/first'); + $info1->method('getEtag') ->willReturn('abc'); - $info2->expects($this->any()) - ->method('getName') + $info2->method('getName') ->willReturn('second'); - $info2->expects($this->any()) - ->method('getEtag') + $info2->method('getPath') + ->willReturn('folder/second'); + $info2->method('getEtag') ->willReturn('def'); $this->view->expects($this->once()) @@ -403,8 +407,12 @@ public function moveSuccessProvider() { private function moveTest($source, $destination, $updatables, $deletables) { $view = new TestViewDirectory($updatables, $deletables); - $sourceInfo = new FileInfo($source, null, null, [], null); - $targetInfo = new FileInfo(dirname($destination), null, null, [], null); + $sourceInfo = new FileInfo($source, null, null, [ + 'type' => FileInfo::TYPE_FOLDER, + ], null); + $targetInfo = new FileInfo(dirname($destination), null, null, [ + 'type' => FileInfo::TYPE_FOLDER, + ], null); $sourceNode = new Directory($view, $sourceInfo); $targetNode = $this->getMockBuilder(Directory::class) @@ -429,8 +437,8 @@ public function testFailingMove() { $view = new TestViewDirectory($updatables, $deletables); - $sourceInfo = new FileInfo($source, null, null, [], null); - $targetInfo = new FileInfo(dirname($destination), null, null, [], null); + $sourceInfo = new FileInfo($source, null, null, ['type' => FileInfo::TYPE_FOLDER], null); + $targetInfo = new FileInfo(dirname($destination), null, null, ['type' => FileInfo::TYPE_FOLDER], null); $sourceNode = new Directory($view, $sourceInfo); $targetNode = $this->getMockBuilder(Directory::class) diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index d12a86f6e8dee..9870a62845c72 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -37,6 +37,7 @@ use OC\Files\View; use OCA\DAV\Connector\Sabre\File; use OCP\Constants; +use OCP\Files\FileInfo; use OCP\Files\ForbiddenException; use OCP\Files\Storage; use OCP\IConfig; @@ -211,7 +212,8 @@ function ($path) use ($storage) { ->willReturnArgument(0); $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -272,7 +274,8 @@ function ($path) use ($storage) { $_SERVER['HTTP_OC_CHUNKED'] = true; $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -282,7 +285,8 @@ function ($path) use ($storage) { $file->releaseLock(ILockingProvider::LOCK_SHARED); $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -326,7 +330,10 @@ private function doPut($path, $viewRoot = null, Request $request = null) { $viewRoot . '/' . ltrim($path, '/'), $this->getMockStorage(), null, - ['permissions' => \OCP\Constants::PERMISSION_ALL], + [ + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, + ], null ); @@ -690,7 +697,8 @@ public function testSimplePutFailsSizeCheck() { $_SERVER['REQUEST_METHOD'] = 'PUT'; $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -723,7 +731,8 @@ public function testSimplePutFailsMoveFromStorage() { $view->lockFile('/test.txt', ILockingProvider::LOCK_EXCLUSIVE); $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -758,7 +767,8 @@ public function testChunkedPutFailsFinalRename() { $_SERVER['HTTP_OC_CHUNKED'] = true; $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); $file->acquireLock(ILockingProvider::LOCK_SHARED); @@ -766,7 +776,8 @@ public function testChunkedPutFailsFinalRename() { $file->releaseLock(ILockingProvider::LOCK_SHARED); $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -797,7 +808,8 @@ public function testSimplePutInvalidChars() { ->willReturnArgument(0); $info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -836,7 +848,8 @@ public function testSetNameInvalidChars() { ->willReturnArgument(0); $info = new \OC\Files\FileInfo('/*', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); $file->setName('/super*star.txt'); @@ -863,7 +876,8 @@ public function testUploadAbort() { $_SERVER['REQUEST_METHOD'] = 'PUT'; $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -897,7 +911,8 @@ public function testDeleteWhenAllowed() { ->willReturn(true); $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -915,7 +930,8 @@ public function testDeleteThrowsWhenDeletionNotAllowed() { ->getMock(); $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => 0 + 'permissions' => 0, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -938,7 +954,8 @@ public function testDeleteThrowsWhenDeletionFailed() { ->willReturn(false); $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -961,7 +978,8 @@ public function testDeleteThrowsWhenDeletionThrows() { ->willThrowException(new ForbiddenException('', true)); $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -997,7 +1015,10 @@ public function testPutLocking() { '/' . $this->user . '/files/' . $path, $this->getMockStorage(), null, - ['permissions' => \OCP\Constants::PERMISSION_ALL], + [ + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, + ], null ); @@ -1129,7 +1150,8 @@ public function testGetFopenFails() { ->willReturn(false); $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -1149,7 +1171,8 @@ public function testGetFopenThrows() { ->willThrowException(new ForbiddenException('', true)); $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -1168,7 +1191,8 @@ public function testGetThrowsIfNoPermission() { ->method('fopen'); $info = new \OC\Files\FileInfo('/test.txt', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_CREATE // no read perm + 'permissions' => \OCP\Constants::PERMISSION_CREATE, // no read perm + 'type' => FileInfo::TYPE_FOLDER, ], null); $file = new \OCA\DAV\Connector\Sabre\File($view, $info); @@ -1215,7 +1239,10 @@ public function testPutLockExpired() { '/' . $this->user . '/files/' . $path, $this->getMockStorage(), null, - ['permissions' => \OCP\Constants::PERMISSION_ALL], + [ + 'permissions' => \OCP\Constants::PERMISSION_ALL, + 'type' => FileInfo::TYPE_FOLDER, + ], null ); diff --git a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php index 7416cf7a3f7ae..5f516cec11390 100644 --- a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php @@ -167,17 +167,14 @@ public function testGetNodeForPath( $fileInfo = $this->getMockBuilder(FileInfo::class) ->disableOriginalConstructor() ->getMock(); - $fileInfo->expects($this->once()) - ->method('getType') + $fileInfo->method('getType') ->willReturn($type); - $fileInfo->expects($this->once()) - ->method('getName') + $fileInfo->method('getName') ->willReturn($outputFileName); $fileInfo->method('getStorage') ->willReturn($this->createMock(\OC\Files\Storage\Common::class)); - $view->expects($this->once()) - ->method('getFileInfo') + $view->method('getFileInfo') ->with($fileInfoQueryPath) ->willReturn($fileInfo); diff --git a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php index f528310e54c30..a81226ccb5e11 100644 --- a/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/SharesPluginTest.php @@ -111,9 +111,7 @@ public function testGetProperties($shareTypes) { ->disableOriginalConstructor() ->getMock(); - $this->userFolder->expects($this->once()) - ->method('get') - ->with('/subdir') + $sabreNode->method('getNode') ->willReturn($node); $this->shareManager->expects($this->any()) @@ -180,16 +178,19 @@ public function testPreloadThenGetProperties($shareTypes) { $node = $this->createMock(Folder::class); $node->method('getId') ->willReturn(123); - $node1 = $this->createMock(File::class); + $node1 = $this->createMock(\OC\Files\Node\File::class); $node1->method('getId') ->willReturn(111); - $node2 = $this->createMock(File::class); + $node2 = $this->createMock(\OC\Files\Node\File::class); $node2->method('getId') ->willReturn(222); - $this->userFolder->method('get') - ->with('/subdir') + $sabreNode->method('getNode') ->willReturn($node); + $sabreNode1->method('getNode') + ->willReturn($node1); + $sabreNode2->method('getNode') + ->willReturn($node2); $dummyShares = array_map(function ($type) { $share = $this->getMockBuilder(IShare::class)->getMock(); From 5e69f98c16b551327edec2788156b699ee1e38d5 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 6 Apr 2022 15:46:38 +0200 Subject: [PATCH 4/4] sh Signed-off-by: Robin Appelman --- apps/dav/lib/Connector/Sabre/SharesPlugin.php | 13 +--------- lib/private/Files/View.php | 24 +++++++++---------- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/SharesPlugin.php b/apps/dav/lib/Connector/Sabre/SharesPlugin.php index bbfbfc04a72eb..57c91e05a8c2a 100644 --- a/apps/dav/lib/Connector/Sabre/SharesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/SharesPlugin.php @@ -201,18 +201,7 @@ public function handleGetProperties( !is_null($propFind->getStatus(self::SHAREES_PROPERTYNAME)) ) ) { - try { - $folderNode = $sabreNode->getNode(); - } catch (NotFoundException $e) { - // If the folder can't be properly found just return - return; - } - - if (!($folderNode instanceof Folder)) { - // Safety check - return; - } - + $folderNode = $sabreNode->getNode(); $this->cachedFolders[] = $sabreNode->getPath(); $childShares = $this->getSharesFolder($folderNode); foreach ($childShares as $id => $shares) { diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index eef87cc65f4e0..30dc5518be873 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -1449,21 +1449,21 @@ public function getDirectoryContent($directory, $mimetype_filter = '', \OCP\File $cache = $storage->getCache($internalPath); $user = \OC_User::getUser(); - if (!$directoryInfo) { - $data = $this->getCacheEntry($storage, $internalPath, $directory); - if (!$data instanceof ICacheEntry || !isset($data['fileid'])) { - return []; - } - } else { - $data = $directoryInfo; - } - - if (!($data->getPermissions() & Constants::PERMISSION_READ)) { + if (!$directoryInfo) { + $data = $this->getCacheEntry($storage, $internalPath, $directory); + if (!$data instanceof ICacheEntry || !isset($data['fileid'])) { return []; } + } else { + $data = $directoryInfo; + } + + if (!($data->getPermissions() & Constants::PERMISSION_READ)) { + return []; + } - $folderId = $data->getId(); - $contents = $cache->getFolderContentsById($folderId); //TODO: mimetype_filter + $folderId = $data->getId(); + $contents = $cache->getFolderContentsById($folderId); //TODO: mimetype_filter $sharingDisabled = \OCP\Util::isSharingDisabledForUser();