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.

3 files changed, 17 insertions, 7 deletions

diff --git a/lib/private/files/filesystem.php b/lib/private/files/filesystem.php
index babf1c7d4ea..0f7508b8f26 100644
--- a/lib/private/files/filesystem.php
+++ b/lib/private/files/filesystem.php
@@ -62,6 +62,7 @@ use OC\Cache\File;
 use OC\Files\Config\MountProviderCollection;
 use OC\Files\Storage\StorageFactory;
 use OCP\Files\Config\IMountProvider;
+use OCP\Files\NotFoundException;
 use OCP\IUserManager;
 
 class Filesystem {
@@ -855,7 +856,7 @@ class Filesystem {
 	 * @param string $path
 	 * @param boolean $includeMountPoints whether to add mountpoint sizes,
 	 * defaults to true
-	 * @return \OC\Files\FileInfo
+	 * @return \OC\Files\FileInfo|bool False if file does not exist
 	 */
 	public static function getFileInfo($path, $includeMountPoints = true) {
 		return self::$defaultInstance->getFileInfo($path, $includeMountPoints);
@@ -891,6 +892,7 @@ class Filesystem {
 	 * Note that the resulting path is not guaranteed to be unique for the id, multiple paths can point to the same file
 	 *
 	 * @param int $id
+	 * @throws NotFoundException
 	 * @return string
 	 */
 	public static function getPath($id) {
diff --git a/lib/private/files/view.php b/lib/private/files/view.php
index 1639f765f69..e7328500ffb 100644
--- a/lib/private/files/view.php
+++ b/lib/private/files/view.php
@@ -48,6 +48,7 @@ use OC\Files\Mount\MoveableMount;
 use OCP\Files\FileNameTooLongException;
 use OCP\Files\InvalidCharacterInPathException;
 use OCP\Files\InvalidPathException;
+use OCP\Files\NotFoundException;
 use OCP\Files\ReservedWordException;
 use OCP\Lock\ILockingProvider;
 use OCP\Lock\LockedException;
@@ -533,6 +534,7 @@ class View {
 	 * @param string $path
 	 * @param mixed $data
 	 * @return bool|mixed
+	 * @throws \Exception
 	 */
 	public function file_put_contents($path, $data) {
 		if (is_resource($data)) { //not having to deal with streams in file_put_contents makes life easier
@@ -989,12 +991,13 @@ class View {
 	 * @param array $hooks (optional)
 	 * @param mixed $extraParam (optional)
 	 * @return mixed
+	 * @throws \Exception
 	 *
 	 * This method takes requests for basic filesystem functions (e.g. reading & writing
 	 * files), processes hooks and proxies, sanitises paths, and finally passes them on to
 	 * \OC\Files\Storage\Storage for delegation to a storage backend for execution
 	 */
-	private function basicOperation($operation, $path, $hooks = array(), $extraParam = null) {
+	private function basicOperation($operation, $path, $hooks = [], $extraParam = null) {
 		$postFix = (substr($path, -1, 1) === '/') ? '/' : '';
 		$absolutePath = Filesystem::normalizePath($this->getAbsolutePath($path));
 		if (Filesystem::isValidPath($path)
@@ -1166,7 +1169,7 @@ class View {
 	 * @param boolean|string $includeMountPoints true to add mountpoint sizes,
 	 * 'ext' to add only ext storage mount point sizes. Defaults to true.
 	 * defaults to true
-	 * @return \OC\Files\FileInfo|false
+	 * @return \OC\Files\FileInfo|bool False if file does not exist
 	 */
 	public function getFileInfo($path, $includeMountPoints = true) {
 		$this->assertPathLength($path);
@@ -1563,7 +1566,8 @@ class View {
 	 * Note that the resulting path is not guarantied to be unique for the id, multiple paths can point to the same file
 	 *
 	 * @param int $id
-	 * @return string|null
+	 * @throws NotFoundException
+	 * @return string
 	 */
 	public function getPath($id) {
 		$id = (int)$id;
@@ -1588,9 +1592,13 @@ class View {
 				}
 			}
 		}
-		return null;
+		throw new NotFoundException(sprintf('File with id "%s" has not been found.', $id));
 	}
 
+	/**
+	 * @param string $path
+	 * @throws InvalidPathException
+	 */
 	private function assertPathLength($path) {
 		$maxLen = min(PHP_MAXPATHLEN, 4000);
 		// Check for the string length - performed using isset() instead of strlen()
diff --git a/lib/private/hook.php b/lib/private/hook.php
index faf5495b646..68621580b16 100644
--- a/lib/private/hook.php
+++ b/lib/private/hook.php
@@ -78,12 +78,12 @@ class OC_Hook{
 	 * @param string $signalName name of signal
 	 * @param mixed $params default: array() array with additional data
 	 * @return bool true if slots exists or false if not
-	 *
+	 * @throws \OC\ServerNotAvailableException
 	 * Emits a signal. To get data from the slot use references!
 	 *
 	 * TODO: write example
 	 */
-	static public function emit($signalClass, $signalName, $params = array()) {
+	static public function emit($signalClass, $signalName, $params = []) {
 
 		// Return false if no hook handlers are listening to this
 		// emitting class