diff --git a/apps/files_sharing/composer/composer/autoload_classmap.php b/apps/files_sharing/composer/composer/autoload_classmap.php index f087686c4ca50..6de418dce0101 100644 --- a/apps/files_sharing/composer/composer/autoload_classmap.php +++ b/apps/files_sharing/composer/composer/autoload_classmap.php @@ -96,6 +96,7 @@ 'OCA\\Files_Sharing\\Settings\\Personal' => $baseDir . '/../lib/Settings/Personal.php', 'OCA\\Files_Sharing\\ShareBackend\\File' => $baseDir . '/../lib/ShareBackend/File.php', 'OCA\\Files_Sharing\\ShareBackend\\Folder' => $baseDir . '/../lib/ShareBackend/Folder.php', + 'OCA\\Files_Sharing\\ShareTargetValidator' => $baseDir . '/../lib/ShareTargetValidator.php', 'OCA\\Files_Sharing\\SharedMount' => $baseDir . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => $baseDir . '/../lib/SharedStorage.php', 'OCA\\Files_Sharing\\SharesReminderJob' => $baseDir . '/../lib/SharesReminderJob.php', diff --git a/apps/files_sharing/composer/composer/autoload_static.php b/apps/files_sharing/composer/composer/autoload_static.php index c39bd4d677a83..fb0da77fb4095 100644 --- a/apps/files_sharing/composer/composer/autoload_static.php +++ b/apps/files_sharing/composer/composer/autoload_static.php @@ -111,6 +111,7 @@ class ComposerStaticInitFiles_Sharing 'OCA\\Files_Sharing\\Settings\\Personal' => __DIR__ . '/..' . '/../lib/Settings/Personal.php', 'OCA\\Files_Sharing\\ShareBackend\\File' => __DIR__ . '/..' . '/../lib/ShareBackend/File.php', 'OCA\\Files_Sharing\\ShareBackend\\Folder' => __DIR__ . '/..' . '/../lib/ShareBackend/Folder.php', + 'OCA\\Files_Sharing\\ShareTargetValidator' => __DIR__ . '/..' . '/../lib/ShareTargetValidator.php', 'OCA\\Files_Sharing\\SharedMount' => __DIR__ . '/..' . '/../lib/SharedMount.php', 'OCA\\Files_Sharing\\SharedStorage' => __DIR__ . '/..' . '/../lib/SharedStorage.php', 'OCA\\Files_Sharing\\SharesReminderJob' => __DIR__ . '/..' . '/../lib/SharesReminderJob.php', diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index cc7ab6e827ce6..005bf9caf4212 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -11,7 +11,6 @@ use InvalidArgumentException; use OC\Files\View; use OCA\Files_Sharing\Event\ShareMountedEvent; -use OCP\Cache\CappedMemoryCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Config\IMountProvider; use OCP\Files\Config\IPartialMountProvider; @@ -29,6 +28,7 @@ use function count; class MountProvider implements IMountProvider, IPartialMountProvider { + /** * @param IConfig $config * @param IManager $shareManager @@ -41,6 +41,7 @@ public function __construct( protected IEventDispatcher $eventDispatcher, protected ICacheFactory $cacheFactory, protected IMountManager $mountManager, + protected ShareTargetValidator $shareTargetValidator, ) { } @@ -270,8 +271,6 @@ private function getMountsFromSuperShares( $view = new View('/' . $userId . '/files'); $ownerViews = []; $sharingDisabledForUser = $this->shareManager->sharingDisabledForUser($userId); - /** @var CappedMemoryCache $folderExistCache */ - $foldersExistCache = new CappedMemoryCache(); $validShareCache = $this->cacheFactory->createLocal('share-valid-mountpoint-max'); $maxValidatedShare = $validShareCache->get($userId) ?? 0; @@ -292,10 +291,17 @@ private function getMountsFromSuperShares( if (!isset($ownerViews[$owner])) { $ownerViews[$owner] = new View('/' . $owner . '/files'); } + $shareId = (int)$parentShare->getId(); + $absMountPoint = '/' . $user->getUID() . '/files/' . trim($parentShare->getTarget(), '/') . '/'; + + // after the mountpoint is verified for the first time, only new mountpoints (e.g. groupfolders can overwrite the target) + if ($shareId > $maxValidatedShare || isset($allMounts[$absMountPoint])) { + $this->shareTargetValidator->verifyMountPoint($user, $parentShare, $allMounts, $groupedShares); + } + $mount = new SharedMount( '\OCA\Files_Sharing\SharedStorage', - $allMounts, [ 'user' => $userId, // parent share @@ -307,10 +313,8 @@ private function getMountsFromSuperShares( ], $loader, $view, - $foldersExistCache, $this->eventDispatcher, $user, - $shareId <= $maxValidatedShare, ); $newMaxValidatedShare = max($shareId, $newMaxValidatedShare); diff --git a/apps/files_sharing/lib/ShareTargetValidator.php b/apps/files_sharing/lib/ShareTargetValidator.php new file mode 100644 index 0000000000000..27c79962a324c --- /dev/null +++ b/apps/files_sharing/lib/ShareTargetValidator.php @@ -0,0 +1,163 @@ +folderExistsCache = new CappedMemoryCache(); + } + + private function getViewForUser(IUser $user): View { + /** + * @psalm-suppress InternalClass + * @psalm-suppress InternalMethod + */ + return new View('/' . $user->getUID() . '/files'); + } + + /** + * check if the parent folder exists otherwise move the mount point up + * + * @param array $allCachedMounts Other mounts for the user, indexed by path + * @param IShare[] $childShares + * @return string + */ + public function verifyMountPoint( + IUser $user, + IShare &$share, + array $allCachedMounts, + array $childShares, + ): string { + $mountPoint = basename($share->getTarget()); + $parent = dirname($share->getTarget()); + + $recipientView = $this->getViewForUser($user); + $event = new VerifyMountPointEvent($share, $recipientView, $parent); + $this->eventDispatcher->dispatchTyped($event); + $parent = $event->getParent(); + + /** @psalm-suppress InternalMethod */ + $absoluteParent = $recipientView->getAbsolutePath($parent); + $this->setupManager->setupForPath($absoluteParent); + $parentMount = $this->mountManager->find($absoluteParent); + + $cached = $this->folderExistsCache->get($parent); + if ($cached !== null) { + $parentExists = $cached; + } else { + $parentCache = $parentMount->getStorage()->getCache(); + $parentExists = $parentCache->inCache($parentMount->getInternalPath($absoluteParent)); + $this->folderExistsCache->set($parent, $parentExists); + } + if (!$parentExists) { + $parent = Helper::getShareFolder($recipientView, $user->getUID()); + /** @psalm-suppress InternalMethod */ + $absoluteParent = $recipientView->getAbsolutePath($parent); + } + + $newAbsoluteMountPoint = $this->generateUniqueTarget( + $share, + Filesystem::normalizePath($absoluteParent . '/' . $mountPoint), + $parentMount, + $allCachedMounts, + ); + + /** @psalm-suppress InternalMethod */ + $newMountPoint = $recipientView->getRelativePath($newAbsoluteMountPoint); + if ($newMountPoint === null) { + return $share->getTarget(); + } + + if ($newMountPoint !== $share->getTarget()) { + $this->updateFileTarget($user, $newMountPoint, $share, $childShares); + } + + return $newMountPoint; + } + + + /** + * @param IMountPoint[] $allCachedMounts + */ + private function generateUniqueTarget( + IShare $share, + string $absolutePath, + IMountPoint $parentMount, + array $allCachedMounts, + ): string { + $pathInfo = pathinfo($absolutePath); + $ext = isset($pathInfo['extension']) ? '.' . $pathInfo['extension'] : ''; + $name = $pathInfo['filename']; + $dir = $pathInfo['dirname']; + + $i = 2; + $parentCache = $parentMount->getStorage()->getCache(); + $internalPath = $parentMount->getInternalPath($absolutePath); + while ($parentCache->inCache($internalPath) || $this->hasConflictingMount($share, $allCachedMounts, $absolutePath)) { + $absolutePath = Filesystem::normalizePath($dir . '/' . $name . ' (' . $i . ')' . $ext); + $internalPath = $parentMount->getInternalPath($absolutePath); + $i++; + } + + return $absolutePath; + } + + /** + * @param IMountPoint[] $allCachedMounts + */ + private function hasConflictingMount(IShare $share, array $allCachedMounts, string $absolutePath): bool { + if (!isset($allCachedMounts[$absolutePath . '/'])) { + return false; + } + + $mount = $allCachedMounts[$absolutePath . '/']; + if ($mount instanceof SharedMount && $mount->getShare()->getNodeId() === $share->getNodeId()) { + // "conflicting" mount is a mount for the current share + return false; + } + + return true; + } + + /** + * update fileTarget in the database if the mount point changed + * + * @param IShare[] $childShares + */ + private function updateFileTarget(IUser $user, string $newPath, IShare &$share, array $childShares) { + $share->setTarget($newPath); + + foreach ($childShares as $tmpShare) { + $tmpShare->setTarget($newPath); + $this->shareManager->moveShare($tmpShare, $user->getUID()); + } + } +} diff --git a/apps/files_sharing/lib/SharedMount.php b/apps/files_sharing/lib/SharedMount.php index 7dc95533d7497..4dff3bcf3b75b 100644 --- a/apps/files_sharing/lib/SharedMount.php +++ b/apps/files_sharing/lib/SharedMount.php @@ -13,14 +13,12 @@ use OC\Files\Mount\MoveableMount; use OC\Files\View; use OCA\Files_Sharing\Exceptions\BrokenPath; -use OCP\Cache\CappedMemoryCache; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\InvalidateMountCacheEvent; use OCP\Files\Storage\IStorageFactory; use OCP\IDBConnection; use OCP\IUser; use OCP\Server; -use OCP\Share\Events\VerifyMountPointEvent; use OCP\Share\IShare; use Psr\Log\LoggerInterface; @@ -41,73 +39,20 @@ class SharedMount extends MountPoint implements MoveableMount, ISharedMountPoint public function __construct( $storage, - array $mountpoints, $arguments, IStorageFactory $loader, private View $recipientView, - CappedMemoryCache $folderExistCache, private IEventDispatcher $eventDispatcher, private IUser $user, - bool $alreadyVerified, ) { $this->superShare = $arguments['superShare']; $this->groupedShares = $arguments['groupedShares']; $absMountPoint = '/' . $user->getUID() . '/files/' . trim($this->superShare->getTarget(), '/') . '/'; - // after the mountpoint is verified for the first time, only new mountpoints (e.g. groupfolders can overwrite the target) - if (!$alreadyVerified || isset($mountpoints[$absMountPoint])) { - $newMountPoint = $this->verifyMountPoint($this->superShare, $mountpoints, $folderExistCache); - $absMountPoint = '/' . $user->getUID() . '/files/' . trim($newMountPoint, '/') . '/'; - } - parent::__construct($storage, $absMountPoint, $arguments, $loader, null, null, MountProvider::class); } - /** - * check if the parent folder exists otherwise move the mount point up - * - * @param IShare $share - * @param SharedMount[] $mountpoints - * @param CappedMemoryCache $folderExistCache - * @return string - */ - private function verifyMountPoint( - IShare $share, - array $mountpoints, - CappedMemoryCache $folderExistCache, - ) { - $mountPoint = basename($share->getTarget()); - $parent = dirname($share->getTarget()); - - $event = new VerifyMountPointEvent($share, $this->recipientView, $parent); - $this->eventDispatcher->dispatchTyped($event); - $parent = $event->getParent(); - - $cached = $folderExistCache->get($parent); - if ($cached) { - $parentExists = $cached; - } else { - $parentExists = $this->recipientView->is_dir($parent); - $folderExistCache->set($parent, $parentExists); - } - if (!$parentExists) { - $parent = Helper::getShareFolder($this->recipientView, $this->user->getUID()); - } - - $newMountPoint = $this->generateUniqueTarget( - Filesystem::normalizePath($parent . '/' . $mountPoint), - $this->recipientView, - $mountpoints - ); - - if ($newMountPoint !== $share->getTarget()) { - $this->updateFileTarget($newMountPoint, $share); - } - - return $newMountPoint; - } - /** * update fileTarget in the database if the mount point changed * @@ -126,30 +71,6 @@ private function updateFileTarget($newPath, &$share) { $this->eventDispatcher->dispatchTyped(new InvalidateMountCacheEvent($this->user)); } - - /** - * @param string $path - * @param View $view - * @param SharedMount[] $mountpoints - * @return mixed - */ - private function generateUniqueTarget($path, $view, array $mountpoints) { - $pathinfo = pathinfo($path); - $ext = isset($pathinfo['extension']) ? '.' . $pathinfo['extension'] : ''; - $name = $pathinfo['filename']; - $dir = $pathinfo['dirname']; - - $i = 2; - $absolutePath = $this->recipientView->getAbsolutePath($path) . '/'; - while ($view->file_exists($path) || isset($mountpoints[$absolutePath])) { - $path = Filesystem::normalizePath($dir . '/' . $name . ' (' . $i . ')' . $ext); - $absolutePath = $this->recipientView->getAbsolutePath($path) . '/'; - $i++; - } - - return $path; - } - /** * Format a path to be relative to the /user/files/ directory * diff --git a/apps/files_sharing/tests/MountProviderTest.php b/apps/files_sharing/tests/MountProviderTest.php index 82c503f4232af..090e2e353dd6b 100644 --- a/apps/files_sharing/tests/MountProviderTest.php +++ b/apps/files_sharing/tests/MountProviderTest.php @@ -10,6 +10,7 @@ use OC\Share20\Share; use OCA\Files_Sharing\MountProvider; use OCA\Files_Sharing\SharedMount; +use OCA\Files_Sharing\ShareTargetValidator; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Cache\ICacheEntry; use OCP\Files\Config\ICachedMountInfo; @@ -54,8 +55,9 @@ protected function setUp(): void { $cacheFactory = $this->createMock(ICacheFactory::class); $cacheFactory->method('createLocal')->willReturn($this->cache); $mountManager = $this->createMock(IMountManager::class); + $shareValidator = $this->createMock(ShareTargetValidator::class); - $this->provider = new MountProvider($this->config, $this->shareManager, $this->logger, $eventDispatcher, $cacheFactory, $mountManager); + $this->provider = new MountProvider($this->config, $this->shareManager, $this->logger, $eventDispatcher, $cacheFactory, $mountManager, $shareValidator); } private function makeMockShareAttributes($attrs) { diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 84f4b71a65515..371f575b9a25e 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1663,12 +1663,6 @@ - - - - - -