From 167f57c15e8d073506810a6c3b3cbc18f0b84c0c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 29 Jun 2015 16:45:08 +0200 Subject: [PATCH] Unlock first path on rename if second path is locked --- lib/private/files/view.php | 7 ++++++- tests/lib/files/view.php | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 0c3bc54a41..f2df2eb0f6 100644 --- a/lib/private/files/view.php +++ b/lib/private/files/view.php @@ -631,7 +631,12 @@ class View { } $this->lockFile($path1, ILockingProvider::LOCK_SHARED, true); - $this->lockFile($path2, ILockingProvider::LOCK_SHARED, true); + try { + $this->lockFile($path2, ILockingProvider::LOCK_SHARED, true); + } catch (LockedException $e) { + $this->unlockFile($path1, ILockingProvider::LOCK_SHARED); + throw $e; + } $run = true; if ($this->shouldEmitHooks() && (Cache\Scanner::isPartialFile($path1) && !Cache\Scanner::isPartialFile($path2))) { diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php index 52273c15f1..382c033f19 100644 --- a/tests/lib/files/view.php +++ b/tests/lib/files/view.php @@ -1758,6 +1758,39 @@ class View extends \Test\TestCase { $this->assertNull($this->getFileLockType($view, $targetPath), 'Target file not locked after operation'); } + /** + * Test rename operation: unlock first path when second path was locked + */ + public function testLockFileRenameUnlockOnException() { + $this->loginAsUser('test'); + + $view = new \OC\Files\View('/' . $this->user . '/files/'); + + $sourcePath = 'original.txt'; + $targetPath = 'target.txt'; + $view->file_put_contents($sourcePath, 'meh'); + + // simulate that the target path is already locked + $view->lockFile($targetPath, ILockingProvider::LOCK_EXCLUSIVE); + + $this->assertNull($this->getFileLockType($view, $sourcePath), 'Source file not locked before operation'); + $this->assertEquals(ILockingProvider::LOCK_EXCLUSIVE, $this->getFileLockType($view, $targetPath), 'Target file is locked before operation'); + + $thrown = false; + try { + $view->rename($sourcePath, $targetPath); + } catch (\OCP\Lock\LockedException $e) { + $thrown = true; + } + + $this->assertTrue($thrown, 'LockedException thrown'); + + $this->assertNull($this->getFileLockType($view, $sourcePath), 'Source file not locked after operation'); + $this->assertEquals(ILockingProvider::LOCK_EXCLUSIVE, $this->getFileLockType($view, $targetPath), 'Target file still locked after operation'); + + $view->unlockFile($targetPath, ILockingProvider::LOCK_EXCLUSIVE); + } + public function lockFileRenameOrCopyCrossStorageDataProvider() { return [ ['rename', 'moveFromStorage', ILockingProvider::LOCK_EXCLUSIVE],