From fb890eee674fece6194a743483ae59f4384c7eb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 8 Aug 2013 00:42:28 +0200 Subject: [PATCH 1/4] fixes #4343 --- lib/connector/sabre/quotaplugin.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/connector/sabre/quotaplugin.php b/lib/connector/sabre/quotaplugin.php index c80a33d04b..0f428b75b1 100644 --- a/lib/connector/sabre/quotaplugin.php +++ b/lib/connector/sabre/quotaplugin.php @@ -43,8 +43,7 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { * @return bool */ public function checkQuota($uri, $data = null) { - $expected = $this->server->httpRequest->getHeader('X-Expected-Entity-Length'); - $length = $expected ? $expected : $this->server->httpRequest->getHeader('Content-Length'); + $length = $this->getLength(); if ($length) { if (substr($uri, 0, 1)!=='/') { $uri='/'.$uri; @@ -57,4 +56,19 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { } return true; } + + private function getLength() + { + $expected = $this->server->httpRequest->getHeader('X-Expected-Entity-Length'); + if ($expected) + return $expected; + + $length = $this->server->httpRequest->getHeader('Content-Length'); + $ocLength = $this->server->httpRequest->getHeader('OC-Total-Length'); + + if ($length && $ocLength) + return max($length, $ocLength); + + return $length; + } } From d3a69bf4c6baba563beceb225b9192a4e22c9c59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 8 Aug 2013 11:04:40 +0200 Subject: [PATCH 2/4] adding unit tests to determine length --- lib/connector/sabre/quotaplugin.php | 2 +- tests/lib/connector/sabre/quotaplugin.php | 47 +++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 tests/lib/connector/sabre/quotaplugin.php diff --git a/lib/connector/sabre/quotaplugin.php b/lib/connector/sabre/quotaplugin.php index 0f428b75b1..730a86666b 100644 --- a/lib/connector/sabre/quotaplugin.php +++ b/lib/connector/sabre/quotaplugin.php @@ -57,7 +57,7 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { return true; } - private function getLength() + public function getLength() { $expected = $this->server->httpRequest->getHeader('X-Expected-Entity-Length'); if ($expected) diff --git a/tests/lib/connector/sabre/quotaplugin.php b/tests/lib/connector/sabre/quotaplugin.php new file mode 100644 index 0000000000..9582af6ec4 --- /dev/null +++ b/tests/lib/connector/sabre/quotaplugin.php @@ -0,0 +1,47 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +class Test_OC_Connector_Sabre_QuotaPlugin extends PHPUnit_Framework_TestCase { + + /** + * @var Sabre_DAV_Server + */ + private $server; + + /** + * @var OC_Connector_Sabre_QuotaPlugin + */ + private $plugin; + + public function setUp() { + $this->server = new Sabre_DAV_Server(); + $this->plugin = new OC_Connector_Sabre_QuotaPlugin(); + $this->plugin->initialize($this->server); + } + + /** + * @dataProvider lengthProvider + */ + public function testLength($expected, $headers) + { + $this->server->httpRequest = new Sabre_HTTP_Request($headers); + $length = $this->plugin->getLength(); + $this->assertEquals($expected, $length); + } + + public function lengthProvider() + { + return array( + array(null, array()), + array(1024, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')), + array(512, array('HTTP_CONTENT_LENGTH' => '512')), + array(2048, array('HTTP_OC_TOTAL_LENGTH' => '2048', 'HTTP_CONTENT_LENGTH' => '1024')), + ); + } + +} From fed1792510ff11941765783653573f45fadc4c70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 8 Aug 2013 13:33:00 +0200 Subject: [PATCH 3/4] adding unit tests for quota checks --- lib/connector/sabre/quotaplugin.php | 75 +++++++++++++---------- tests/lib/connector/sabre/quotaplugin.php | 56 ++++++++++++++++- 2 files changed, 98 insertions(+), 33 deletions(-) diff --git a/lib/connector/sabre/quotaplugin.php b/lib/connector/sabre/quotaplugin.php index 730a86666b..eb95a839b8 100644 --- a/lib/connector/sabre/quotaplugin.php +++ b/lib/connector/sabre/quotaplugin.php @@ -3,45 +3,55 @@ /** * This plugin check user quota and deny creating files when they exceeds the quota. * - * @copyright Copyright (C) 2012 entreCables S.L. All rights reserved. * @author Sergio Cambra + * @copyright Copyright (C) 2012 entreCables S.L. All rights reserved. * @license http://code.google.com/p/sabredav/wiki/License Modified BSD License */ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { /** - * Reference to main server object - * - * @var Sabre_DAV_Server - */ + * Reference to main server object + * + * @var Sabre_DAV_Server + */ private $server; /** - * This initializes the plugin. - * - * This function is called by Sabre_DAV_Server, after - * addPlugin is called. - * - * This method should set up the requires event subscriptions. - * - * @param Sabre_DAV_Server $server - * @return void - */ + * is kept public to allow overwrite for unit testing + * + * @var \OC\Files\View + */ + public $fileView; + + /** + * This initializes the plugin. + * + * This function is called by Sabre_DAV_Server, after + * addPlugin is called. + * + * This method should set up the requires event subscriptions. + * + * @param Sabre_DAV_Server $server + * @return void + */ public function initialize(Sabre_DAV_Server $server) { - $this->server = $server; - $this->server->subscribeEvent('beforeWriteContent', array($this, 'checkQuota'), 10); - $this->server->subscribeEvent('beforeCreateFile', array($this, 'checkQuota'), 10); + $this->server = $server; + $server->subscribeEvent('beforeWriteContent', array($this, 'checkQuota'), 10); + $server->subscribeEvent('beforeCreateFile', array($this, 'checkQuota'), 10); + + // initialize fileView + $this->fileView = \OC\Files\Filesystem::getView(); } /** - * This method is called before any HTTP method and forces users to be authenticated - * - * @param string $method - * @throws Sabre_DAV_Exception - * @return bool - */ + * This method is called before any HTTP method and validates there is enough free space to store the file + * + * @param string $method + * @throws Sabre_DAV_Exception + * @return bool + */ public function checkQuota($uri, $data = null) { $length = $this->getLength(); if ($length) { @@ -49,7 +59,7 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { $uri='/'.$uri; } list($parentUri, $newName) = Sabre_DAV_URLUtil::splitPath($uri); - $freeSpace = \OC\Files\Filesystem::free_space($parentUri); + $freeSpace = $this->fileView->free_space($parentUri); if ($freeSpace !== \OC\Files\FREE_SPACE_UNKNOWN && $length > $freeSpace) { throw new Sabre_DAV_Exception_InsufficientStorage(); } @@ -59,15 +69,16 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { public function getLength() { - $expected = $this->server->httpRequest->getHeader('X-Expected-Entity-Length'); - if ($expected) - return $expected; + $req = $this->server->httpRequest; + $length = $req->getHeader('X-Expected-Entity-Length'); + if (!$length) { + $length = $req->getHeader('Content-Length'); + } - $length = $this->server->httpRequest->getHeader('Content-Length'); - $ocLength = $this->server->httpRequest->getHeader('OC-Total-Length'); - - if ($length && $ocLength) + $ocLength = $req->getHeader('OC-Total-Length'); + if ($length && $ocLength) { return max($length, $ocLength); + } return $length; } diff --git a/tests/lib/connector/sabre/quotaplugin.php b/tests/lib/connector/sabre/quotaplugin.php index 9582af6ec4..1186de2874 100644 --- a/tests/lib/connector/sabre/quotaplugin.php +++ b/tests/lib/connector/sabre/quotaplugin.php @@ -34,14 +34,68 @@ class Test_OC_Connector_Sabre_QuotaPlugin extends PHPUnit_Framework_TestCase { $this->assertEquals($expected, $length); } - public function lengthProvider() + /** + * @dataProvider quotaOkayProvider + */ + public function testCheckQuota($quota, $headers) { + $this->plugin->fileView = $this->buildFileViewMock($quota); + + $this->server->httpRequest = new Sabre_HTTP_Request($headers); + $result = $this->plugin->checkQuota(''); + $this->assertTrue($result); + } + + /** + * @expectedException Sabre_DAV_Exception_InsufficientStorage + * @dataProvider quotaExceededProvider + */ + public function testCheckExceededQuota($quota, $headers) + { + $this->plugin->fileView = $this->buildFileViewMock($quota); + + $this->server->httpRequest = new Sabre_HTTP_Request($headers); + $this->plugin->checkQuota(''); + } + + public function quotaOkayProvider() { + return array( + array(1024, array()), + array(1024, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')), + array(1024, array('HTTP_CONTENT_LENGTH' => '512')), + array(1024, array('HTTP_OC_TOTAL_LENGTH' => '1024', 'HTTP_CONTENT_LENGTH' => '512')), + // OC\Files\FREE_SPACE_UNKNOWN = -2 + array(-2, array()), + array(-2, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')), + array(-2, array('HTTP_CONTENT_LENGTH' => '512')), + array(-2, array('HTTP_OC_TOTAL_LENGTH' => '1024', 'HTTP_CONTENT_LENGTH' => '512')), + ); + } + + public function quotaExceededProvider() { + return array( + array(1023, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')), + array(511, array('HTTP_CONTENT_LENGTH' => '512')), + array(2047, array('HTTP_OC_TOTAL_LENGTH' => '2048', 'HTTP_CONTENT_LENGTH' => '1024')), + ); + } + + public function lengthProvider() { return array( array(null, array()), array(1024, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')), array(512, array('HTTP_CONTENT_LENGTH' => '512')), array(2048, array('HTTP_OC_TOTAL_LENGTH' => '2048', 'HTTP_CONTENT_LENGTH' => '1024')), + array(4096, array('HTTP_OC_TOTAL_LENGTH' => '2048', 'HTTP_X_EXPECTED_ENTITY_LENGTH' => '4096')), ); } + private function buildFileViewMock($quota) { + // mock filesysten + $view = $this->getMock('\OC\Files\View', array('free_space'), array(), '', FALSE); + $view->expects($this->any())->method('free_space')->withAnyParameters()->will($this->returnValue($quota)); + + return $view; + } + } From d1a6d2bc8fce5cefe828f09a77a8b9c9172b5c57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 26 Aug 2013 20:21:16 +0200 Subject: [PATCH 4/4] lacy initialization of fileView - in case basic auth is used FileSystem is not yet initialized during the initialize() call --- lib/connector/sabre/quotaplugin.php | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/connector/sabre/quotaplugin.php b/lib/connector/sabre/quotaplugin.php index c8ce65a857..ea2cb81d1f 100644 --- a/lib/connector/sabre/quotaplugin.php +++ b/lib/connector/sabre/quotaplugin.php @@ -40,9 +40,6 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { $server->subscribeEvent('beforeWriteContent', array($this, 'checkQuota'), 10); $server->subscribeEvent('beforeCreateFile', array($this, 'checkQuota'), 10); - - // initialize fileView - $this->fileView = \OC\Files\Filesystem::getView(); } /** @@ -59,7 +56,7 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { $uri='/'.$uri; } list($parentUri, $newName) = Sabre_DAV_URLUtil::splitPath($uri); - $freeSpace = $this->fileView->free_space($parentUri); + $freeSpace = $this->getFreeSpace($parentUri); if ($freeSpace !== \OC\Files\SPACE_UNKNOWN && $length > $freeSpace) { throw new Sabre_DAV_Exception_InsufficientStorage(); } @@ -82,4 +79,19 @@ class OC_Connector_Sabre_QuotaPlugin extends Sabre_DAV_ServerPlugin { return $length; } + + /** + * @param $parentUri + * @return mixed + */ + public function getFreeSpace($parentUri) + { + if (is_null($this->fileView)) { + // initialize fileView + $this->fileView = \OC\Files\Filesystem::getView(); + } + + $freeSpace = $this->fileView->free_space($parentUri); + return $freeSpace; + } }