From 53eff9792f1ae5ece49dfe79b40d1d73ae530688 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 11 Jan 2016 12:38:13 +0100 Subject: [PATCH 1/3] Check the quota on the actual file's storage in dav quota plugin Fix quota plugin to use the correct file name when chunking When chunking, the file name is the compound name, so need to convert it to the correct final file name before doing the free space check. This ensures that in the case of shared files, the correct storage is used for the quota check. --- apps/dav/lib/connector/sabre/quotaplugin.php | 17 ++- .../unit/connector/sabre/quotaplugin.php | 109 +++++++++++++++++- 2 files changed, 116 insertions(+), 10 deletions(-) diff --git a/apps/dav/lib/connector/sabre/quotaplugin.php b/apps/dav/lib/connector/sabre/quotaplugin.php index a02827da499..b1c3bbfbbb9 100644 --- a/apps/dav/lib/connector/sabre/quotaplugin.php +++ b/apps/dav/lib/connector/sabre/quotaplugin.php @@ -95,12 +95,14 @@ class QuotaPlugin extends \Sabre\DAV\ServerPlugin { $req = $this->server->httpRequest; if ($req->getHeader('OC-Chunked')) { $info = \OC_FileChunking::decodeName($newName); - $chunkHandler = new \OC_FileChunking($info); + $chunkHandler = $this->getFileChunking($info); // subtract the already uploaded size to see whether // there is still enough space for the remaining chunks $length -= $chunkHandler->getCurrentSize(); + // use target file name for free space check in case of shared files + $uri = rtrim($parentUri, '/') . '/' . $info['name']; } - $freeSpace = $this->getFreeSpace($parentUri); + $freeSpace = $this->getFreeSpace($uri); if ($freeSpace !== \OCP\Files\FileInfo::SPACE_UNKNOWN && $length > $freeSpace) { if (isset($chunkHandler)) { $chunkHandler->cleanup(); @@ -111,6 +113,11 @@ class QuotaPlugin extends \Sabre\DAV\ServerPlugin { return true; } + public function getFileChunking($info) { + // FIXME: need a factory for better mocking support + return new \OC_FileChunking($info); + } + public function getLength() { $req = $this->server->httpRequest; $length = $req->getHeader('X-Expected-Entity-Length'); @@ -127,12 +134,12 @@ class QuotaPlugin extends \Sabre\DAV\ServerPlugin { } /** - * @param string $parentUri + * @param string $uri * @return mixed */ - public function getFreeSpace($parentUri) { + public function getFreeSpace($uri) { try { - $freeSpace = $this->view->free_space($parentUri); + $freeSpace = $this->view->free_space(ltrim($uri, '/')); return $freeSpace; } catch (\OCP\Files\StorageNotAvailableException $e) { throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); diff --git a/apps/dav/tests/unit/connector/sabre/quotaplugin.php b/apps/dav/tests/unit/connector/sabre/quotaplugin.php index cc4339ecc1a..b5a8bfef31c 100644 --- a/apps/dav/tests/unit/connector/sabre/quotaplugin.php +++ b/apps/dav/tests/unit/connector/sabre/quotaplugin.php @@ -39,10 +39,13 @@ class QuotaPlugin extends \Test\TestCase { */ private $plugin; - private function init($quota) { - $view = $this->buildFileViewMock($quota); + private function init($quota, $checkedPath = '') { + $view = $this->buildFileViewMock($quota, $checkedPath); $this->server = new \Sabre\DAV\Server(); - $this->plugin = new \OCA\DAV\Connector\Sabre\QuotaPlugin($view); + $this->plugin = $this->getMockBuilder('\OCA\DAV\Connector\Sabre\QuotaPlugin') + ->setConstructorArgs([$view]) + ->setMethods(['getFileChunking']) + ->getMock(); $this->plugin->initialize($this->server); } @@ -51,6 +54,8 @@ class QuotaPlugin extends \Test\TestCase { */ public function testLength($expected, $headers) { $this->init(0); + $this->plugin->expects($this->never()) + ->method('getFileChunking'); $this->server->httpRequest = new \Sabre\HTTP\Request(null, null, $headers); $length = $this->plugin->getLength(); $this->assertEquals($expected, $length); @@ -61,6 +66,8 @@ class QuotaPlugin extends \Test\TestCase { */ public function testCheckQuota($quota, $headers) { $this->init($quota); + $this->plugin->expects($this->never()) + ->method('getFileChunking'); $this->server->httpRequest = new \Sabre\HTTP\Request(null, null, $headers); $result = $this->plugin->checkQuota(''); @@ -73,11 +80,26 @@ class QuotaPlugin extends \Test\TestCase { */ public function testCheckExceededQuota($quota, $headers) { $this->init($quota); + $this->plugin->expects($this->never()) + ->method('getFileChunking'); $this->server->httpRequest = new \Sabre\HTTP\Request(null, null, $headers); $this->plugin->checkQuota(''); } + /** + * @dataProvider quotaOkayProvider + */ + public function testCheckQuotaOnPath($quota, $headers) { + $this->init($quota, 'sub/test.txt'); + $this->plugin->expects($this->never()) + ->method('getFileChunking'); + + $this->server->httpRequest = new \Sabre\HTTP\Request(null, null, $headers); + $result = $this->plugin->checkQuota('/sub/test.txt'); + $this->assertTrue($result); + } + public function quotaOkayProvider() { return array( array(1024, array()), @@ -110,12 +132,89 @@ class QuotaPlugin extends \Test\TestCase { ); } - private function buildFileViewMock($quota) { + public function quotaChunkedOkProvider() { + return array( + array(1024, 0, array('X-EXPECTED-ENTITY-LENGTH' => '1024')), + array(1024, 0, array('CONTENT-LENGTH' => '512')), + array(1024, 0, array('OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512')), + // with existing chunks (allowed size = total length - chunk total size) + array(400, 128, array('X-EXPECTED-ENTITY-LENGTH' => '512')), + array(400, 128, array('CONTENT-LENGTH' => '512')), + array(400, 128, array('OC-TOTAL-LENGTH' => '512', 'CONTENT-LENGTH' => '500')), + // \OCP\Files\FileInfo::SPACE-UNKNOWN = -2 + array(-2, 0, array('X-EXPECTED-ENTITY-LENGTH' => '1024')), + array(-2, 0, array('CONTENT-LENGTH' => '512')), + array(-2, 0, array('OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512')), + array(-2, 128, array('X-EXPECTED-ENTITY-LENGTH' => '1024')), + array(-2, 128, array('CONTENT-LENGTH' => '512')), + array(-2, 128, array('OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512')), + ); + } + + /** + * @dataProvider quotaChunkedOkProvider + */ + public function testCheckQuotaChunkedOk($quota, $chunkTotalSize, $headers) { + $this->init($quota, 'sub/test.txt'); + + $mockChunking = $this->getMockBuilder('\OC_FileChunking') + ->disableOriginalConstructor() + ->getMock(); + $mockChunking->expects($this->once()) + ->method('getCurrentSize') + ->will($this->returnValue($chunkTotalSize)); + + $this->plugin->expects($this->once()) + ->method('getFileChunking') + ->will($this->returnValue($mockChunking)); + + $headers['OC-CHUNKED'] = 1; + $this->server->httpRequest = new \Sabre\HTTP\Request(null, null, $headers); + $result = $this->plugin->checkQuota('/sub/test.txt-chunking-12345-3-1'); + $this->assertTrue($result); + } + + public function quotaChunkedFailProvider() { + return array( + array(400, 0, array('X-EXPECTED-ENTITY-LENGTH' => '1024')), + array(400, 0, array('CONTENT-LENGTH' => '512')), + array(400, 0, array('OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512')), + // with existing chunks (allowed size = total length - chunk total size) + array(380, 128, array('X-EXPECTED-ENTITY-LENGTH' => '512')), + array(380, 128, array('CONTENT-LENGTH' => '512')), + array(380, 128, array('OC-TOTAL-LENGTH' => '512', 'CONTENT-LENGTH' => '500')), + ); + } + + /** + * @dataProvider quotaChunkedFailProvider + * @expectedException \Sabre\DAV\Exception\InsufficientStorage + */ + public function testCheckQuotaChunkedFail($quota, $chunkTotalSize, $headers) { + $this->init($quota, 'sub/test.txt'); + + $mockChunking = $this->getMockBuilder('\OC_FileChunking') + ->disableOriginalConstructor() + ->getMock(); + $mockChunking->expects($this->once()) + ->method('getCurrentSize') + ->will($this->returnValue($chunkTotalSize)); + + $this->plugin->expects($this->once()) + ->method('getFileChunking') + ->will($this->returnValue($mockChunking)); + + $headers['OC-CHUNKED'] = 1; + $this->server->httpRequest = new \Sabre\HTTP\Request(null, null, $headers); + $this->plugin->checkQuota('/sub/test.txt-chunking-12345-3-1'); + } + + private function buildFileViewMock($quota, $checkedPath) { // mock filesysten $view = $this->getMock('\OC\Files\View', array('free_space'), array(), '', false); $view->expects($this->any()) ->method('free_space') - ->with($this->identicalTo('')) + ->with($this->identicalTo($checkedPath)) ->will($this->returnValue($quota)); return $view; From 8f96ef147f7ddf517e83e10b2dab5f4e94da9f06 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 16 Feb 2016 14:39:04 +0100 Subject: [PATCH 2/3] Don't apply quota in stream wrapper for part files When overwriting shared files as recipient, the part file is written on the uploader's storage before overwriting the target file. If the uploader has no quota left, they should still be able to overwrite that file with Webdav. To make this work, they need to be able to write the part file to their own storage first. --- lib/private/files/storage/wrapper/quota.php | 27 +++++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/lib/private/files/storage/wrapper/quota.php b/lib/private/files/storage/wrapper/quota.php index 844505679df..500677b092e 100644 --- a/lib/private/files/storage/wrapper/quota.php +++ b/lib/private/files/storage/wrapper/quota.php @@ -141,16 +141,33 @@ class Quota extends Wrapper { */ public function fopen($path, $mode) { $source = $this->storage->fopen($path, $mode); - $free = $this->free_space(''); - if ($source && $free >= 0 && $mode !== 'r' && $mode !== 'rb') { - // only apply quota for files, not metadata, trash or others - if (strpos(ltrim($path, '/'), 'files/') === 0) { - return \OC\Files\Stream\Quota::wrap($source, $free); + + // don't apply quota for part files + if (!$this->isPartFile($path)) { + $free = $this->free_space(''); + if ($source && $free >= 0 && $mode !== 'r' && $mode !== 'rb') { + // only apply quota for files, not metadata, trash or others + if (strpos(ltrim($path, '/'), 'files/') === 0) { + return \OC\Files\Stream\Quota::wrap($source, $free); + } } } return $source; } + /** + * Checks whether the given path is a part file + * + * @param string $path Path that may identify a .part file + * @return string File path without .part extension + * @note this is needed for reusing keys + */ + private function isPartFile($path) { + $extension = pathinfo($path, PATHINFO_EXTENSION); + + return ($extension === 'part'); + } + /** * @param \OCP\Files\Storage $sourceStorage * @param string $sourceInternalPath From ceaefc2c54f4a5d4528bf2e7ba4607297d2453f4 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 16 Feb 2016 12:39:44 +0100 Subject: [PATCH 3/3] Defer quota check in web UI when overwriting shared file When receiving a shared file, the quota for that file counts in the owner's storage, not the current user's storage. To make it possible to overwrite the file even when the current user doesn't have enough space, the quota check is deferred for such files. --- apps/files/ajax/upload.php | 8 +++++-- apps/files/js/file-upload.js | 43 ++++++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/apps/files/ajax/upload.php b/apps/files/ajax/upload.php index 5bf69d3e304..36aaed5ad9e 100644 --- a/apps/files/ajax/upload.php +++ b/apps/files/ajax/upload.php @@ -136,8 +136,12 @@ $maxUploadFileSize = $storageStats['uploadMaxFilesize']; $maxHumanFileSize = OCP\Util::humanFileSize($maxUploadFileSize); $totalSize = 0; -foreach ($files['size'] as $size) { - $totalSize += $size; +$isReceivedShare = \OC::$server->getRequest()->getParam('isReceivedShare', false) === 'true'; +// defer quota check for received shares +if (!$isReceivedShare) { + foreach ($files['size'] as $size) { + $totalSize += $size; + } } if ($maxUploadFileSize >= 0 and $totalSize > $maxUploadFileSize) { OCP\JSON::error(array('data' => array('message' => $l->t('Not enough storage available'), diff --git a/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index d419cb3a2c0..8ba294e2a7f 100644 --- a/apps/files/js/file-upload.js +++ b/apps/files/js/file-upload.js @@ -251,7 +251,26 @@ OC.Upload = { $('#file_upload_start').trigger(new $.Event('resized')); }, + /** + * Returns whether the given file is known to be a received shared file + * + * @param {Object} file file + * @return {bool} true if the file is a shared file + */ + _isReceivedSharedFile: function(file) { + if (!window.FileList) { + return false; + } + var $tr = window.FileList.findFileEl(file.name); + if (!$tr.length) { + return false; + } + + return ($tr.attr('data-mounttype') === 'shared-root' && $tr.attr('data-mime') !== 'httpd/unix-directory'); + }, + init: function() { + var self = this; if ( $('#file_upload_start').exists() ) { var file_upload_param = { dropZone: $('#content'), // restrict dropZone to content div @@ -341,10 +360,15 @@ OC.Upload = { } } - // add size - selection.totalBytes += file.size; - // update size of biggest file - selection.biggestFileBytes = Math.max(selection.biggestFileBytes, file.size); + // only count if we're not overwriting an existing shared file + if (self._isReceivedSharedFile(file)) { + file.isReceivedShare = true; + } else { + // add size + selection.totalBytes += file.size; + // update size of biggest file + selection.biggestFileBytes = Math.max(selection.biggestFileBytes, file.size); + } // check PHP upload limit against biggest file if (selection.biggestFileBytes > $('#upload_limit').val()) { @@ -430,11 +454,16 @@ OC.Upload = { fileDirectory = data.files[0].relativePath; } - addFormData(data.formData, { + var params = { requesttoken: oc_requesttoken, dir: data.targetDir || FileList.getCurrentDirectory(), - file_directory: fileDirectory - }); + file_directory: fileDirectory, + }; + if (data.files[0].isReceivedShare) { + params.isReceivedShare = true; + } + + addFormData(data.formData, params); }, fail: function(e, data) { OC.Upload.log('fail', e, data);