From 5685d68bd3cdc3205b72a8c0c60be2417bf2b934 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 28 Jun 2023 15:48:55 +0200 Subject: [PATCH 1/2] add more checks to ensure mounts aren't empty Signed-off-by: Robin Appelman --- apps/files_sharing/lib/External/Manager.php | 4 ++++ apps/files_sharing/tests/External/ManagerTest.php | 13 ++++++++++--- .../Files/Config/MountProviderCollection.php | 5 +++++ lib/private/Files/Mount/Manager.php | 11 ++++++++++- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php index d2e113c8bb36e..63c2c19d25bba 100644 --- a/apps/files_sharing/lib/External/Manager.php +++ b/apps/files_sharing/lib/External/Manager.php @@ -606,6 +606,10 @@ public function removeShare($mountPoint): bool { $this->logger->error('Mount point to remove share not found', ['mountPoint' => $mountPoint]); return false; } + if (!$mountPointObj instanceof Mount) { + $this->logger->error('Mount point to remove share is not an external share, share probably doesn\'t exist', ['mountPoint' => $mountPoint]); + return false; + } $id = $mountPointObj->getStorage()->getCache()->getId(''); $mountPoint = $this->stripPath($mountPoint); diff --git a/apps/files_sharing/tests/External/ManagerTest.php b/apps/files_sharing/tests/External/ManagerTest.php index 86386144932ce..0e80479eafed5 100644 --- a/apps/files_sharing/tests/External/ManagerTest.php +++ b/apps/files_sharing/tests/External/ManagerTest.php @@ -31,8 +31,10 @@ namespace OCA\Files_Sharing\Tests\External; use OC\Federation\CloudIdManager; +use OC\Files\Mount\MountPoint; use OC\Files\SetupManagerFactory; use OC\Files\Storage\StorageFactory; +use OC\Files\Storage\Temporary; use OCA\Files_Sharing\External\Manager; use OCA\Files_Sharing\External\MountProvider; use OCA\Files_Sharing\Tests\TestCase; @@ -191,13 +193,18 @@ private function createManagerForUser($userId) { } private function setupMounts() { - $this->mountManager->clear(); + $this->clearMounts(); $mounts = $this->testMountProvider->getMountsForUser($this->user, new StorageFactory()); foreach ($mounts as $mount) { $this->mountManager->addMount($mount); } } + private function clearMounts() { + $this->mountManager->clear(); + $this->mountManager->addMount(new MountPoint(Temporary::class, '', [])); + } + public function testAddUserShare() { $this->doTestAddShare([ 'remote' => 'http://localhost', @@ -235,7 +242,7 @@ public function doTestAddShare($shareData1, $isGroup = false) { if ($isGroup) { $this->manager->expects($this->never())->method('tryOCMEndPoint'); } else { - $this->manager->expects($this->any())->method('tryOCMEndPoint') + $this->manager->method('tryOCMEndPoint') ->withConsecutive( ['http://localhost', 'token1', '2342', 'accept'], ['http://localhost', 'token3', '2342', 'decline'], @@ -415,7 +422,7 @@ public function doTestAddShare($shareData1, $isGroup = false) { $this->assertEmpty(self::invokePrivate($this->manager, 'getShares', [null]), 'Asserting all shares for the user have been deleted'); - $this->mountManager->clear(); + $this->clearMounts(); self::invokePrivate($this->manager, 'setupMounts'); $this->assertNotMount($shareData1['name']); $this->assertNotMount('{{TemporaryMountPointName#' . $shareData1['name'] . '}}'); diff --git a/lib/private/Files/Config/MountProviderCollection.php b/lib/private/Files/Config/MountProviderCollection.php index ae6481e45bbb1..d251199fd435f 100644 --- a/lib/private/Files/Config/MountProviderCollection.php +++ b/lib/private/Files/Config/MountProviderCollection.php @@ -238,6 +238,11 @@ public function getRootMounts(): array { $mounts = array_reduce($mounts, function (array $mounts, array $providerMounts) { return array_merge($mounts, $providerMounts); }, []); + + if (count($mounts) === 0) { + throw new \Exception("No root mounts provided by any provider"); + } + return $mounts; } diff --git a/lib/private/Files/Mount/Manager.php b/lib/private/Files/Mount/Manager.php index 805cce658a678..ba933d8f20e08 100644 --- a/lib/private/Files/Mount/Manager.php +++ b/lib/private/Files/Mount/Manager.php @@ -99,6 +99,15 @@ public function find(string $path): IMountPoint { return $this->pathCache[$path]; } + + + if (count($this->mounts) === 0) { + $this->setupManager->setupRoot(); + if (count($this->mounts) === 0) { + throw new \Exception("No mounts even after explicitly setting up the root mounts"); + } + } + $current = $path; while (true) { $mountPoint = $current . '/'; @@ -115,7 +124,7 @@ public function find(string $path): IMountPoint { } } - throw new NotFoundException("No mount for path " . $path . " existing mounts: " . implode(",", array_keys($this->mounts))); + throw new NotFoundException("No mount for path " . $path . " existing mounts (" . count($this->mounts) ."): " . implode(",", array_keys($this->mounts))); } /** From 1eb3293e539f995677c3e3baacff0ccb1ae3da3f Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Tue, 25 Jul 2023 15:16:54 +0200 Subject: [PATCH 2/2] setup filesystem wrappers before we mark the root as setup Signed-off-by: Robin Appelman --- lib/private/Files/SetupManager.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/private/Files/SetupManager.php b/lib/private/Files/SetupManager.php index cae0fd2f23275..564ea04ce8c98 100644 --- a/lib/private/Files/SetupManager.php +++ b/lib/private/Files/SetupManager.php @@ -337,12 +337,13 @@ public function setupRoot(): void { if ($this->rootSetup) { return; } + + $this->setupBuiltinWrappers(); + $this->rootSetup = true; $this->eventLogger->start('fs:setup:root', 'Setup root filesystem'); - $this->setupBuiltinWrappers(); - $rootMounts = $this->mountProviderCollection->getRootMounts(); foreach ($rootMounts as $rootMountProvider) { $this->mountManager->addMount($rootMountProvider);