From 62bc53143e7f6b13fad1d0bb9e899a3715d55e49 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 9 Mar 2016 09:11:41 +0100 Subject: [PATCH 1/2] Add locking to modifying operation of the OCS Share API Fixes #17243 This is done in the OCS Share API instead of the share manager since we want lazy shares in general. However when doing modifying calls via the OCS Share API it is fine to force real nodes. * Updated unit tests to work with logging --- apps/files_sharing/api/share20ocs.php | 21 +- .../tests/api/share20ocstest.php | 193 ++++++++++++++---- 2 files changed, 175 insertions(+), 39 deletions(-) diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php index 6809853001..8b2c413b6f 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -28,6 +28,7 @@ use OCP\IRequest; use OCP\IURLGenerator; use OCP\IUser; use OCP\Files\IRootFolder; +use OCP\Lock\LockedException; use OCP\Share; use OCP\Share\IManager; @@ -205,12 +206,20 @@ class Share20OCS { return new \OC_OCS_Result(null, 404, $this->l->t('Wrong share ID, share doesn\'t exist')); } + try { + $share->getNode()->lock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + } catch (LockedException $e) { + return new \OC_OCS_Result(null, 404, 'could not delete share'); + } + if (!$this->canAccessShare($share)) { return new \OC_OCS_Result(null, 404, $this->l->t('Could not delete share')); } $this->shareManager->deleteShare($share); + $share->getNode()->unlock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + return new \OC_OCS_Result(); } @@ -238,6 +247,7 @@ class Share20OCS { } $share->setNode($path); + $share->getNode()->lock(\OCP\Lock\ILockingProvider::LOCK_SHARED); // Parse permissions (if available) $permissions = $this->request->getParam('permissions', null); @@ -368,8 +378,11 @@ class Share20OCS { return new \OC_OCS_Result(null, 403, $e->getMessage()); } - $share = $this->formatShare($share); - return new \OC_OCS_Result($share); + $output = $this->formatShare($share); + + $share->getNode()->unlock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + return new \OC_OCS_Result($output); } /** @@ -512,6 +525,8 @@ class Share20OCS { return new \OC_OCS_Result(null, 404, $this->l->t('Wrong share ID, share doesn\'t exist')); } + $share->getNode()->lock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + if (!$this->canAccessShare($share)) { return new \OC_OCS_Result(null, 404, $this->l->t('Wrong share ID, share doesn\'t exist')); } @@ -611,6 +626,8 @@ class Share20OCS { return new \OC_OCS_Result(null, 400, $e->getMessage()); } + $share->getNode()->unlock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + return new \OC_OCS_Result($this->formatShare($share)); } diff --git a/apps/files_sharing/tests/api/share20ocstest.php b/apps/files_sharing/tests/api/share20ocstest.php index 56c350aa99..ffb74da2af 100644 --- a/apps/files_sharing/tests/api/share20ocstest.php +++ b/apps/files_sharing/tests/api/share20ocstest.php @@ -29,6 +29,7 @@ use OCP\IRequest; use OCP\IURLGenerator; use OCP\IUser; use OCP\Files\IRootFolder; +use OCP\Lock\LockedException; /** * Class Share20OCSTest @@ -137,8 +138,11 @@ class Share20OCSTest extends \Test\TestCase { } public function testDeleteShare() { + $node = $this->getMock('\OCP\Files\File'); + $share = $this->newShare(); - $share->setSharedBy($this->currentUser->getUID()); + $share->setSharedBy($this->currentUser->getUID()) + ->setNode($node); $this->shareManager ->expects($this->once()) ->method('getShareById') @@ -149,10 +153,45 @@ class Share20OCSTest extends \Test\TestCase { ->method('deleteShare') ->with($share); + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $node->expects($this->once()) + ->method('unlock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $expected = new \OC_OCS_Result(); $this->assertEquals($expected, $this->ocs->deleteShare(42)); } + public function testDeleteShareLocked() { + $node = $this->getMock('\OCP\Files\File'); + + $share = $this->newShare(); + $share->setSharedBy($this->currentUser->getUID()) + ->setNode($node); + $this->shareManager + ->expects($this->once()) + ->method('getShareById') + ->with('ocinternal:42') + ->willReturn($share); + $this->shareManager + ->expects($this->never()) + ->method('deleteShare') + ->with($share); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED) + ->will($this->throwException(new LockedException('mypath'))); + $node->expects($this->never()) + ->method('unlock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $expected = new \OC_OCS_Result(null, 404, 'could not delete share'); + $this->assertEquals($expected, $this->ocs->deleteShare(42)); + } + /* * FIXME: Enable once we have a federated Share Provider @@ -526,7 +565,7 @@ class Share20OCSTest extends \Test\TestCase { } public function testCreateShareInvalidPermissions() { - $share = $this->getMock('\OCP\Share\IShare'); + $share = $this->newShare(); $this->shareManager->method('newShare')->willReturn($share); $this->request @@ -548,6 +587,10 @@ class Share20OCSTest extends \Test\TestCase { ->with('valid-path') ->willReturn($path); + $path->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $expected = new \OC_OCS_Result(null, 404, 'invalid permissions'); $result = $this->ocs->createShare(); @@ -557,7 +600,7 @@ class Share20OCSTest extends \Test\TestCase { } public function testCreateShareUserNoShareWith() { - $share = $this->getMock('\OCP\Share\IShare'); + $share = $this->newShare(); $this->shareManager->method('newShare')->willReturn($share); $this->request @@ -585,6 +628,10 @@ class Share20OCSTest extends \Test\TestCase { ->with('valid-path') ->willReturn($path); + $path->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $expected = new \OC_OCS_Result(null, 404, 'Please specify a valid user'); $result = $this->ocs->createShare(); @@ -594,7 +641,7 @@ class Share20OCSTest extends \Test\TestCase { } public function testCreateShareUserNoValidShareWith() { - $share = $this->getMock('\OCP\Share\IShare'); + $share = $this->newShare(); $this->shareManager->method('newShare')->willReturn($share); $this->request @@ -625,6 +672,10 @@ class Share20OCSTest extends \Test\TestCase { $expected = new \OC_OCS_Result(null, 404, 'Please specify a valid user'); + $path->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $result = $this->ocs->createShare(); $this->assertEquals($expected->getMeta(), $result->getMeta()); @@ -632,9 +683,8 @@ class Share20OCSTest extends \Test\TestCase { } public function testCreateShareUser() { - $share = $this->getMock('\OCP\Share\IShare'); + $share = $this->newShare(); $this->shareManager->method('newShare')->willReturn($share); - $this->shareManager->method('createShare')->will($this->returnArgument(0)); $ocs = $this->getMockBuilder('OCA\Files_Sharing\API\Share20OCS') ->setConstructorArgs([ @@ -677,15 +727,26 @@ class Share20OCSTest extends \Test\TestCase { $this->userManager->method('userExists')->with('validUser')->willReturn(true); - $share->method('setNode')->with($path); - $share->method('setPermissions') - ->with( - \OCP\Constants::PERMISSION_ALL & - ~\OCP\Constants::PERMISSION_DELETE & - ~\OCP\Constants::PERMISSION_CREATE); - $share->method('setShareType')->with(\OCP\Share::SHARE_TYPE_USER); - $share->method('setSharedWith')->with('validUser'); - $share->method('setSharedBy')->with('currentUser'); + $path->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $path->expects($this->once()) + ->method('unlock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $this->shareManager->method('createShare') + ->with($this->callback(function (\OCP\Share\IShare $share) use ($path) { + return $share->getNode() === $path && + $share->getPermissions() === ( + \OCP\Constants::PERMISSION_ALL & + ~\OCP\Constants::PERMISSION_DELETE & + ~\OCP\Constants::PERMISSION_CREATE + ) && + $share->getShareType() === \OCP\Share::SHARE_TYPE_USER && + $share->getSharedWith() === 'validUser' && + $share->getSharedBy() === 'currentUser'; + })) + ->will($this->returnArgument(0)); $expected = new \OC_OCS_Result(); $result = $ocs->createShare(); @@ -695,7 +756,7 @@ class Share20OCSTest extends \Test\TestCase { } public function testCreateShareGroupNoValidShareWith() { - $share = $this->getMock('\OCP\Share\IShare'); + $share = $this->newShare(); $this->shareManager->method('newShare')->willReturn($share); $this->shareManager->method('createShare')->will($this->returnArgument(0)); @@ -727,6 +788,10 @@ class Share20OCSTest extends \Test\TestCase { $expected = new \OC_OCS_Result(null, 404, 'Please specify a valid user'); + $path->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $result = $this->ocs->createShare(); $this->assertEquals($expected->getMeta(), $result->getMeta()); @@ -734,9 +799,8 @@ class Share20OCSTest extends \Test\TestCase { } public function testCreateShareGroup() { - $share = $this->getMock('\OCP\Share\IShare'); + $share = $this->newShare(); $this->shareManager->method('newShare')->willReturn($share); - $this->shareManager->method('createShare')->will($this->returnArgument(0)); $ocs = $this->getMockBuilder('OCA\Files_Sharing\API\Share20OCS') ->setConstructorArgs([ @@ -783,11 +847,22 @@ class Share20OCSTest extends \Test\TestCase { ->method('allowGroupSharing') ->willReturn(true); - $share->method('setNode')->with($path); - $share->method('setPermissions')->with(\OCP\Constants::PERMISSION_ALL); - $share->method('setShareType')->with(\OCP\Share::SHARE_TYPE_GROUP); - $share->method('setSharedWith')->with('validGroup'); - $share->method('setSharedBy')->with('currentUser'); + $path->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $path->expects($this->once()) + ->method('unlock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + $this->shareManager->method('createShare') + ->with($this->callback(function (\OCP\Share\IShare $share) use ($path) { + return $share->getNode() === $path && + $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && + $share->getShareType() === \OCP\Share::SHARE_TYPE_GROUP && + $share->getSharedWith() === 'validGroup' && + $share->getSharedBy() === 'currentUser'; + })) + ->will($this->returnArgument(0)); $expected = new \OC_OCS_Result(); $result = $ocs->createShare(); @@ -797,7 +872,7 @@ class Share20OCSTest extends \Test\TestCase { } public function testCreateShareGroupNotAllowed() { - $share = $this->getMock('\OCP\Share\IShare'); + $share = $this->newShare(); $this->shareManager->method('newShare')->willReturn($share); $this->request @@ -832,9 +907,8 @@ class Share20OCSTest extends \Test\TestCase { ->method('allowGroupSharing') ->willReturn(false); - $share->method('setNode')->with($path); - $expected = new \OC_OCS_Result(null, 404, 'Group sharing is disabled by the administrator'); + $result = $this->ocs->createShare(); $this->assertEquals($expected->getMeta(), $result->getMeta()); @@ -1154,7 +1228,13 @@ class Share20OCSTest extends \Test\TestCase { } public function testUpdateShareCantAccess() { - $share = \OC::$server->getShareManager()->newShare(); + $node = $this->getMock('\OCP\Files\Folder'); + $share = $this->newShare(); + $share->setNode($node); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); @@ -1166,10 +1246,16 @@ class Share20OCSTest extends \Test\TestCase { } public function testUpdateNoParametersLink() { - $share = \OC::$server->getShareManager()->newShare(); + $node = $this->getMock('\OCP\Files\Folder'); + $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser->getUID()) - ->setShareType(\OCP\Share::SHARE_TYPE_LINK); + ->setShareType(\OCP\Share::SHARE_TYPE_LINK) + ->setNode($node); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); @@ -1181,10 +1267,16 @@ class Share20OCSTest extends \Test\TestCase { } public function testUpdateNoParametersOther() { - $share = \OC::$server->getShareManager()->newShare(); + $node = $this->getMock('\OCP\Files\Folder'); + $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser->getUID()) - ->setShareType(\OCP\Share::SHARE_TYPE_GROUP); + ->setShareType(\OCP\Share::SHARE_TYPE_GROUP) + ->setNode($node); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); @@ -1198,13 +1290,22 @@ class Share20OCSTest extends \Test\TestCase { public function testUpdateLinkShareClear() { $ocs = $this->mockFormatShare(); - $share = \OC::$server->getShareManager()->newShare(); + $node = $this->getMock('\OCP\Files\Folder'); + $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser->getUID()) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') ->setExpirationDate(new \DateTime()) - ->setPermissions(\OCP\Constants::PERMISSION_ALL); + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setNode($node); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $node->expects($this->once()) + ->method('unlock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); $this->request ->method('getParam') @@ -1364,13 +1465,22 @@ class Share20OCSTest extends \Test\TestCase { $date = new \DateTime('2000-01-01'); $date->setTime(0,0,0); - $share = \OC::$server->getShareManager()->newShare(); + $node = $this->getMock('\OCP\Files\File'); + $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser->getUID()) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') ->setExpirationDate($date) - ->setPermissions(\OCP\Constants::PERMISSION_ALL); + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setNode($node); + + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $node->expects($this->once()) + ->method('unlock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); $this->request ->method('getParam') @@ -1398,13 +1508,15 @@ class Share20OCSTest extends \Test\TestCase { public function testUpdateLinkShareExpireDateDoesNotChangeOther() { $ocs = $this->mockFormatShare(); - $share = \OC::$server->getShareManager()->newShare(); + $node = $this->getMock('\OCP\Files\File'); + $share = $this->newShare(); $share->setPermissions(\OCP\Constants::PERMISSION_ALL) ->setSharedBy($this->currentUser->getUID()) ->setShareType(\OCP\Share::SHARE_TYPE_LINK) ->setPassword('password') ->setExpirationDate(new \DateTime()) - ->setPermissions(\OCP\Constants::PERMISSION_ALL); + ->setPermissions(\OCP\Constants::PERMISSION_ALL) + ->setNode($node); $this->request ->method('getParam') @@ -1412,6 +1524,13 @@ class Share20OCSTest extends \Test\TestCase { ['expireDate', null, '2010-12-23'], ])); + $node->expects($this->once()) + ->method('lock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $node->expects($this->once()) + ->method('unlock') + ->with(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share); $this->shareManager->expects($this->once())->method('updateShare')->with( From 8a652d12380d47a77fca692fb74a7209eafa5dcd Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 17 Mar 2016 11:09:11 +0100 Subject: [PATCH 2/2] Unlock before all returns --- apps/files_sharing/api/share20ocs.php | 57 +++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/apps/files_sharing/api/share20ocs.php b/apps/files_sharing/api/share20ocs.php index 8b2c413b6f..af76284532 100644 --- a/apps/files_sharing/api/share20ocs.php +++ b/apps/files_sharing/api/share20ocs.php @@ -31,9 +31,9 @@ use OCP\Files\IRootFolder; use OCP\Lock\LockedException; use OCP\Share; use OCP\Share\IManager; - use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\Exceptions\GenericShareException; +use OCP\Lock\ILockingProvider; /** * Class Share20OCS @@ -207,18 +207,19 @@ class Share20OCS { } try { - $share->getNode()->lock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $share->getNode()->lock(ILockingProvider::LOCK_SHARED); } catch (LockedException $e) { return new \OC_OCS_Result(null, 404, 'could not delete share'); } if (!$this->canAccessShare($share)) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, $this->l->t('Could not delete share')); } $this->shareManager->deleteShare($share); - $share->getNode()->unlock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(); } @@ -242,12 +243,17 @@ class Share20OCS { $userFolder = $this->rootFolder->getUserFolder($this->currentUser->getUID()); try { $path = $userFolder->get($path); - } catch (\OCP\Files\NotFoundException $e) { + } catch (NotFoundException $e) { return new \OC_OCS_Result(null, 404, $this->l->t('Wrong path, file/folder doesn\'t exist')); } $share->setNode($path); - $share->getNode()->lock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + + try { + $share->getNode()->lock(ILockingProvider::LOCK_SHARED); + } catch (LockedException $e) { + return new \OC_OCS_Result(null, 404, 'Could not create share'); + } // Parse permissions (if available) $permissions = $this->request->getParam('permissions', null); @@ -258,6 +264,7 @@ class Share20OCS { } if ($permissions < 0 || $permissions > \OCP\Constants::PERMISSION_ALL) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, 'invalid permissions'); } @@ -285,17 +292,20 @@ class Share20OCS { if ($shareType === \OCP\Share::SHARE_TYPE_USER) { // Valid user is required to share if ($shareWith === null || !$this->userManager->userExists($shareWith)) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, $this->l->t('Please specify a valid user')); } $share->setSharedWith($shareWith); $share->setPermissions($permissions); } else if ($shareType === \OCP\Share::SHARE_TYPE_GROUP) { if (!$this->shareManager->allowGroupSharing()) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, $this->l->t('Group sharing is disabled by the administrator')); } // Valid group is required to share if ($shareWith === null || !$this->groupManager->groupExists($shareWith)) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, $this->l->t('Please specify a valid group')); } $share->setSharedWith($shareWith); @@ -303,6 +313,7 @@ class Share20OCS { } else if ($shareType === \OCP\Share::SHARE_TYPE_LINK) { //Can we even share links? if (!$this->shareManager->shareApiAllowLinks()) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, $this->l->t('Public link sharing is disabled by the administrator')); } @@ -312,6 +323,7 @@ class Share20OCS { */ $existingShares = $this->shareManager->getSharesBy($this->currentUser->getUID(), \OCP\Share::SHARE_TYPE_LINK, $path, false, 1, 0); if (!empty($existingShares)) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result($this->formatShare($existingShares[0])); } @@ -319,11 +331,13 @@ class Share20OCS { if ($publicUpload === 'true') { // Check if public upload is allowed if (!$this->shareManager->shareApiLinkAllowPublicUpload()) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 403, $this->l->t('Public upload disabled by the administrator')); } // Public upload can only be set for folders if ($path instanceof \OCP\Files\File) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, $this->l->t('Public upload is only possible for publicly shared folders')); } @@ -351,18 +365,21 @@ class Share20OCS { $expireDate = $this->parseDate($expireDate); $share->setExpirationDate($expireDate); } catch (\Exception $e) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, $this->l->t('Invalid date, date format must be YYYY-MM-DD')); } } } else if ($shareType === \OCP\Share::SHARE_TYPE_REMOTE) { if (!$this->shareManager->outgoingServer2ServerSharesAllowed()) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 403, $this->l->t('Sharing %s failed because the back end does not allow shares from type %s', [$path->getPath(), $shareType])); } $share->setSharedWith($shareWith); $share->setPermissions($permissions); } else { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 400, $this->l->t('Unknown share type')); } @@ -373,8 +390,10 @@ class Share20OCS { $share = $this->shareManager->createShare($share); } catch (GenericShareException $e) { $code = $e->getCode() === 0 ? 403 : $e->getCode(); + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, $code, $e->getHint()); }catch (\Exception $e) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 403, $e->getMessage()); } @@ -467,17 +486,28 @@ class Share20OCS { $userFolder = $this->rootFolder->getUserFolder($this->currentUser->getUID()); try { $path = $userFolder->get($path); + $path->lock(ILockingProvider::LOCK_SHARED); } catch (\OCP\Files\NotFoundException $e) { return new \OC_OCS_Result(null, 404, $this->l->t('Wrong path, file/folder doesn\'t exist')); + } catch (LockedException $e) { + return new \OC_OCS_Result(null, 404, $this->l->t('Could not lock path')); } } if ($sharedWithMe === 'true') { - return $this->getSharedWithMe($path); + $result = $this->getSharedWithMe($path); + if ($path !== null) { + $path->unlock(ILockingProvider::LOCK_SHARED); + } + return $result; } if ($subfiles === 'true') { - return $this->getSharesInDir($path); + $result = $this->getSharesInDir($path); + if ($path !== null) { + $path->unlock(ILockingProvider::LOCK_SHARED); + } + return $result; } if ($reshares === 'true') { @@ -507,6 +537,10 @@ class Share20OCS { } } + if ($path !== null) { + $path->unlock(ILockingProvider::LOCK_SHARED); + } + return new \OC_OCS_Result($formatted); } @@ -528,6 +562,7 @@ class Share20OCS { $share->getNode()->lock(\OCP\Lock\ILockingProvider::LOCK_SHARED); if (!$this->canAccessShare($share)) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, $this->l->t('Wrong share ID, share doesn\'t exist')); } @@ -541,6 +576,7 @@ class Share20OCS { */ if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) { if ($permissions === null && $password === null && $publicUpload === null && $expireDate === null) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 400, 'Wrong or no update parameter given'); } @@ -558,15 +594,18 @@ class Share20OCS { if ($newPermissions !== null && $newPermissions !== \OCP\Constants::PERMISSION_READ && $newPermissions !== (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE)) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 400, $this->l->t('Can\'t change permissions for public share links')); } if ($newPermissions === (\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE)) { if (!$this->shareManager->shareApiLinkAllowPublicUpload()) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 403, $this->l->t('Public upload disabled by the administrator')); } if (!($share->getNode() instanceof \OCP\Files\Folder)) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 400, $this->l->t('Public upload is only possible for publicly shared folders')); } } @@ -581,6 +620,7 @@ class Share20OCS { try { $expireDate = $this->parseDate($expireDate); } catch (\Exception $e) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 400, $e->getMessage()); } $share->setExpirationDate($expireDate); @@ -595,6 +635,7 @@ class Share20OCS { } else { // For other shares only permissions is valid. if ($permissions === null) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 400, $this->l->t('Wrong or no update parameter given')); } else { $permissions = (int)$permissions; @@ -614,6 +655,7 @@ class Share20OCS { } if ($share->getPermissions() & ~$maxPermissions) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 404, $this->l->t('Cannot increase permissions')); } } @@ -623,6 +665,7 @@ class Share20OCS { try { $share = $this->shareManager->updateShare($share); } catch (\Exception $e) { + $share->getNode()->unlock(ILockingProvider::LOCK_SHARED); return new \OC_OCS_Result(null, 400, $e->getMessage()); }