From 3b34a77c5849fc248653017e01e52d00b72f10d2 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 23 Apr 2018 20:31:57 +0200 Subject: [PATCH] Make the ShareAPIController strict Fixes #9279 With types we can force php to not cast a full nummeric user to an int. Signed-off-by: Roeland Jago Douma --- .../lib/Controller/ShareAPIController.php | 79 +++++++++---------- 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 595e21825f..e89b27e5e0 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -1,4 +1,5 @@ userManager->get($share->getSharedBy()); $shareOwner = $this->userManager->get($share->getShareOwner()); @@ -233,7 +234,7 @@ class ShareAPIController extends OCSController { * @param string $property * @return string */ - private function getDisplayNameFromAddressBook($query, $property) { + private function getDisplayNameFromAddressBook(string $query, string $property): string { // FIXME: If we inject the contacts manager it gets initialized bofore any address books are registered $result = \OC::$server->getContactsManager()->search($query, [$property]); foreach ($result as $r) { @@ -256,7 +257,7 @@ class ShareAPIController extends OCSController { * @return DataResponse * @throws OCSNotFoundException */ - public function getShare($id) { + public function getShare(string $id): DataResponse { try { $share = $this->getShareById($id); } catch (ShareNotFound $e) { @@ -284,7 +285,7 @@ class ShareAPIController extends OCSController { * @return DataResponse * @throws OCSNotFoundException */ - public function deleteShare($id) { + public function deleteShare(string $id): DataResponse { try { $share = $this->getShareById($id); } catch (ShareNotFound $e) { @@ -332,14 +333,14 @@ class ShareAPIController extends OCSController { * @suppress PhanUndeclaredClassMethod */ public function createShare( - $path = null, - $permissions = null, - $shareType = -1, - $shareWith = null, - $publicUpload = 'false', - $password = '', - $expireDate = '' - ) { + string $path = null, + int $permissions = null, + int $shareType = -1, + string $shareWith = null, + string $publicUpload = 'false', + string $password = '', + string $expireDate = '' + ): DataResponse { $share = $this->shareManager->newShare(); if ($permissions === null) { @@ -384,7 +385,7 @@ class ShareAPIController extends OCSController { * We check the permissions via webdav. But the permissions of the mount point * do not equal the share permissions. Here we fix that for federated mounts. */ - if ($path->getStorage()->instanceOfStorage('OCA\Files_Sharing\External\Storage')) { + if ($path->getStorage()->instanceOfStorage(Storage::class)) { $permissions &= ~($permissions & ~$path->getPermissions()); } @@ -510,7 +511,7 @@ class ShareAPIController extends OCSController { * @param boolean $includeTags * @return DataResponse */ - private function getSharedWithMe($node = null, $includeTags) { + private function getSharedWithMe($node = null, bool $includeTags): DataResponse { $userShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_USER, $node, -1, 0); $groupShares = $this->shareManager->getSharedWith($this->currentUser, \OCP\Share::SHARE_TYPE_GROUP, $node, -1, 0); @@ -545,7 +546,7 @@ class ShareAPIController extends OCSController { * @return DataResponse * @throws OCSBadRequestException */ - private function getSharesInDir($folder) { + private function getSharesInDir(Folder $folder): DataResponse { if (!($folder instanceof \OCP\Files\Folder)) { throw new OCSBadRequestException($this->l->t('Not a directory')); } @@ -597,12 +598,12 @@ class ShareAPIController extends OCSController { * @throws OCSNotFoundException */ public function getShares( - $shared_with_me = 'false', - $reshares = 'false', - $subfiles = 'false', - $path = null, - $include_tags = 'false' - ) { + string $shared_with_me = 'false', + string $reshares = 'false', + string $subfiles = 'false', + string $path = null, + string $include_tags = 'false' + ): DataResponse { if ($path !== null) { $userFolder = $this->rootFolder->getUserFolder($this->currentUser); @@ -673,7 +674,7 @@ class ShareAPIController extends OCSController { /** * @NoAdminRequired * - * @param int $id + * @param string $id * @param int $permissions * @param string $password * @param string $publicUpload @@ -684,12 +685,12 @@ class ShareAPIController extends OCSController { * @throws OCSForbiddenException */ public function updateShare( - $id, - $permissions = null, - $password = null, - $publicUpload = null, - $expireDate = null - ) { + string $id, + int $permissions = null, + string $password = null, + string $publicUpload = null, + string $expireDate = null + ): DataResponse { try { $share = $this->getShareById($id); } catch (ShareNotFound $e) { @@ -730,7 +731,7 @@ class ShareAPIController extends OCSController { Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE, // correct Constants::PERMISSION_CREATE, // hidden file list Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE, // allow to edit single files - ]) + ], true) ) { throw new OCSBadRequestException($this->l->t('Can\'t change permissions for public share links')); } @@ -830,11 +831,7 @@ class ShareAPIController extends OCSController { return new DataResponse($this->formatShare($share)); } - /** - * @param \OCP\Share\IShare $share - * @return bool - */ - protected function canAccessShare(\OCP\Share\IShare $share, $checkGroups = true) { + protected function canAccessShare(\OCP\Share\IShare $share, bool $checkGroups = true): bool { // A file with permissions 0 can't be accessed by us. So Don't show it if ($share->getPermissions() === 0) { return false; @@ -880,7 +877,7 @@ class ShareAPIController extends OCSController { * @throws \Exception * @return \DateTime */ - private function parseDate($expireDate) { + private function parseDate(string $expireDate): \DateTime { try { $date = new \DateTime($expireDate); } catch (\Exception $e) { @@ -904,7 +901,7 @@ class ShareAPIController extends OCSController { * @return \OCP\Share\IShare * @throws ShareNotFound */ - private function getShareById($id) { + private function getShareById(string $id): IShare { $share = null; // First check if it is an internal share. @@ -946,6 +943,7 @@ class ShareAPIController extends OCSController { * Lock a Node * * @param \OCP\Files\Node $node + * @throws LockedException */ private function lock(\OCP\Files\Node $node) { $node->lock(ILockingProvider::LOCK_SHARED); @@ -954,6 +952,7 @@ class ShareAPIController extends OCSController { /** * Cleanup the remaining locks + * @throws @LockedException */ public function cleanup() { if ($this->lockedNode !== null) {