From 356eef07398f8829a2558eee809599be60441b59 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 6 Nov 2013 11:57:04 +0100 Subject: [PATCH 1/2] Quota storage wrapper is now used for all users in sharing mode When accessing a shared folder, the folder's owner appears as mountpoint but wasn't wrapped by a quota storage wrapper. This fix makes sure that all home storages are wrapped by a quota storage wrapper, if applicable, to make sure quotas are respected when uploading into shared folders. --- apps/files/tests/ajax_rename.php | 15 ++++++++++ lib/private/util.php | 33 +++++++++++++++------ tests/lib/util.php | 49 ++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/apps/files/tests/ajax_rename.php b/apps/files/tests/ajax_rename.php index e654255c407..3735b0a49c8 100644 --- a/apps/files/tests/ajax_rename.php +++ b/apps/files/tests/ajax_rename.php @@ -22,9 +22,18 @@ */ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { + private static $user; function setUp() { // mock OC_L10n + if (!self::$user) { + self::$user = uniqid(); + } + \OC_User::createUser(self::$user, 'password'); + \OC_User::setUserId(self::$user); + + \OC\Files\Filesystem::init(self::$user, '/' . self::$user . '/files'); + $l10nMock = $this->getMock('\OC_L10N', array('t'), array(), '', false); $l10nMock->expects($this->any()) ->method('t') @@ -39,6 +48,12 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { $this->files = new \OCA\Files\App($viewMock, $l10nMock); } + function tearDown() { + $result = \OC_User::deleteUser(self::$user); + $this->assertTrue($result); + \OC\Files\Filesystem::tearDown(); + } + /** * @brief test rename of file/folder named "Shared" */ diff --git a/lib/private/util.php b/lib/private/util.php index 176eb4bc369..88f1f197b5d 100755 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -53,16 +53,31 @@ class OC_Util { //if we aren't logged in, there is no use to set up the filesystem if( $user != "" ) { - $quota = self::getUserQuota($user); - if ($quota !== \OC\Files\SPACE_UNLIMITED) { - \OC\Files\Filesystem::addStorageWrapper(function($mountPoint, $storage) use ($quota, $user) { - if ($mountPoint === '/' . $user . '/'){ - return new \OC\Files\Storage\Wrapper\Quota(array('storage' => $storage, 'quota' => $quota)); - } else { - return $storage; + \OC\Files\Filesystem::addStorageWrapper(function($mountPoint, $storage){ + // set up quota for home storages, even for other users + // which can happen when using sharing + + if (strlen($mountPoint) > 1) { + // the user name will be extracted from the mountpoint + // with the format '/username/' (no suffix) + $user = null; + // find second separator + $nextSepPos = strpos($mountPoint, '/', 1); + // next separator is the last one, format matches + if ($nextSepPos === strlen($mountPoint) - 1) { + $user = substr($mountPoint, 1, $nextSepPos - 1); } - }); - } + if ($user) { + $quota = OC_Util::getUserQuota($user); + if ($quota !== \OC\Files\SPACE_UNLIMITED) { + return new \OC\Files\Storage\Wrapper\Quota(array('storage' => $storage, 'quota' => $quota)); + } + } + } + + return $storage; + }); + $userDir = '/'.$user.'/files'; $userRoot = OC_User::getHome($user); $userDirectory = $userRoot . '/files'; diff --git a/tests/lib/util.php b/tests/lib/util.php index d607a3e7725..852caaeccc3 100644 --- a/tests/lib/util.php +++ b/tests/lib/util.php @@ -93,6 +93,55 @@ class Test_Util extends PHPUnit_Framework_TestCase { $this->assertStringStartsWith('oc', OC_Util::getInstanceId()); } + /** + * Tests that the home storage is not wrapped when no quota exists. + */ + function testHomeStorageWrapperWithoutQuota() { + $user1 = uniqid(); + \OC_User::createUser($user1, 'test'); + OC_Preferences::setValue($user1, 'files', 'quota', 'none'); + \OC_User::setUserId($user1); + + \OC_Util::setupFS($user1); + + $userMount = \OC\Files\Filesystem::getMountManager()->find('/' . $user1 . '/'); + $this->assertNotNull($userMount); + $this->assertNotInstanceOf('\OC\Files\Storage\Wrapper\Quota', $userMount->getStorage()); + + // clean up + \OC_User::setUserId(''); + \OC_User::deleteUser($user1); + OC_Preferences::deleteUser($user1); + \OC_Util::tearDownFS(); + } + + /** + * Tests that the home storage is not wrapped when no quota exists. + */ + function testHomeStorageWrapperWithQuota() { + $user1 = uniqid(); + \OC_User::createUser($user1, 'test'); + OC_Preferences::setValue($user1, 'files', 'quota', '1024'); + \OC_User::setUserId($user1); + + \OC_Util::setupFS($user1); + + $userMount = \OC\Files\Filesystem::getMountManager()->find('/' . $user1 . '/'); + $this->assertNotNull($userMount); + $this->assertInstanceOf('\OC\Files\Storage\Wrapper\Quota', $userMount->getStorage()); + + // ensure that root wasn't wrapped + $rootMount = \OC\Files\Filesystem::getMountManager()->find('/'); + $this->assertNotNull($rootMount); + $this->assertNotInstanceOf('\OC\Files\Storage\Wrapper\Quota', $rootMount->getStorage()); + + // clean up + \OC_User::setUserId(''); + \OC_User::deleteUser($user1); + OC_Preferences::deleteUser($user1); + \OC_Util::tearDownFS(); + } + /** * @dataProvider baseNameProvider */ From 69e8e7dbd5039652cceb078025248c308ffd0d55 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 21 Nov 2013 12:17:47 +0100 Subject: [PATCH 2/2] Now using the "Home" storage detection approach for quota To find out whether to apply a quota, we now try and detect whether the storage to wrap is a "Home" storage. --- lib/private/files/storage/home.php | 17 +++++++++++++++++ lib/private/util.php | 20 +++++--------------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/lib/private/files/storage/home.php b/lib/private/files/storage/home.php index b4ceb8f4f9b..1c2a682f197 100644 --- a/lib/private/files/storage/home.php +++ b/lib/private/files/storage/home.php @@ -22,6 +22,12 @@ class Home extends Local { */ protected $user; + /** + * @brief Construct a Home storage instance + * @param array $arguments array with "user" containing the + * storage owner and "legacy" containing "true" if the storage is + * a legacy storage with "local::" URL instead of the new "home::" one. + */ public function __construct($arguments) { $this->user = $arguments['user']; $datadir = $this->user->getHome(); @@ -40,10 +46,21 @@ class Home extends Local { return $this->id; } + /** + * @return \OC\Files\Cache\HomeCache + */ public function getCache($path = '') { if (!isset($this->cache)) { $this->cache = new \OC\Files\Cache\HomeCache($this); } return $this->cache; } + + /** + * @brief Returns the owner of this home storage + * @return \OC\User\User owner of this home storage + */ + public function getUser() { + return $this->user; + } } diff --git a/lib/private/util.php b/lib/private/util.php index 88f1f197b5d..959d36a89e9 100755 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -57,21 +57,11 @@ class OC_Util { // set up quota for home storages, even for other users // which can happen when using sharing - if (strlen($mountPoint) > 1) { - // the user name will be extracted from the mountpoint - // with the format '/username/' (no suffix) - $user = null; - // find second separator - $nextSepPos = strpos($mountPoint, '/', 1); - // next separator is the last one, format matches - if ($nextSepPos === strlen($mountPoint) - 1) { - $user = substr($mountPoint, 1, $nextSepPos - 1); - } - if ($user) { - $quota = OC_Util::getUserQuota($user); - if ($quota !== \OC\Files\SPACE_UNLIMITED) { - return new \OC\Files\Storage\Wrapper\Quota(array('storage' => $storage, 'quota' => $quota)); - } + if ($storage instanceof \OC\Files\Storage\Home) { + $user = $storage->getUser()->getUID(); + $quota = OC_Util::getUserQuota($user); + if ($quota !== \OC\Files\SPACE_UNLIMITED) { + return new \OC\Files\Storage\Wrapper\Quota(array('storage' => $storage, 'quota' => $quota)); } }