Bug description
While running the repair steps after upgrading to 34.0.3, occ maintenance:repair --include-expensive aborts at the new "Cleanup share names with false conflicts" step:
- Cleanup share names with false conflicts
28/907 [>---------------------------] 3%An unhandled exception has been thrown:
Error: Call to a member function getRootId() on null in apps/files_sharing/lib/Repair/CleanupShareTarget.php:128
Root cause (apps/files_sharing/lib/Repair/CleanupShareTarget.php):
$oldMountPoint = "/{$recipient->getUID()}/files$oldTarget/";
...
/** @var ICachedMountInfo $mount */
$mount = $userMounts[$oldMountPoint]; // line 122 — no null check
...
'fileid' => $mount->getRootId(), // line 128 — fatals when the key is missing
$userMounts is built from IUserMountCache::getMountsForUser(). When the recipient's mount cache has no entry for the old share target (in our case some recipients hadn't logged in for a long time, so oc_mounts had no row for those targets), the array lookup yields null and the getRootId() call throws Error.
Two aggravating details:
- Line 111 in the same method already guards the exact same lookup with
fn ($path) => $userMounts[$path] ?? null — line 122 just misses the same guard.
- The
catch (\Exception $e) at line 132 doesn't catch \Error, so the whole repair run aborts instead of skipping the one share.
Note that moveShare() (line 116) has already updated the oc_share row at that point — only the mount-cache refresh needs guarding, and the cache rebuilds itself on the user's next login anyway.
Suggested fix:
/** @var ICachedMountInfo|null $mount */
$mount = $userMounts[$oldMountPoint] ?? null;
if ($mount !== null) {
$userMounts[$newMountPoint] = $mount;
unset($userMounts[$oldMountPoint]);
$this->userMountCache->removeMount($oldMountPoint);
$this->userMountCache->addMount($recipient, $newMountPoint, new CacheEntry([
'fileid' => $mount->getRootId(),
'storage' => $mount->getStorageId(),
]), $mount->getMountProvider(), $mount->getMountId());
}
Patched it locally like this and the step completed over all 907 shares without further issues. Changing catch (\Exception ...) to catch (\Throwable ...) might also be worth considering so a single bad share can't abort the whole run.
Steps to reproduce
- Have user/group shares whose
file_target matches % (_) (_)% (duplicated conflict suffixes like Folder (2) (2))
- Make sure at least one recipient of such a share has no row in
oc_mounts for that target (e.g. a user who hasn't logged in since the share was created)
- Run
occ maintenance:repair --include-expensive
Expected behavior
The repair step should skip (or log) shares whose mount info is not cached, and finish the run.
Environment
- Nextcloud Server 34.0.3 (upgraded from 33.0.x)
- PostgreSQL, Apache, Linux
Full stack trace
Error: Call to a member function getRootId() on null in apps/files_sharing/lib/Repair/CleanupShareTarget.php:128
Stack trace:
#0 lib/private/Repair.php(104): OCA\Files_Sharing\Repair\CleanupShareTarget->run()
#1 core/Command/Maintenance/Repair.php(96): OC\Repair->run()
#2 3rdparty/symfony/console/Command/Command.php(326): OC\Core\Command\Maintenance\Repair->execute()
#3 3rdparty/symfony/console/Application.php(1098): Symfony\Component\Console\Command\Command->run()
#4 3rdparty/symfony/console/Application.php(324): Symfony\Component\Console\Application->doRunCommand()
#5 3rdparty/symfony/console/Application.php(175): Symfony\Component\Console\Application->doRun()
#6 lib/private/Console/Application.php(209): Symfony\Component\Console\Application->run()
#7 console.php(92): OC\Console\Application->run()
#8 occ(33): require_once('...')
#9 {main}
Bug description
While running the repair steps after upgrading to 34.0.3,
occ maintenance:repair --include-expensiveaborts at the new "Cleanup share names with false conflicts" step:Root cause (
apps/files_sharing/lib/Repair/CleanupShareTarget.php):$userMountsis built fromIUserMountCache::getMountsForUser(). When the recipient's mount cache has no entry for the old share target (in our case some recipients hadn't logged in for a long time, sooc_mountshad no row for those targets), the array lookup yieldsnulland thegetRootId()call throwsError.Two aggravating details:
fn ($path) => $userMounts[$path] ?? null— line 122 just misses the same guard.catch (\Exception $e)at line 132 doesn't catch\Error, so the whole repair run aborts instead of skipping the one share.Note that
moveShare()(line 116) has already updated theoc_sharerow at that point — only the mount-cache refresh needs guarding, and the cache rebuilds itself on the user's next login anyway.Suggested fix:
Patched it locally like this and the step completed over all 907 shares without further issues. Changing
catch (\Exception ...)tocatch (\Throwable ...)might also be worth considering so a single bad share can't abort the whole run.Steps to reproduce
file_targetmatches% (_) (_)%(duplicated conflict suffixes likeFolder (2) (2))oc_mountsfor that target (e.g. a user who hasn't logged in since the share was created)occ maintenance:repair --include-expensiveExpected behavior
The repair step should skip (or log) shares whose mount info is not cached, and finish the run.
Environment
Full stack trace