From a99e099f22ad4589ef1ca5995ac56f88b4a2bce9 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 27 May 2025 15:18:25 +0200 Subject: [PATCH 1/4] refactor(OC): Setup filesystem using ISetupManager instead of unused \OC\Cache\File Signed-off-by: provokateurin --- lib/OC.php | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/lib/OC.php b/lib/OC.php index eeaf7c190cd4c..c24b6551853e8 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -19,6 +19,7 @@ use OCP\App\Events\AppsLoadedEvent; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Events\BeforeFileSystemSetupEvent; +use OCP\Files\ISetupManager; use OCP\Group\Events\GroupDeletedEvent; use OCP\Group\Events\UserRemovedEvent; use OCP\IAppConfig; @@ -1023,29 +1024,14 @@ public static function registerCleanupHooks(\OC\SystemConfig $systemConfig): voi // NOTE: This will be replaced to use OCP $userSession = Server::get(\OC\User\Session::class); $userSession->listen('\OC\User', 'postLogin', function () use ($userSession) { - if (!defined('PHPUNIT_RUN') && $userSession->isLoggedIn()) { + if (!defined('PHPUNIT_RUN') && $userSession->isLoggedIn() && ($user = $userSession->getUser()) !== null) { // reset brute force delay for this IP address and username - $uid = $userSession->getUser()->getUID(); + $uid = $user->getUID(); $request = Server::get(IRequest::class); $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, - ]); - } + Server::get(ISetupManager::class)->setupForUser($user); } }); } From 7b715e2c8801788f10aa5506e11a04c8399c5491 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Mon, 2 Jun 2025 15:36:57 +0200 Subject: [PATCH 2/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 5b692c1a6c9f5..b0390ff4fd3a1 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -491,7 +491,16 @@ public function __construct( $this->registerAlias(IURLGenerator::class, URLGenerator::class); - $this->registerAlias(ICache::class, Cache\File::class); + $this->registerService(ICache::class, static 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, static function (Server $c) { $profiler = $c->get(IProfiler::class); $logger = $c->get(LoggerInterface::class); From d80ece51b4ec22669537862665621da7cc8cae5a Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 27 May 2025 15:18:56 +0200 Subject: [PATCH 3/4] chore: Remove unused \OC\Cache\File Signed-off-by: provokateurin --- build/psalm-baseline.xml | 19 -- lib/composer/composer/autoload_classmap.php | 1 - lib/composer/composer/autoload_static.php | 1 - lib/private/Cache/File.php | 202 -------------------- tests/lib/Cache/FileCacheTest.php | 163 ---------------- 5 files changed, 386 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 3e991c6d14e32..b6da12c8b93f5 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -3223,25 +3223,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 6f4d27b27eee9..16f9582fd70dc 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1406,7 +1406,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 addc9b0cd20d0..0000000000000 --- a/lib/private/Cache/File.php +++ /dev/null @@ -1,202 +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 ForbiddenException('Can\t get cache storage, user not logged in'); - } - } - - /** - * @param string $key - * @return mixed|null - * @throws ForbiddenException - */ - #[\Override] - 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 ForbiddenException - */ - #[\Override] - 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 ForbiddenException - */ - #[\Override] - 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 ForbiddenException - */ - #[\Override] - public function remove($key) { - $storage = $this->getStorage(); - if (!$storage) { - return false; - } - return $storage->unlink($key); - } - - /** - * @param string $prefix - * @return bool - * @throws ForbiddenException - */ - #[\Override] - 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 ForbiddenException - */ - public function gc() { - $storage = $this->getStorage(); - if ($storage) { - $ttl = Server::get(IConfig::class)->getSystemValueInt('cache_chunk_gc_ttl', 60 * 60 * 24); - $now = time() - $ttl; - - $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 (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 (LockNotAcquiredException $e) { - Server::get(LoggerInterface::class)->debug('Could not cleanup locked chunk "' . $file . '"', ['app' => 'core']); - } - } - } - } - } - - #[\Override] - 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 8cf5eecf809b1..0000000000000 --- a/tests/lib/Cache/FileCacheTest.php +++ /dev/null @@ -1,163 +0,0 @@ -skipUnless(OC_User::isLoggedIn()); - } - - #[\Override] - 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'); - } - - #[\Override] - 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), true); - - $this->instance->set('key1', 'value1'); - $this->instance->set('key2', 'value2'); - - $this->instance->gc(); - } -} From b9c3f34cc72bcc2b3791700141fab402d804f28c Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Tue, 23 Jun 2026 14:33:26 +0200 Subject: [PATCH 4/4] fix(psalm): Fix some unrelated psalm issues Signed-off-by: Carl Schwan --- lib/private/Authentication/WebAuthn/Manager.php | 2 +- lib/private/Avatar/Avatar.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Authentication/WebAuthn/Manager.php b/lib/private/Authentication/WebAuthn/Manager.php index b32184add015c..4837fb324aa23 100644 --- a/lib/private/Authentication/WebAuthn/Manager.php +++ b/lib/private/Authentication/WebAuthn/Manager.php @@ -142,7 +142,7 @@ public function finishRegister(PublicKeyCredentialCreationOptions $publicKeyCred } private function stripPort(string $serverHost): string { - return preg_replace('/(:\d+$)/', '', $serverHost); + return preg_replace('/(:\d+$)/', '', $serverHost) ?? ''; } public function startAuthentication(string $uid, string $serverHost): PublicKeyCredentialRequestOptions { diff --git a/lib/private/Avatar/Avatar.php b/lib/private/Avatar/Avatar.php index bcf85a58cb312..2abf041e23056 100644 --- a/lib/private/Avatar/Avatar.php +++ b/lib/private/Avatar/Avatar.php @@ -190,7 +190,7 @@ protected function generateAvatar(string $userDisplayName, int $size, bool $dark $data = ob_get_contents(); ob_end_clean(); - return $data; + return $data ?: ''; } /**