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())

diff --git a/lib/private/files/mount/mountpoint.php b/lib/private/files/mount/mountpoint.php
index 77a51a17020390511af6c26b38dc8c596afc2704..85edb7cb570315db2c0eeccfb4f43fef4d696a01 100644 (file)
--- a/lib/private/files/mount/mountpoint.php
+++ b/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);
+               }
        }
 
        /**

diff --git a/tests/lib/files/mount/mountpoint.php b/tests/lib/files/mount/mountpoint.php
new file mode 100644 (file)
index 0000000..5a9c6de
--- /dev/null
+++ b/tests/lib/files/mount/mountpoint.php
@@ -0,0 +1,69 @@
+<?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);
+       }
+}