From b8c61b3515ef406c4feafc851f12262e417ba157 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Thu, 1 Jun 2023 20:19:24 +0200 Subject: [PATCH] fix(caching): Avoid checking existence before fetching The cache might expire between checking for key existence and fetching the value. In this rare case the code continues with a null value when it doesn't expect one. Signed-off-by: Christoph Wurst --- apps/files_external/lib/Lib/Storage/Swift.php | 5 +++-- apps/files_sharing/lib/External/Storage.php | 5 +++-- apps/files_sharing/lib/SharedMount.php | 5 +++-- lib/private/Accounts/AccountManager.php | 5 +++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/apps/files_external/lib/Lib/Storage/Swift.php b/apps/files_external/lib/Lib/Storage/Swift.php index 85b3727f4db6d..26e6c5315cbf4 100644 --- a/apps/files_external/lib/Lib/Storage/Swift.php +++ b/apps/files_external/lib/Lib/Storage/Swift.php @@ -126,9 +126,10 @@ private function normalizePath(string $path) { * @throws \OCP\Files\StorageNotAvailableException */ private function fetchObject(string $path) { - if ($this->objectCache->hasKey($path)) { + $cached = $this->objectCache->get($path); + if ($cached !== null) { // might be "false" if object did not exist from last check - return $this->objectCache->get($path); + return $cached; } try { $object = $this->getContainer()->getObject($path); diff --git a/apps/files_sharing/lib/External/Storage.php b/apps/files_sharing/lib/External/Storage.php index 506a2f83c2dac..9e6c169e14006 100644 --- a/apps/files_sharing/lib/External/Storage.php +++ b/apps/files_sharing/lib/External/Storage.php @@ -265,8 +265,9 @@ protected function testRemote(): bool { private function testRemoteUrl(string $url): bool { $cache = $this->memcacheFactory->createDistributed('files_sharing_remote_url'); - if ($cache->hasKey($url)) { - return (bool)$cache->get($url); + $cached = $cache->get($url); + if ($cached !== null) { + return (bool)$cached; } $client = $this->httpClient->newClient(); diff --git a/apps/files_sharing/lib/SharedMount.php b/apps/files_sharing/lib/SharedMount.php index 943f14dc2bd5d..6cde9f82bec01 100644 --- a/apps/files_sharing/lib/SharedMount.php +++ b/apps/files_sharing/lib/SharedMount.php @@ -117,8 +117,9 @@ private function verifyMountPoint( $this->eventDispatcher->dispatchTyped($event); $parent = $event->getParent(); - if ($folderExistCache->hasKey($parent)) { - $parentExists = $folderExistCache->get($parent); + $cached = $folderExistCache->get($parent); + if ($cached) { + $parentExists = $cached; } else { $parentExists = $this->recipientView->is_dir($parent); $folderExistCache->set($parent, $parentExists); diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index 60065272a5899..e3068a7ff25f9 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -795,8 +795,9 @@ private function parseAccountData(IUser $user, $data): Account { } public function getAccount(IUser $user): IAccount { - if ($this->internalCache->hasKey($user->getUID())) { - return $this->internalCache->get($user->getUID()); + $cached = $this->internalCache->get($user->getUID()); + if ($cached !== null) { + return $cached; } $account = $this->parseAccountData($user, $this->getUser($user)); $this->internalCache->set($user->getUID(), $account);