Throw exception on `getPath` if file does not exist

Currently the `getPath` methods returned `NULL` in case when a file with the specified ID does not exist. This however mandates that developers are checking for the `NULL` case and if they do not the door for bugs with all kind of impact is widely opened.

This is especially harmful if used in context with Views where the final result is limited based on the result of `getPath`, if `getPath` returns `NULL` PHP type juggles this to an empty string resulting in all possible kind of bugs.

While one could argue that this is a misusage of the API the fact is that it is very often misused and an exception will trigger an immediate stop of execution as well as log this behaviour and show a pretty error page.

I also adjusted some usages where I believe that we need to catch these errors, in most cases this is though simply an error that should hard-fail.

4 files changed, 26 insertions, 11 deletions

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;
 	}
 
 	/**