Merge pull request #16957 from owncloud/throw-exception-if-file-does-not-exist

Throw exception on `getPath` if file does not exist

10 files changed, 88 insertions, 47 deletions

diff --git a/apps/files_sharing/api/server2server.php b/apps/files_sharing/api/server2server.php
index 2e0468039b4..832193f2b99 100644
--- a/apps/files_sharing/api/server2server.php
+++ b/apps/files_sharing/api/server2server.php
@@ -25,6 +25,7 @@
 namespace OCA\Files_Sharing\API;
 
 use OCA\Files_Sharing\Activity;
+use OCP\Files\NotFoundException;
 
 class Server2Server {
 
@@ -264,7 +265,11 @@ class Server2Server {
 	private function getFile($user, $fileSource) {
 		\OC_Util::setupFS($user);
 
-		$file = \OC\Files\Filesystem::getPath($fileSource);
+		try {
+			$file = \OC\Files\Filesystem::getPath($fileSource);
+		} catch (NotFoundException $e) {
+			$file = null;
+		}
 		$args = \OC\Files\Filesystem::is_dir($file) ? array('dir' => $file) : array('dir' => dirname($file), 'scrollto' => $file);
 		$link = \OCP\Util::linkToAbsolute('files', 'index.php', $args);
 
diff --git a/apps/files_sharing/lib/controllers/sharecontroller.php b/apps/files_sharing/lib/controllers/sharecontroller.php
index 616b64e6c59..8a4e67eccf6 100644
--- a/apps/files_sharing/lib/controllers/sharecontroller.php
+++ b/apps/files_sharing/lib/controllers/sharecontroller.php
@@ -333,8 +333,7 @@ class ShareController extends Controller {
 				OC_Util::tearDownFS();
 				OC_Util::setupFS($rootLinkItem['uid_owner']);
 				$path = Filesystem::getPath($linkItem['file_source']);
-
-				if(!empty($path) && Filesystem::isReadable($path)) {
+				if(Filesystem::isReadable($path)) {
 					return $path;
 				}
 			}
diff --git a/apps/files_sharing/lib/helper.php b/apps/files_sharing/lib/helper.php
index b15f70fcde3..beb8c69c415 100644
--- a/apps/files_sharing/lib/helper.php
+++ b/apps/files_sharing/lib/helper.php
@@ -28,6 +28,8 @@
  */
 namespace OCA\Files_Sharing;
 
+use OCP\Files\NotFoundException;
+
 class Helper {
 
 	public static function registerHooks() {
@@ -48,6 +50,7 @@ class Helper {
 	 * @param string $token string share token
 	 * @param string $relativePath optional path relative to the share
 	 * @param string $password optional password
+	 * @return array
 	 */
 	public static function setupFromToken($token, $relativePath = null, $password = null) {
 		\OC_User::setIncognitoMode(true);
@@ -71,10 +74,11 @@ class Helper {
 			\OCP\JSON::checkUserExists($rootLinkItem['uid_owner']);
 			\OC_Util::tearDownFS();
 			\OC_Util::setupFS($rootLinkItem['uid_owner']);
-			$path = \OC\Files\Filesystem::getPath($linkItem['file_source']);
 		}
 
-		if ($path === null) {
+		try {
+			$path = \OC\Files\Filesystem::getPath($linkItem['file_source']);
+		} catch (NotFoundException $e) {
 			\OCP\Util::writeLog('share', 'could not resolve linkItem', \OCP\Util::DEBUG);
 			\OC_Response::setStatus(404);
 			\OCP\JSON::error(array('success' => false));
diff --git a/apps/files_sharing/lib/propagation/recipientpropagator.php b/apps/files_sharing/lib/propagation/recipientpropagator.php
index 8e064e2ee54..f0941ce6cb7 100644
--- a/apps/files_sharing/lib/propagation/recipientpropagator.php
+++ b/apps/files_sharing/lib/propagation/recipientpropagator.php
@@ -25,6 +25,7 @@ namespace OCA\Files_Sharing\Propagation;
 use OC\Files\Cache\ChangePropagator;
 use OC\Files\View;
 use OC\Share\Share;
+use OCP\Files\NotFoundException;
 
 /**
  * Propagate etags for share recipients
@@ -128,6 +129,9 @@ class RecipientPropagator {
 
 	protected $propagatingIds = [];
 
+	/**
+	 * @param int $id
+	 */
 	public function propagateById($id) {
 		if (isset($this->propagatingIds[$id])) {
 			return;
@@ -142,7 +146,13 @@ class RecipientPropagator {
 			if ($share['share_with'] === $this->userId) {
 				$user = $share['uid_owner'];
 				$view = new View('/' . $user . '/files');
-				$path = $view->getPath($share['file_source']);
+
+				try {
+					$path = $view->getPath($share['file_source']);
+				} catch (NotFoundException $e) {
+					$path = null;
+				}
+
 				$watcher = new ChangeWatcher($view, $this->manager->getSharePropagator($user));
 				$watcher->writeHook(['path' => $path]);
 			}
diff --git a/apps/files_sharing/lib/share/file.php b/apps/files_sharing/lib/share/file.php
index 6c676f47a0c..67ef3937e6d 100644
--- a/apps/files_sharing/lib/share/file.php
+++ b/apps/files_sharing/lib/share/file.php
@@ -40,15 +40,16 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent {
 	private $path;
 
 	public function isValidSource($itemSource, $uidOwner) {
-		$path = \OC\Files\Filesystem::getPath($itemSource);
-		if ($path) {
+		try {
+			$path = \OC\Files\Filesystem::getPath($itemSource);
 			// FIXME: attributes should not be set here,
 			// keeping this pattern for now to avoid unexpected
 			// regressions
 			$this->path = \OC\Files\Filesystem::normalizePath(basename($path));
 			return true;
+		} catch (\OCP\Files\NotFoundException $e) {
+			return false;
 		}
-		return false;
 	}
 
 	public function getFilePath($itemSource, $uidOwner) {
@@ -57,12 +58,13 @@ class OC_Share_Backend_File implements OCP\Share_Backend_File_Dependent {
 			$this->path = null;
 			return $path;
 		} else {
-			$path = \OC\Files\Filesystem::getPath($itemSource);
-			if ($path) {
+			try {
+				$path = \OC\Files\Filesystem::getPath($itemSource);
 				return $path;
+			} catch (\OCP\Files\NotFoundException $e) {
+				return false;
 			}
 		}
-		return false;
 	}
 
 	/**
diff --git a/apps/files_sharing/tests/controller/sharecontroller.php b/apps/files_sharing/tests/controller/sharecontroller.php
index 0b56aafc8a9..db5eb75d761 100644
--- a/apps/files_sharing/tests/controller/sharecontroller.php
+++ b/apps/files_sharing/tests/controller/sharecontroller.php
@@ -199,8 +199,7 @@ class ShareControllerTest extends \Test\TestCase {
 	}
 
 	/**
-	 * @expectedException \Exception
-	 * @expectedExceptionMessage No file found belonging to file.
+	 * @expectedException \OCP\Files\NotFoundException
 	 */
 	public function testShowShareWithDeletedFile() {
 		$this->container['UserManager']->expects($this->once())
@@ -216,8 +215,7 @@ class ShareControllerTest extends \Test\TestCase {
 	}
 
 	/**
-	 * @expectedException \Exception
-	 * @expectedExceptionMessage No file found belonging to file.
+	 * @expectedException \OCP\Files\NotFoundException
 	 */
 	public function testDownloadShareWithDeletedFile() {
 		$this->container['UserManager']->expects($this->once())
diff --git a/apps/files_sharing/tests/etagpropagation.php b/apps/files_sharing/tests/etagpropagation.php
index 2490fef03e1..9f9b65ed527 100644
--- a/apps/files_sharing/tests/etagpropagation.php
+++ b/apps/files_sharing/tests/etagpropagation.php
@@ -173,7 +173,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsChanged($users, 'sub1/sub2');
 	}
 
-	private function assertAllUnchaged() {
+	private function assertAllUnchanged() {
 		$users = [self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4];
 		$this->assertEtagsNotChanged($users);
@@ -186,7 +186,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testOwnerWritesToSingleFileShare() {
@@ -195,7 +195,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsNotChanged([self::TEST_FILES_SHARING_API_USER4, self::TEST_FILES_SHARING_API_USER3]);
 		$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testOwnerWritesToShareWithReshare() {
@@ -204,7 +204,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testOwnerRenameInShare() {
@@ -214,7 +214,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testOwnerRenameInReShare() {
@@ -223,7 +223,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testOwnerRenameIntoReShare() {
@@ -232,7 +232,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testOwnerRenameOutOfReShare() {
@@ -241,7 +241,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testOwnerDeleteInShare() {
@@ -251,7 +251,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testOwnerDeleteInReShare() {
@@ -260,7 +260,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testOwnerUnshares() {
@@ -283,7 +283,7 @@ class EtagPropagation extends TestCase {
 			self::TEST_FILES_SHARING_API_USER4,
 		]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientUnsharesFromSelf() {
@@ -298,7 +298,7 @@ class EtagPropagation extends TestCase {
 			self::TEST_FILES_SHARING_API_USER4,
 		]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientWritesToShare() {
@@ -308,7 +308,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientWritesToReshare() {
@@ -317,7 +317,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientWritesToOtherRecipientsReshare() {
@@ -326,7 +326,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientRenameInShare() {
@@ -336,7 +336,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientRenameInReShare() {
@@ -345,7 +345,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientRenameResharedFolder() {
@@ -356,7 +356,7 @@ class EtagPropagation extends TestCase {
 
 		$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER2], 'sub1');
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientDeleteInShare() {
@@ -366,7 +366,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientDeleteInReShare() {
@@ -375,7 +375,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testReshareRecipientWritesToReshare() {
@@ -384,7 +384,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testReshareRecipientRenameInReShare() {
@@ -393,7 +393,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testReshareRecipientDeleteInReShare() {
@@ -402,7 +402,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2,
 			self::TEST_FILES_SHARING_API_USER3, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testRecipientUploadInDirectReshare() {
@@ -411,7 +411,7 @@ class EtagPropagation extends TestCase {
 		$this->assertEtagsNotChanged([self::TEST_FILES_SHARING_API_USER3]);
 		$this->assertEtagsChanged([self::TEST_FILES_SHARING_API_USER1, self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 
 	public function testEtagChangeOnPermissionsChange() {
@@ -424,6 +424,6 @@ class EtagPropagation extends TestCase {
 
 		$this->assertEtagsForFoldersChanged([self::TEST_FILES_SHARING_API_USER2, self::TEST_FILES_SHARING_API_USER4]);
 
-		$this->assertAllUnchaged();
+		$this->assertAllUnchanged();
 	}
 }
diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php
index 839a47a7bf2..9eada31f5df 100644
--- a/apps/files_trashbin/lib/trashbin.php
+++ b/apps/files_trashbin/lib/trashbin.php
@@ -41,6 +41,7 @@ use OC\Files\Filesystem;
 use OC\Files\View;
 use OCA\Files_Trashbin\AppInfo\Application;
 use OCA\Files_Trashbin\Command\Expire;
+use OCP\Files\NotFoundException;
 
 class Trashbin {
 
@@ -64,15 +65,24 @@ class Trashbin {
 		self::getUidAndFilename($params['path']);
 	}
 
+	/**
+	 * @param string $filename
+	 * @return array
+	 * @throws \OC\User\NoUserException
+	 */
 	public static function getUidAndFilename($filename) {
 		$uid = \OC\Files\Filesystem::getOwner($filename);
 		\OC\Files\Filesystem::initMountPoints($uid);
 		if ($uid != \OCP\User::getUser()) {
 			$info = \OC\Files\Filesystem::getFileInfo($filename);
 			$ownerView = new \OC\Files\View('/' . $uid . '/files');
-			$filename = $ownerView->getPath($info['fileid']);
+			try {
+				$filename = $ownerView->getPath($info['fileid']);
+			} catch (NotFoundException $e) {
+				$filename = null;
+			}
 		}
-		return array($uid, $filename);
+		return [$uid, $filename];
 	}
 
 	/**
diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php
index dd8af1b8d18..6737bf20f90 100644
--- a/apps/files_versions/lib/storage.php
+++ b/apps/files_versions/lib/storage.php
@@ -44,6 +44,7 @@ namespace OCA\Files_Versions;
 use OCA\Files_Versions\AppInfo\Application;
 use OCA\Files_Versions\Command\Expire;
 use OCP\Lock\ILockingProvider;
+use OCP\Files\NotFoundException;
 
 class Storage {
 
@@ -74,15 +75,24 @@ class Storage {
 	/** @var \OCA\Files_Versions\AppInfo\Application */
 	private static $application;
 
+	/**
+	 * @param string $filename
+	 * @return array
+	 * @throws \OC\User\NoUserException
+	 */
 	public static function getUidAndFilename($filename) {
 		$uid = \OC\Files\Filesystem::getOwner($filename);
 		\OC\Files\Filesystem::initMountPoints($uid);
 		if ( $uid != \OCP\User::getUser() ) {
 			$info = \OC\Files\Filesystem::getFileInfo($filename);
 			$ownerView = new \OC\Files\View('/'.$uid.'/files');
-			$filename = $ownerView->getPath($info['fileid']);
+			try {
+				$filename = $ownerView->getPath($info['fileid']);
+			} catch (NotFoundException $e) {
+				$filename = null;
+			}
 		}
-		return array($uid, $filename);
+		return [$uid, $filename];
 	}
 
 	/**
diff --git a/apps/files_versions/tests/versions.php b/apps/files_versions/tests/versions.php
index 2979de2ac98..b9bc0932a84 100644
--- a/apps/files_versions/tests/versions.php
+++ b/apps/files_versions/tests/versions.php
@@ -759,7 +759,11 @@ class Test_Files_Versioning extends \Test\TestCase {
 		);
 	}
 
-	private function createAndCheckVersions($view, $path) {
+	/**
+	 * @param \OC\Files\View $view
+	 * @param string $path
+	 */
+	private function createAndCheckVersions(\OC\Files\View $view, $path) {
 		$view->file_put_contents($path, 'test file');
 		$view->file_put_contents($path, 'version 1');
 		$view->file_put_contents($path, 'version 2');
@@ -782,7 +786,6 @@ class Test_Files_Versioning extends \Test\TestCase {
 	/**
 	 * @param string $user
 	 * @param bool $create
-	 * @param bool $password
 	 */
 	public static function loginHelper($user, $create = false) {