From f62722e970e5681f5c577ffb1dc76bea566fbec0 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Thu, 2 Oct 2025 10:35:01 +0200 Subject: [PATCH 1/4] fix(Share20\Manager): Explicitly setup the filesystem for the user Signed-off-by: provokateurin --- lib/private/Share20/Manager.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 4abc3a3f54cad..e39ae781bd097 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -9,6 +9,7 @@ use OC\Core\AppInfo\ConfigLexicon; use OC\Files\Mount\MoveableMount; +use OC\Files\SetupManager; use OC\KnownUser\KnownUserService; use OC\Share20\Exception\ProviderException; use OCA\Files_Sharing\AppInfo\Application; @@ -39,6 +40,7 @@ use OCP\Security\IHasher; use OCP\Security\ISecureRandom; use OCP\Security\PasswordContext; +use OCP\Server; use OCP\Share; use OCP\Share\Events\BeforeShareDeletedEvent; use OCP\Share\Events\ShareAcceptedEvent; @@ -91,6 +93,12 @@ public function __construct( // The constructor of LegacyHooks registers the listeners of share events // do not remove if those are not properly migrated $this->legacyHooks = new LegacyHooks($this->dispatcher); + + $user = $this->userSession->getUser(); + if ($user !== null) { + // The VerifyMountPointEvent is required for Talk and Deck, so an explicit FS setup is required for sharing. + Server::get(SetupManager::class)->setupForUser($user); + } } /** From 996c974235c6453a6e3d9ef1c46209e223c423a3 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 27 May 2025 15:18:25 +0200 Subject: [PATCH 2/4] perf(base): Stop setting up the FS for every basic auth request Signed-off-by: provokateurin --- apps/files_sharing/tests/TestCase.php | 1 + lib/base.php | 17 ----------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/apps/files_sharing/tests/TestCase.php b/apps/files_sharing/tests/TestCase.php index 9a6935e46b634..a538e3b3baa10 100644 --- a/apps/files_sharing/tests/TestCase.php +++ b/apps/files_sharing/tests/TestCase.php @@ -196,6 +196,7 @@ protected function loginHelper($user, $create = false, $password = false) { Server::get(IUserSession::class)->setUser(null); Filesystem::tearDown(); Server::get(IUserSession::class)->login($user, $password); + Filesystem::initMountPoints($user); \OC::$server->getUserFolder($user); \OC_Util::setupFS($user); diff --git a/lib/base.php b/lib/base.php index b890cdb6dd74f..9ba767c917c14 100644 --- a/lib/base.php +++ b/lib/base.php @@ -918,23 +918,6 @@ public static function registerCleanupHooks(\OC\SystemConfig $systemConfig): voi $throttler = Server::get(IThrottler::class); $throttler->resetDelay($request->getRemoteAddress(), 'login', ['user' => $uid]); } - - try { - $cache = new \OC\Cache\File(); - $cache->gc(); - } catch (\OC\ServerNotAvailableException $e) { - // not a GC exception, pass it on - throw $e; - } catch (\OC\ForbiddenException $e) { - // filesystem blocked for this request, ignore - } catch (\Exception $e) { - // a GC exception should not prevent users from using OC, - // so log the exception - Server::get(LoggerInterface::class)->warning('Exception when running cache gc.', [ - 'app' => 'core', - 'exception' => $e, - ]); - } }); } } From 0a8b458f49a4dd07299c208cbf656f0d1cab78ab Mon Sep 17 00:00:00 2001 From: provokateurin Date: Mon, 2 Jun 2025 15:36:57 +0200 Subject: [PATCH 3/4] refactor(Server): Deprecate \OCP\ICache service and replace it with a local cache Signed-off-by: provokateurin --- lib/private/Server.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/private/Server.php b/lib/private/Server.php index 4445788ec4e24..f7bb42e79087b 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -586,7 +586,16 @@ public function __construct($webRoot, \OC\Config $config) { $this->registerAlias(IURLGenerator::class, URLGenerator::class); - $this->registerAlias(ICache::class, Cache\File::class); + $this->registerService(ICache::class, function ($c) { + /** @var LoggerInterface $logger */ + $logger = $c->get(LoggerInterface::class); + $logger->debug('The requested service "' . ICache::class . '" is deprecated. Please use "' . ICacheFactory::class . '" instead to create a cache. This service will be removed in a future Nextcloud version.', ['app' => 'serverDI']); + + /** @var ICacheFactory $cacheFactory */ + $cacheFactory = $c->get(ICacheFactory::class); + return $cacheFactory->isLocalCacheAvailable() ? $cacheFactory->createLocal() : $cacheFactory->createInMemory(); + }); + $this->registerService(Factory::class, function (Server $c) { $profiler = $c->get(IProfiler::class); $logger = $c->get(LoggerInterface::class); From 0786ca8ac0f618e84f607474746d3f624f2a5d56 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 27 May 2025 15:18:56 +0200 Subject: [PATCH 4/4] chore: Remove unused \OC\Cache\File Signed-off-by: provokateurin --- build/psalm-baseline.xml | 6 - lib/composer/composer/autoload_classmap.php | 1 - lib/composer/composer/autoload_static.php | 1 - lib/private/Cache/File.php | 190 -------------------- tests/lib/Cache/FileCacheTest.php | 160 ----------------- 5 files changed, 358 deletions(-) delete mode 100644 lib/private/Cache/File.php delete mode 100644 tests/lib/Cache/FileCacheTest.php diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index df8b4d89e8bec..807292dcb4dfb 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3215,12 +3215,6 @@ providers]]> - - - - - - $baseDir . '/lib/private/BinaryFinder.php', 'OC\\Blurhash\\Listener\\GenerateBlurhashMetadata' => $baseDir . '/lib/private/Blurhash/Listener/GenerateBlurhashMetadata.php', 'OC\\Broadcast\\Events\\BroadcastEvent' => $baseDir . '/lib/private/Broadcast/Events/BroadcastEvent.php', - 'OC\\Cache\\File' => $baseDir . '/lib/private/Cache/File.php', 'OC\\Calendar\\AvailabilityResult' => $baseDir . '/lib/private/Calendar/AvailabilityResult.php', 'OC\\Calendar\\CalendarEventBuilder' => $baseDir . '/lib/private/Calendar/CalendarEventBuilder.php', 'OC\\Calendar\\CalendarQuery' => $baseDir . '/lib/private/Calendar/CalendarQuery.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 3b18f00da9697..c503068aafe96 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1230,7 +1230,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\BinaryFinder' => __DIR__ . '/../../..' . '/lib/private/BinaryFinder.php', 'OC\\Blurhash\\Listener\\GenerateBlurhashMetadata' => __DIR__ . '/../../..' . '/lib/private/Blurhash/Listener/GenerateBlurhashMetadata.php', 'OC\\Broadcast\\Events\\BroadcastEvent' => __DIR__ . '/../../..' . '/lib/private/Broadcast/Events/BroadcastEvent.php', - 'OC\\Cache\\File' => __DIR__ . '/../../..' . '/lib/private/Cache/File.php', 'OC\\Calendar\\AvailabilityResult' => __DIR__ . '/../../..' . '/lib/private/Calendar/AvailabilityResult.php', 'OC\\Calendar\\CalendarEventBuilder' => __DIR__ . '/../../..' . '/lib/private/Calendar/CalendarEventBuilder.php', 'OC\\Calendar\\CalendarQuery' => __DIR__ . '/../../..' . '/lib/private/Calendar/CalendarQuery.php', diff --git a/lib/private/Cache/File.php b/lib/private/Cache/File.php deleted file mode 100644 index ceddf472ebdc4..0000000000000 --- a/lib/private/Cache/File.php +++ /dev/null @@ -1,190 +0,0 @@ -storage !== null) { - return $this->storage; - } - $session = Server::get(IUserSession::class); - if ($session->isLoggedIn()) { - $rootView = new View(); - $userId = $session->getUser()->getUID(); - Filesystem::initMountPoints($userId); - if (!$rootView->file_exists('/' . $userId . '/cache')) { - $rootView->mkdir('/' . $userId . '/cache'); - } - $this->storage = new View('/' . $userId . '/cache'); - return $this->storage; - } else { - Server::get(LoggerInterface::class)->error('Can\'t get cache storage, user not logged in', ['app' => 'core']); - throw new \OC\ForbiddenException('Can\t get cache storage, user not logged in'); - } - } - - /** - * @param string $key - * @return mixed|null - * @throws \OC\ForbiddenException - */ - public function get($key) { - $result = null; - if ($this->hasKey($key)) { - $storage = $this->getStorage(); - $result = $storage->file_get_contents($key); - } - return $result; - } - - /** - * Returns the size of the stored/cached data - * - * @param string $key - * @return int - */ - public function size($key) { - $result = 0; - if ($this->hasKey($key)) { - $storage = $this->getStorage(); - $result = $storage->filesize($key); - } - return $result; - } - - /** - * @param string $key - * @param mixed $value - * @param int $ttl - * @return bool|mixed - * @throws \OC\ForbiddenException - */ - public function set($key, $value, $ttl = 0) { - $storage = $this->getStorage(); - $result = false; - // unique id to avoid chunk collision, just in case - $uniqueId = Server::get(ISecureRandom::class)->generate( - 16, - ISecureRandom::CHAR_ALPHANUMERIC - ); - - // use part file to prevent hasKey() to find the key - // while it is being written - $keyPart = $key . '.' . $uniqueId . '.part'; - if ($storage && $storage->file_put_contents($keyPart, $value)) { - if ($ttl === 0) { - $ttl = 86400; // 60*60*24 - } - $result = $storage->touch($keyPart, time() + $ttl); - $result &= $storage->rename($keyPart, $key); - } - return $result; - } - - /** - * @param string $key - * @return bool - * @throws \OC\ForbiddenException - */ - public function hasKey($key) { - $storage = $this->getStorage(); - if ($storage && $storage->is_file($key) && $storage->isReadable($key)) { - return true; - } - return false; - } - - /** - * @param string $key - * @return bool|mixed - * @throws \OC\ForbiddenException - */ - public function remove($key) { - $storage = $this->getStorage(); - if (!$storage) { - return false; - } - return $storage->unlink($key); - } - - /** - * @param string $prefix - * @return bool - * @throws \OC\ForbiddenException - */ - public function clear($prefix = '') { - $storage = $this->getStorage(); - if ($storage && $storage->is_dir('/')) { - $dh = $storage->opendir('/'); - if (is_resource($dh)) { - while (($file = readdir($dh)) !== false) { - if ($file !== '.' && $file !== '..' && ($prefix === '' || str_starts_with($file, $prefix))) { - $storage->unlink('/' . $file); - } - } - } - } - return true; - } - - /** - * Runs GC - * @throws \OC\ForbiddenException - */ - public function gc() { - $storage = $this->getStorage(); - if ($storage) { - // extra hour safety, in case of stray part chunks that take longer to write, - // because touch() is only called after the chunk was finished - $now = time() - 3600; - $dh = $storage->opendir('/'); - if (!is_resource($dh)) { - return null; - } - while (($file = readdir($dh)) !== false) { - if ($file !== '.' && $file !== '..') { - try { - $mtime = $storage->filemtime('/' . $file); - if ($mtime < $now) { - $storage->unlink('/' . $file); - } - } catch (\OCP\Lock\LockedException $e) { - // ignore locked chunks - Server::get(LoggerInterface::class)->debug('Could not cleanup locked chunk "' . $file . '"', ['app' => 'core']); - } catch (\OCP\Files\ForbiddenException $e) { - Server::get(LoggerInterface::class)->debug('Could not cleanup forbidden chunk "' . $file . '"', ['app' => 'core']); - } catch (\OCP\Files\LockNotAcquiredException $e) { - Server::get(LoggerInterface::class)->debug('Could not cleanup locked chunk "' . $file . '"', ['app' => 'core']); - } - } - } - } - } - - public static function isAvailable(): bool { - return true; - } -} diff --git a/tests/lib/Cache/FileCacheTest.php b/tests/lib/Cache/FileCacheTest.php deleted file mode 100644 index 4daa8d3b7effe..0000000000000 --- a/tests/lib/Cache/FileCacheTest.php +++ /dev/null @@ -1,160 +0,0 @@ -skipUnless(OC_User::isLoggedIn()); - } - - protected function setUp(): void { - parent::setUp(); - - //login - $this->createUser('test', 'test'); - - $this->user = \OC_User::getUser(); - \OC_User::setUserId('test'); - - //clear all proxies and hooks so we can do clean testing - \OC_Hook::clear('OC_Filesystem'); - - /** @var IMountManager $manager */ - $manager = Server::get(IMountManager::class); - $manager->removeMount('/test'); - - $storage = new Temporary([]); - Filesystem::mount($storage, [], '/test/cache'); - - //set up the users dir - $this->rootView = new View(''); - $this->rootView->mkdir('/test'); - - $this->instance = new File(); - - // forces creation of cache folder for subsequent tests - $this->instance->set('hack', 'hack'); - } - - protected function tearDown(): void { - if ($this->instance) { - $this->instance->remove('hack', 'hack'); - } - - \OC_User::setUserId($this->user); - - if ($this->instance) { - $this->instance->clear(); - $this->instance = null; - } - - parent::tearDown(); - } - - private function setupMockStorage() { - $mockStorage = $this->getMockBuilder(Local::class) - ->onlyMethods(['filemtime', 'unlink']) - ->setConstructorArgs([['datadir' => Server::get(ITempManager::class)->getTemporaryFolder()]]) - ->getMock(); - - Filesystem::mount($mockStorage, [], '/test/cache'); - - return $mockStorage; - } - - public function testGarbageCollectOldKeys(): void { - $mockStorage = $this->setupMockStorage(); - - $mockStorage->expects($this->atLeastOnce()) - ->method('filemtime') - ->willReturn(100); - $mockStorage->expects($this->once()) - ->method('unlink') - ->with('key1') - ->willReturn(true); - - $this->instance->set('key1', 'value1'); - $this->instance->gc(); - } - - public function testGarbageCollectLeaveRecentKeys(): void { - $mockStorage = $this->setupMockStorage(); - - $mockStorage->expects($this->atLeastOnce()) - ->method('filemtime') - ->willReturn(time() + 3600); - $mockStorage->expects($this->never()) - ->method('unlink') - ->with('key1'); - $this->instance->set('key1', 'value1'); - $this->instance->gc(); - } - - public static function lockExceptionProvider(): array { - return [ - [new LockedException('key1')], - [new LockNotAcquiredException('key1', 1)], - ]; - } - - #[\PHPUnit\Framework\Attributes\DataProvider('lockExceptionProvider')] - public function testGarbageCollectIgnoreLockedKeys($testException): void { - $mockStorage = $this->setupMockStorage(); - - $mockStorage->expects($this->atLeastOnce()) - ->method('filemtime') - ->willReturn(100); - $mockStorage->expects($this->atLeastOnce()) - ->method('unlink')->willReturnOnConsecutiveCalls($this->throwException($testException), $this->returnValue(true)); - - $this->instance->set('key1', 'value1'); - $this->instance->set('key2', 'value2'); - - $this->instance->gc(); - } -}