Prevent wrapping null storage

Can happen when trying to instantiate external storages that have
incomplete config, where the constructor throws an exception (the
exception is caught in createStorage())

lib/private/files/mount/mountpoint.php

@@ -41,6 +41,14 @@ class MountPoint implements IMountPoint {
 	 */
 	private $loader;
 
+	/**
+	 * Specified whether the storage is invalid after failing to
+	 * instantiate it.
+	 *
+	 * @var bool
+	 */
+	private $invalidStorage = false;
+
 	/**
 	 * @param string|\OC\Files\Storage\Storage $storage
 	 * @param string $mountpoint
@@ -99,10 +107,15 @@ class MountPoint implements IMountPoint {
 	 * @return \OC\Files\Storage\Storage
 	 */
 	private function createStorage() {
+		if ($this->invalidStorage) {
+			return null;
+		}
+
 		if (class_exists($this->class)) {
 			try {
 				return $this->loader->getInstance($this->mountPoint, $this->class, $this->arguments);
 			} catch (\Exception $exception) {
+				$this->invalidStorage = true;
 				if ($this->mountPoint === '/') {
 					// the root storage could not be initialized, show the user!
 					throw new \Exception('The root storage could not be initialized. Please contact your local administrator.', $exception->getCode(), $exception);
@@ -113,6 +126,7 @@ class MountPoint implements IMountPoint {
 			}
 		} else {
 			\OC_Log::write('core', 'storage backend ' . $this->class . ' not found', \OC_Log::ERROR);
+			$this->invalidStorage = true;
 			return null;
 		}
 	}
@@ -137,6 +151,7 @@ class MountPoint implements IMountPoint {
 				if (is_null($storage)) {
 					return null;
 				}
+
 				$this->storage = $storage;
 			}
 			$this->storageId = $this->storage->getId();
@@ -177,7 +192,11 @@ class MountPoint implements IMountPoint {
 	 * @param callable $wrapper
 	 */
 	public function wrapStorage($wrapper) {
-		$this->storage = $wrapper($this->mountPoint, $this->getStorage());
+		$storage = $this->getStorage();
+		// storage can be null if it couldn't be initialized
+		if ($storage != null) {
+			$this->storage = $wrapper($this->mountPoint, $storage);
+		}
 	}
 
 	/**

tests/lib/files/mount/mountpoint.php

+<?php
+/**
+ * Copyright (c) 2015 Vincent Petry <pvince81@owncloud.com>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace Test\Files\Mount;
+
+class MountPoint extends \Test\TestCase {
+
+	public function testGetStorage() {
+		$storage = $this->getMock('\OCP\Files\Storage');
+		$storage->expects($this->once())
+			->method('getId')
+			->will($this->returnValue(123));
+
+		$loader = $this->getMock('\OCP\Files\Storage\IStorageFactory');
+		$loader->expects($this->once())
+			->method('getInstance')
+			->will($this->returnValue($storage));
+
+		$mountPoint = new \OC\Files\Mount\MountPoint(
+			// just use this because a real class is needed
+			'\Test\Files\Mount\MountPoint',
+			'/mountpoint',
+			null,
+			$loader
+		);
+
+		$this->assertEquals($storage, $mountPoint->getStorage());
+		$this->assertEquals(123, $mountPoint->getStorageId());
+	}
+
+	public function testInvalidStorage() {
+		$loader = $this->getMock('\OCP\Files\Storage\IStorageFactory');
+		$loader->expects($this->once())
+			->method('getInstance')
+			->will($this->throwException(new \Exception('Test storage init exception')));
+
+		$called = false;
+		$wrapper = function($mountPoint, $storage) use ($called) {
+			$called = true;
+		};
+
+		$mountPoint = new \OC\Files\Mount\MountPoint(
+			// just use this because a real class is needed
+			'\Test\Files\Mount\MountPoint',
+			'/mountpoint',
+			null,
+			$loader
+		);
+
+		$this->assertNull($mountPoint->getStorage());
+		// call it again to make sure the init code only ran once
+		$this->assertNull($mountPoint->getStorage());
+
+		$this->assertNull($mountPoint->getStorageId());
+
+		// wrapping doesn't fail
+		$mountPoint->wrapStorage($wrapper);
+
+		$this->assertNull($mountPoint->getStorage());
+
+		// storage wrapper never called
+		$this->assertFalse($called);
+	}
+}