From 2f3b351adf92df9dba0e391fe51511ad2ae7deab Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 24 Jul 2026 08:19:14 -0400 Subject: [PATCH 1/3] fix(view): harden rename lock cleanup Preserve operation errors, release partially acquired locks, and log cleanup failures without masking the original exception. Assisted-by: Copilot:GPT-5.6 Signed-off-by: Josh --- lib/private/Files/View.php | 175 +++++++++++++++++++++++++++++-------- 1 file changed, 137 insertions(+), 38 deletions(-) diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index c4162c7df046f..844087bc41dd6 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -763,13 +763,19 @@ public function rename($source, $target, array $options = []) { return false; } - $this->lockFile($source, ILockingProvider::LOCK_SHARED, true); + $sourceLocked = false; + $targetLocked = false; + $sourceLockType = ILockingProvider::LOCK_SHARED; + $targetLockType = ILockingProvider::LOCK_SHARED; + $operationException = null; + try { - $this->lockFile($target, ILockingProvider::LOCK_SHARED, true); + $sourceLocked = $this->lockFile($source, ILockingProvider::LOCK_SHARED, true); + $targetLocked = $this->lockFile($target, ILockingProvider::LOCK_SHARED, true); $run = true; if ($this->shouldEmitHooks($source) && (Scanner::isPartialFile($source) && !Scanner::isPartialFile($target))) { - // if it was a rename from a part file to a regular file it was a write and not a rename operation + // If this is a rename from a part file to a regular file, it is a write rather than a rename. $this->emit_file_hooks_pre($exists, $target, $run); } elseif ($this->shouldEmitHooks($source)) { $sourcePath = $this->getHookPath($source); @@ -785,6 +791,7 @@ public function rename($source, $target, array $options = []) { ); } } + if ($run) { $manager = Filesystem::getMountManager(); $mount1 = $this->getMount($source); @@ -795,8 +802,12 @@ public function rename($source, $target, array $options = []) { $internalPath2 = $mount2->getInternalPath($absolutePath2); $this->changeLock($source, ILockingProvider::LOCK_EXCLUSIVE, true); + $sourceLockType = ILockingProvider::LOCK_EXCLUSIVE; + + $lockDowngradeException = null; try { $this->changeLock($target, ILockingProvider::LOCK_EXCLUSIVE, true); + $targetLockType = ILockingProvider::LOCK_EXCLUSIVE; if ($checkSubMounts) { $movedMounts = $mountManager->findIn($this->getAbsolutePath($source)); @@ -807,75 +818,163 @@ public function rename($source, $target, array $options = []) { if ($internalPath1 === '') { $sourceParentMount = $this->getMount(dirname($source)); $movedMounts[] = $mount1; - $this->validateMountMove($movedMounts, $sourceParentMount, $mount2, !$this->targetIsNotShared($targetUser, $absolutePath2)); - /** - * @var MountPoint|IMovableMount $mount1 - */ + $this->validateMountMove( + $movedMounts, + $sourceParentMount, + $mount2, + !$this->targetIsNotShared($targetUser, $absolutePath2), + ); + + /** @var MountPoint|IMovableMount $mount1 */ $sourceMountPoint = $mount1->getMountPoint(); $result = $mount1->moveMount($absolutePath2); $manager->moveMount($sourceMountPoint, $mount1->getMountPoint()); - // moving a file/folder within the same mount point + // moving a file/folder within the same mount point } elseif ($storage1 === $storage2) { if (count($movedMounts) > 0) { - $this->validateMountMove($movedMounts, $mount1, $mount2, !$this->targetIsNotShared($targetUser, $absolutePath2)); + $this->validateMountMove( + $movedMounts, $mount1, + $mount2, + !$this->targetIsNotShared($targetUser, $absolutePath2), + ); } + if ($storage1) { $result = $storage1->rename($internalPath1, $internalPath2); - } else { - $result = false; } - // moving a file/folder between storages (from $storage1 to $storage2) + + // moving a file/folder between storages (from $storage1 to $storage2) } else { if (count($movedMounts) > 0) { - $this->validateMountMove($movedMounts, $mount1, $mount2, !$this->targetIsNotShared($targetUser, $absolutePath2)); + $this->validateMountMove( + $movedMounts, + $mount1, + $mount2, + !$this->targetIsNotShared($targetUser, $absolutePath2), + ); } + $result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2); } if ((Scanner::isPartialFile($source) && !Scanner::isPartialFile($target)) && $result !== false) { - // if it was a rename from a part file to a regular file it was a write and not a rename operation + // A part-file rename is a write rather than a rename. $this->writeUpdate($storage2, $internalPath2); - } elseif ($result) { - if ($internalPath1 !== '') { // don't do a cache update for moved mounts - $this->renameUpdate($storage1, $storage2, $internalPath1, $internalPath2); - } + } elseif ($result && $internalPath1 !== '') { + // Do not update cache entries for moved mounts. + $this->renameUpdate($storage1, $storage2, $internalPath1, $internalPath2); } - } catch (\Exception $e) { + } catch (\Throwable $e) { + $operationException = $e; throw $e; } finally { - $this->changeLock($source, ILockingProvider::LOCK_SHARED, true); - $this->changeLock($target, ILockingProvider::LOCK_SHARED, true); + try { + if ($sourceLockType === ILockingProvider::LOCK_EXCLUSIVE) { + $this->changeLock($source, ILockingProvider::LOCK_SHARED, true); + $sourceLockType = ILockingProvider::LOCK_SHARED; + } + } catch (\Throwable $sourceCleanupException) { + if ($operationException !== null) { + $this->logger->error('Failed to downgrade source lock after rename operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $sourceCleanupException, + ]); + } else { + $lockDowngradeException = $sourceCleanupException; + } + } + + try { + if ($targetLockType === ILockingProvider::LOCK_EXCLUSIVE) { + $this->changeLock($target, ILockingProvider::LOCK_SHARED, true); + $targetLockType = ILockingProvider::LOCK_SHARED; + } + } catch (\Throwable $targetCleanupException) { + if ($operationException !== null || $lockDowngradeException !== null) { + $this->logger->error('Failed to downgrade target lock after rename operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $targetCleanupException, + ]); + } else { + $lockDowngradeException = $targetCleanupException; + } + } + + if ($operationException === null && $lockDowngradeException !== null) { + throw $lockDowngradeException; + } } if ((Scanner::isPartialFile($source) && !Scanner::isPartialFile($target)) && $result !== false) { if ($this->shouldEmitHooks()) { $this->emit_file_hooks_post($exists, $target); } - } elseif ($result) { - if ($this->shouldEmitHooks($source) && $this->shouldEmitHooks($target)) { - $sourcePath = $this->getHookPath($source); - $targetPath = $this->getHookPath($target); - if ($sourcePath !== null && $targetPath !== null) { - \OC_Hook::emit( - Filesystem::CLASSNAME, - Filesystem::signal_post_rename, - [ - Filesystem::signal_param_oldpath => $sourcePath, - Filesystem::signal_param_newpath => $targetPath, - ] - ); - } + } elseif ($result && $this->shouldEmitHooks($source) && $this->shouldEmitHooks($target)) { + $sourcePath = $this->getHookPath($source); + $targetPath = $this->getHookPath($target); + if ($sourcePath !== null && $targetPath !== null) { + \OC_Hook::emit( + Filesystem::CLASSNAME, + Filesystem::signal_post_rename, + [ + Filesystem::signal_param_oldpath => $sourcePath, + Filesystem::signal_param_newpath => $targetPath, + ] + ); } } } - } catch (\Exception $e) { + } catch (\Throwable $e) { + $operationException ??= $e; throw $e; } finally { - $this->unlockFile($source, ILockingProvider::LOCK_SHARED, true); - $this->unlockFile($target, ILockingProvider::LOCK_SHARED, true); + $unlockException = null; + + try { + if ($targetLocked) { + $this->unlockFile($target, $targetLockType, true); + } + } catch (\Throwable $targetCleanupException) { + if ($operationException !== null) { + $this->logger->error('Failed to release target lock after rename operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $targetCleanupException, + ]); + } else { + $unlockException = $targetCleanupException; + } + } + + try { + if ($sourceLocked) { + $this->unlockFile($source, $sourceLockType, true); + } + } catch (\Throwable $sourceCleanupException) { + if ($operationException !== null || $unlockException !== null) { + $this->logger->error('Failed to release source lock after rename operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $sourceCleanupException, + ]); + } else { + $unlockException = $sourceCleanupException; + } + } + + if ($operationException === null && $unlockException !== null) { + throw $unlockException; + } } } + return $result; } From 84c002b0fd18523ff7627df5f2c35953250a34dc Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 24 Jul 2026 08:23:18 -0400 Subject: [PATCH 2/3] test(view): cover rename lock cleanup after storage failure Signed-off-by: Josh --- tests/lib/Files/ViewTest.php | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/lib/Files/ViewTest.php b/tests/lib/Files/ViewTest.php index 3b8718564020e..f66edcf89bc04 100644 --- a/tests/lib/Files/ViewTest.php +++ b/tests/lib/Files/ViewTest.php @@ -2358,6 +2358,42 @@ public function testLockFileRenameUnlockOnException(): void { $view->unlockFile($targetPath, ILockingProvider::LOCK_EXCLUSIVE); } + public function testLockFileRenameUnlocksAfterStorageException(): void { + $view = new View('/' . self::$user . '/files/'); + + /** @var Temporary&MockObject $storage */ + $storage = $this->getMockBuilder(Temporary::class) + ->onlyMethods(['rename']) + ->getMock(); + + Filesystem::mount($storage, [], self::$user . '/'); + $storage->mkdir('files'); + + $sourcePath = 'source.txt'; + $targetPath = 'target.txt'; + $view->file_put_contents($sourcePath, 'content'); + + $storage->expects($this->once()) + ->method('rename') + ->willThrowException(new \RuntimeException('Simulated rename failure')); + + try { + $view->rename($sourcePath, $targetPath); + $this->fail('Expected the storage exception to be rethrown'); + } catch (\RuntimeException $e) { + $this->assertSame('Simulated rename failure', $e->getMessage()); + } + + $this->assertNull( + $this->getFileLockType($view, $sourcePath, true), + 'The source mount-point lock must be released after a failed rename' + ); + $this->assertNull( + $this->getFileLockType($view, $targetPath, true), + 'The target mount-point lock must be released after a failed rename' + ); + } + /** * Test rename operation: unlock first path when second path was locked */ From 4ed54a1530fe890740aeff4462b4f54d9f8d68cb Mon Sep 17 00:00:00 2001 From: Josh Date: Fri, 24 Jul 2026 08:42:33 -0400 Subject: [PATCH 3/3] refactor(view): simplify rename control flow Signed-off-by: Josh --- lib/private/Files/View.php | 399 +++++++++++++++++++------------------ 1 file changed, 204 insertions(+), 195 deletions(-) diff --git a/lib/private/Files/View.php b/lib/private/Files/View.php index 844087bc41dd6..1ce707ceed83a 100644 --- a/lib/private/Files/View.php +++ b/lib/private/Files/View.php @@ -719,13 +719,15 @@ public function deleteAll($directory) { } /** - * Rename/move a file or folder from the source path to target path. + * Rename or move a file or folder. * - * @param string $source source path - * @param string $target target path - * @param array $options + * @param string $source Source path + * @param string $target Target path + * @param array{checkSubMounts?: bool} $options Options controlling mount validation + * @return mixed Result from the underlying storage operation, or false on failure * - * @return bool|mixed + * @throws ForbiddenException When moving a folder into one of its descendants + * @throws InvalidPathException * @throws LockedException */ public function rename($source, $target, array $options = []) { @@ -738,240 +740,247 @@ public function rename($source, $target, array $options = []) { throw new ForbiddenException('Moving a folder into a child folder is forbidden', false); } + if ( + !Filesystem::isValidPath($target) + || !Filesystem::isValidPath($source) + || Filesystem::isFileBlacklisted($target) + ) { + return false; + } + /** @var IMountManager $mountManager */ $mountManager = Server::get(IMountManager::class); $targetParts = explode('/', $absolutePath2); $targetUser = $targetParts[1] ?? null; $result = false; - if ( - Filesystem::isValidPath($target) - && Filesystem::isValidPath($source) - && !Filesystem::isFileBlacklisted($target) - ) { - $source = $this->getRelativePath($absolutePath1); - $target = $this->getRelativePath($absolutePath2); - $exists = $this->file_exists($target); - if ($source === null || $target === null) { - return false; - } + $source = $this->getRelativePath($absolutePath1); + $target = $this->getRelativePath($absolutePath2); + if ($source === null || $target === null) { + return false; + } - try { - $this->verifyPath(dirname($target), basename($target)); - } catch (InvalidPathException) { - return false; - } + $exists = $this->file_exists($target); - $sourceLocked = false; - $targetLocked = false; - $sourceLockType = ILockingProvider::LOCK_SHARED; - $targetLockType = ILockingProvider::LOCK_SHARED; - $operationException = null; + try { + $this->verifyPath(dirname($target), basename($target)); + } catch (InvalidPathException) { + return false; + } - try { - $sourceLocked = $this->lockFile($source, ILockingProvider::LOCK_SHARED, true); - $targetLocked = $this->lockFile($target, ILockingProvider::LOCK_SHARED, true); + $sourceLocked = false; + $targetLocked = false; + $sourceLockType = ILockingProvider::LOCK_SHARED; + $targetLockType = ILockingProvider::LOCK_SHARED; + $operationException = null; - $run = true; - if ($this->shouldEmitHooks($source) && (Scanner::isPartialFile($source) && !Scanner::isPartialFile($target))) { - // If this is a rename from a part file to a regular file, it is a write rather than a rename. - $this->emit_file_hooks_pre($exists, $target, $run); - } elseif ($this->shouldEmitHooks($source)) { - $sourcePath = $this->getHookPath($source); - $targetPath = $this->getHookPath($target); - if ($sourcePath !== null && $targetPath !== null) { - \OC_Hook::emit( - Filesystem::CLASSNAME, Filesystem::signal_rename, - [ - Filesystem::signal_param_oldpath => $sourcePath, - Filesystem::signal_param_newpath => $targetPath, - Filesystem::signal_param_run => &$run - ] - ); - } + try { + $sourceLocked = $this->lockFile($source, ILockingProvider::LOCK_SHARED, true); + $targetLocked = $this->lockFile($target, ILockingProvider::LOCK_SHARED, true); + + $run = true; + if ($this->shouldEmitHooks($source) && (Scanner::isPartialFile($source) && !Scanner::isPartialFile($target))) { + // If this is a rename from a part file to a regular file, it is a write rather than a rename. + $this->emit_file_hooks_pre($exists, $target, $run); + } elseif ($this->shouldEmitHooks($source)) { + $sourcePath = $this->getHookPath($source); + $targetPath = $this->getHookPath($target); + if ($sourcePath !== null && $targetPath !== null) { + \OC_Hook::emit( + Filesystem::CLASSNAME, Filesystem::signal_rename, + [ + Filesystem::signal_param_oldpath => $sourcePath, + Filesystem::signal_param_newpath => $targetPath, + Filesystem::signal_param_run => &$run + ] + ); } + } - if ($run) { - $manager = Filesystem::getMountManager(); - $mount1 = $this->getMount($source); - $mount2 = $this->getMount($target); - $storage1 = $mount1->getStorage(); - $storage2 = $mount2->getStorage(); - $internalPath1 = $mount1->getInternalPath($absolutePath1); - $internalPath2 = $mount2->getInternalPath($absolutePath2); + if ($run) { + $manager = Filesystem::getMountManager(); + $mount1 = $this->getMount($source); + $mount2 = $this->getMount($target); + $storage1 = $mount1->getStorage(); + $storage2 = $mount2->getStorage(); + $internalPath1 = $mount1->getInternalPath($absolutePath1); + $internalPath2 = $mount2->getInternalPath($absolutePath2); - $this->changeLock($source, ILockingProvider::LOCK_EXCLUSIVE, true); - $sourceLockType = ILockingProvider::LOCK_EXCLUSIVE; + $this->changeLock($source, ILockingProvider::LOCK_EXCLUSIVE, true); + $sourceLockType = ILockingProvider::LOCK_EXCLUSIVE; - $lockDowngradeException = null; - try { - $this->changeLock($target, ILockingProvider::LOCK_EXCLUSIVE, true); - $targetLockType = ILockingProvider::LOCK_EXCLUSIVE; + $lockDowngradeException = null; + try { + $this->changeLock($target, ILockingProvider::LOCK_EXCLUSIVE, true); + $targetLockType = ILockingProvider::LOCK_EXCLUSIVE; - if ($checkSubMounts) { - $movedMounts = $mountManager->findIn($this->getAbsolutePath($source)); - } else { - $movedMounts = []; - } + if ($checkSubMounts) { + $movedMounts = $mountManager->findIn($this->getAbsolutePath($source)); + } else { + $movedMounts = []; + } + + if ($internalPath1 === '') { + $sourceParentMount = $this->getMount(dirname($source)); + $movedMounts[] = $mount1; + $this->validateMountMove( + $movedMounts, + $sourceParentMount, + $mount2, + !$this->targetIsNotShared($targetUser, $absolutePath2), + ); + + /** @var MountPoint|IMovableMount $mount1 */ + $sourceMountPoint = $mount1->getMountPoint(); + $result = $mount1->moveMount($absolutePath2); + $manager->moveMount($sourceMountPoint, $mount1->getMountPoint()); - if ($internalPath1 === '') { - $sourceParentMount = $this->getMount(dirname($source)); - $movedMounts[] = $mount1; + // moving a file/folder within the same mount point + } elseif ($storage1 === $storage2) { + if (count($movedMounts) > 0) { $this->validateMountMove( $movedMounts, - $sourceParentMount, + $mount1, $mount2, !$this->targetIsNotShared($targetUser, $absolutePath2), ); - - /** @var MountPoint|IMovableMount $mount1 */ - $sourceMountPoint = $mount1->getMountPoint(); - $result = $mount1->moveMount($absolutePath2); - $manager->moveMount($sourceMountPoint, $mount1->getMountPoint()); - - // moving a file/folder within the same mount point - } elseif ($storage1 === $storage2) { - if (count($movedMounts) > 0) { - $this->validateMountMove( - $movedMounts, $mount1, - $mount2, - !$this->targetIsNotShared($targetUser, $absolutePath2), - ); - } - - if ($storage1) { - $result = $storage1->rename($internalPath1, $internalPath2); - } - - // moving a file/folder between storages (from $storage1 to $storage2) - } else { - if (count($movedMounts) > 0) { - $this->validateMountMove( - $movedMounts, - $mount1, - $mount2, - !$this->targetIsNotShared($targetUser, $absolutePath2), - ); - } - - $result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2); } - if ((Scanner::isPartialFile($source) && !Scanner::isPartialFile($target)) && $result !== false) { - // A part-file rename is a write rather than a rename. - $this->writeUpdate($storage2, $internalPath2); - } elseif ($result && $internalPath1 !== '') { - // Do not update cache entries for moved mounts. - $this->renameUpdate($storage1, $storage2, $internalPath1, $internalPath2); - } - } catch (\Throwable $e) { - $operationException = $e; - throw $e; - } finally { - try { - if ($sourceLockType === ILockingProvider::LOCK_EXCLUSIVE) { - $this->changeLock($source, ILockingProvider::LOCK_SHARED, true); - $sourceLockType = ILockingProvider::LOCK_SHARED; - } - } catch (\Throwable $sourceCleanupException) { - if ($operationException !== null) { - $this->logger->error('Failed to downgrade source lock after rename operation', [ - 'app' => 'core', - 'source' => $source, - 'target' => $target, - 'exception' => $sourceCleanupException, - ]); - } else { - $lockDowngradeException = $sourceCleanupException; - } + if ($storage1) { + $result = $storage1->rename($internalPath1, $internalPath2); + } else { + // Missing source storage is a failed rename + $result = false; } - try { - if ($targetLockType === ILockingProvider::LOCK_EXCLUSIVE) { - $this->changeLock($target, ILockingProvider::LOCK_SHARED, true); - $targetLockType = ILockingProvider::LOCK_SHARED; - } - } catch (\Throwable $targetCleanupException) { - if ($operationException !== null || $lockDowngradeException !== null) { - $this->logger->error('Failed to downgrade target lock after rename operation', [ - 'app' => 'core', - 'source' => $source, - 'target' => $target, - 'exception' => $targetCleanupException, - ]); - } else { - $lockDowngradeException = $targetCleanupException; - } + // moving a file/folder between storages (from $storage1 to $storage2) + } else { + if (count($movedMounts) > 0) { + $this->validateMountMove( + $movedMounts, + $mount1, + $mount2, + !$this->targetIsNotShared($targetUser, $absolutePath2), + ); } - if ($operationException === null && $lockDowngradeException !== null) { - throw $lockDowngradeException; - } + $result = $storage2->moveFromStorage($storage1, $internalPath1, $internalPath2); } if ((Scanner::isPartialFile($source) && !Scanner::isPartialFile($target)) && $result !== false) { - if ($this->shouldEmitHooks()) { - $this->emit_file_hooks_post($exists, $target); + // A part-file rename is a write rather than a rename. + $this->writeUpdate($storage2, $internalPath2); + } elseif ($result && $internalPath1 !== '') { + // Do not update cache entries for moved mounts. + $this->renameUpdate($storage1, $storage2, $internalPath1, $internalPath2); + } + } catch (\Throwable $e) { + $operationException = $e; + throw $e; + } finally { + try { + if ($sourceLockType === ILockingProvider::LOCK_EXCLUSIVE) { + $this->changeLock($source, ILockingProvider::LOCK_SHARED, true); + $sourceLockType = ILockingProvider::LOCK_SHARED; } - } elseif ($result && $this->shouldEmitHooks($source) && $this->shouldEmitHooks($target)) { - $sourcePath = $this->getHookPath($source); - $targetPath = $this->getHookPath($target); - if ($sourcePath !== null && $targetPath !== null) { - \OC_Hook::emit( - Filesystem::CLASSNAME, - Filesystem::signal_post_rename, - [ - Filesystem::signal_param_oldpath => $sourcePath, - Filesystem::signal_param_newpath => $targetPath, - ] - ); + } catch (\Throwable $sourceCleanupException) { + if ($operationException !== null) { + $this->logger->error('Failed to downgrade source lock after rename operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $sourceCleanupException, + ]); + } else { + $lockDowngradeException = $sourceCleanupException; } } - } - } catch (\Throwable $e) { - $operationException ??= $e; - throw $e; - } finally { - $unlockException = null; - try { - if ($targetLocked) { - $this->unlockFile($target, $targetLockType, true); + try { + if ($targetLockType === ILockingProvider::LOCK_EXCLUSIVE) { + $this->changeLock($target, ILockingProvider::LOCK_SHARED, true); + $targetLockType = ILockingProvider::LOCK_SHARED; + } + } catch (\Throwable $targetCleanupException) { + if ($operationException !== null || $lockDowngradeException !== null) { + $this->logger->error('Failed to downgrade target lock after rename operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $targetCleanupException, + ]); + } else { + $lockDowngradeException = $targetCleanupException; + } } - } catch (\Throwable $targetCleanupException) { - if ($operationException !== null) { - $this->logger->error('Failed to release target lock after rename operation', [ - 'app' => 'core', - 'source' => $source, - 'target' => $target, - 'exception' => $targetCleanupException, - ]); - } else { - $unlockException = $targetCleanupException; + + if ($operationException === null && $lockDowngradeException !== null) { + throw $lockDowngradeException; } } - try { - if ($sourceLocked) { - $this->unlockFile($source, $sourceLockType, true); + if ((Scanner::isPartialFile($source) && !Scanner::isPartialFile($target)) && $result !== false) { + if ($this->shouldEmitHooks()) { + $this->emit_file_hooks_post($exists, $target); } - } catch (\Throwable $sourceCleanupException) { - if ($operationException !== null || $unlockException !== null) { - $this->logger->error('Failed to release source lock after rename operation', [ - 'app' => 'core', - 'source' => $source, - 'target' => $target, - 'exception' => $sourceCleanupException, - ]); - } else { - $unlockException = $sourceCleanupException; + } elseif ($result && $this->shouldEmitHooks($source) && $this->shouldEmitHooks($target)) { + $sourcePath = $this->getHookPath($source); + $targetPath = $this->getHookPath($target); + if ($sourcePath !== null && $targetPath !== null) { + \OC_Hook::emit( + Filesystem::CLASSNAME, + Filesystem::signal_post_rename, + [ + Filesystem::signal_param_oldpath => $sourcePath, + Filesystem::signal_param_newpath => $targetPath, + ] + ); } } + } + } catch (\Throwable $e) { + $operationException ??= $e; + throw $e; + } finally { + $unlockException = null; + + try { + if ($targetLocked) { + $this->unlockFile($target, $targetLockType, true); + } + } catch (\Throwable $targetCleanupException) { + if ($operationException !== null) { + $this->logger->error('Failed to release target lock after rename operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $targetCleanupException, + ]); + } else { + $unlockException = $targetCleanupException; + } + } - if ($operationException === null && $unlockException !== null) { - throw $unlockException; + try { + if ($sourceLocked) { + $this->unlockFile($source, $sourceLockType, true); } + } catch (\Throwable $sourceCleanupException) { + if ($operationException !== null || $unlockException !== null) { + $this->logger->error('Failed to release source lock after rename operation', [ + 'app' => 'core', + 'source' => $source, + 'target' => $target, + 'exception' => $sourceCleanupException, + ]); + } else { + $unlockException = $sourceCleanupException; + } + } + + if ($operationException === null && $unlockException !== null) { + throw $unlockException; } }