Merge pull request #28160 from owncloud/fix-newdav-share-quota

Fix new DAV quota check target path
owncloud · Jun 29, 2017 · 2548f08 · 2548f08
2 parents 1a2a671 + c6e1a85
commit 2548f08
Show file tree

Hide file tree

Showing 7 changed files with 473 additions and 97 deletions.
diff --git a/apps/dav/lib/Connector/Sabre/QuotaPlugin.php b/apps/dav/lib/Connector/Sabre/QuotaPlugin.php
@@ -28,6 +28,9 @@
 use Sabre\DAV\Exception\InsufficientStorage;
 use Sabre\DAV\Exception\ServiceUnavailable;
 use Sabre\HTTP\URLUtil;
+use OCA\DAV\Upload\FutureFile;
+use Sabre\DAV\INode;
+use OCA\DAV\Connector\Sabre\Node;
 
 /**
  * This plugin check user quota and deny creating files when they exceeds the quota.
@@ -72,26 +75,82 @@ public function initialize(\Sabre\DAV\Server $server) {
 
  $this->server = $server;
 
- $server->on('beforeWriteContent', [$this, 'checkQuota'], 10);
- $server->on('beforeCreateFile', [$this, 'checkQuota'], 10);
+ $server->on('beforeWriteContent', [$this, 'handleBeforeWriteContent'], 10);
+ $server->on('beforeCreateFile', [$this, 'handleBeforeCreateFile'], 10);
+ $server->on('beforeMove', [$this, 'handleBeforeMove'], 10);
+ }
+
+ /**
+ * Check if we're moving a Futurefile in which case we need to check
+ * the quota on the target destination.
+ *
+ * @param string $source source path
+ * @param string $destination destination path
+ */
+ public function handleBeforeMove($source, $destination) {
+ $sourceNode = $this->server->tree->getNodeForPath($source);
+ if (!$sourceNode instanceof FutureFile) {
+ return;
+ }
+
+ // get target node for proper path conversion
+ if ($this->server->tree->nodeExists($destination)) {
+ $destinationNode = $this->server->tree->getNodeForPath($destination);
+ $path = $destinationNode->getPath();
+ } else {
+ $parentNode = $this->server->tree->getNodeForPath(dirname($destination));
+ $path = $parentNode->getPath();
+ }
+
+ return $this->checkQuota($path, $sourceNode->getSize());
+ }
+
+ /**
+ * Check quota before writing content
+ *
+ * @param string $uri target file URI
+ * @param INode $node Sabre Node
+ * @param resource $data data
+ * @param bool $modified modified
+ */
+ public function handleBeforeWriteContent($uri, $node, $data, $modified) {
+ if (!$node instanceof Node) {
+ return;
+ }
+ return $this->checkQuota($node->getPath());
+ }
+
+ /**
+ * Check quota before creating file
+ *
+ * @param string $uri target file URI
+ * @param resource $data data
+ * @param INode $parent Sabre Node
+ * @param bool $modified modified
+ */
+ public function handleBeforeCreateFile($uri, $data, $parent, $modified) {
+ if (!$parent instanceof Node) {
+ return;
+ }
+ return $this->checkQuota($parent->getPath() . '/' . basename($uri));
  }
 
  /**
  * This method is called before any HTTP method and validates there is enough free space to store the file
  *
- * @param string $uri
+ * @param string $path path of the user's home
+ * @param int $length size to check whether it fits
  * @throws InsufficientStorage
  * @return bool
  */
- public function checkQuota($uri) {
- $length = $this->getLength();
+ public function checkQuota($path, $length = null) {
+ if ($length === null) {
+ $length = $this->getLength();
+ }
  if ($length) {
- if (substr($uri, 0, 1) !== '/') {
- $uri = '/' . $uri;
- }
- list($parentUri, $newName) = URLUtil::splitPath($uri);
- if(is_null($parentUri)) {
- $parentUri = '';
+ list($parentPath, $newName) = URLUtil::splitPath($path);
+ if(is_null($parentPath)) {
+ $parentPath = '';
  }
  $req = $this->server->httpRequest;
  if ($req->getHeader('OC-Chunked')) {
@@ -101,9 +160,9 @@ public function checkQuota($uri) {
  // 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'];
+ $path = rtrim($parentPath, '/') . '/' . $info['name'];
  }
- $freeSpace = $this->getFreeSpace($uri);
+ $freeSpace = $this->getFreeSpace($path);
  if ($freeSpace !== FileInfo::SPACE_UNKNOWN && $length > $freeSpace) {
  if (isset($chunkHandler)) {
  $chunkHandler->cleanup();

diff --git a/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php
@@ -22,7 +22,13 @@
  *
  */
 namespace OCA\DAV\Tests\unit\Connector\Sabre;
+
 use Test\TestCase;
+use OCA\DAV\Connector\Sabre\File;
+use OCA\DAV\Connector\Sabre\Directory;
+use Sabre\DAV\Tree;
+use OCA\DAV\Connector\Sabre\QuotaPlugin;
+use OCA\DAV\Upload\FutureFile;
 
 /**
  * Copyright (c) 2013 Thomas Müller <thomas.mueller@tmit.eu>
@@ -228,4 +234,108 @@ private function buildFileViewMock($quota, $checkedPath) {
  return $view;
  }
 
+ public function pathDataProvider() {
+ $node = $this->createMock(File::class);
+ $node->method('getPath')->willReturn('/test/sub/test.txt');
+
+ $parentNode = $this->createMock(Directory::class);
+ $parentNode->method('getPath')->willReturn('/test/sub');
+
+ return [
+ ['beforeWriteContent', ['/files/user0/test/sub/test.txt', $node, null, false], '/test/sub/test.txt'],
+ ['beforeCreateFile', ['/files/user0/test/sub/test.txt', null, $parentNode, false], '/test/sub/test.txt'],
+ ];
+ }
+
+ /**
+ * @dataProvider pathDataProvider
+ */
+ public function testPath($event, $eventArgs, $expectedPath) {
+ $plugin = $this->getMockBuilder(QuotaPlugin::class)
+ ->setConstructorArgs([null])
+ ->setMethods(['getFileChunking', 'checkQuota'])
+ ->getMock();
+
+ $server = new \Sabre\DAV\Server();
+ $plugin->initialize($server);
+
+ $plugin->expects($this->once())
+ ->method('checkQuota')
+ ->with($expectedPath);
+
+ $server->emit($event, $eventArgs);
+
+ }
+
+ public function testPathBeforeModeTargetExists() {
+ $plugin = $this->getMockBuilder(QuotaPlugin::class)
+ ->setConstructorArgs([null])
+ ->setMethods(['getFileChunking', 'checkQuota'])
+ ->getMock();
+
+ $server = new \Sabre\DAV\Server();
+ $server->tree = $this->createMock(Tree::class);
+
+ $source = '/uploads/chunking-1/.file';
+ $destination = '/files/user0/test/sub/test.txt';
+
+ $sourceNode = $this->createMock(FutureFile::class);
+ $sourceNode->method('getSize')->willReturn(12345);
+
+ $destinationNode = $this->createMock(File::class);
+ $destinationNode->method('getPath')->willReturn('/test/sub/test.txt');
+
+ $server->tree->method('nodeExists')
+ ->with($destination)
+ ->willReturn(true);
+ $server->tree->method('getNodeForPath')
+ ->will($this->returnValueMap([
+ [$source, $sourceNode],
+ [$destination, $destinationNode],
+ ]));
+
+ $plugin->initialize($server);
+ $plugin->expects($this->once())
+ ->method('checkQuota')
+ ->with('/test/sub/test.txt', 12345);
+
+ $server->emit('beforeMove', [$source, $destination]);
+
+ }
+
+ public function testPathBeforeModeTargetDoesNotExists() {
+ $plugin = $this->getMockBuilder(QuotaPlugin::class)
+ ->setConstructorArgs([null])
+ ->setMethods(['getFileChunking', 'checkQuota'])
+ ->getMock();
+
+ $server = new \Sabre\DAV\Server();
+ $server->tree = $this->createMock(Tree::class);
+
+ $source = '/uploads/chunking-1/.file';
+ $destination = '/files/user0/test/sub/test.txt';
+
+ $sourceNode = $this->createMock(FutureFile::class);
+ $sourceNode->method('getSize')->willReturn(12345);
+
+ $parentDestinationNode = $this->createMock(Directory::class);
+ $parentDestinationNode->method('getPath')->willReturn('/test/sub');
+
+ $server->tree->method('nodeExists')
+ ->with($destination)
+ ->willReturn(false);
+ $server->tree->method('getNodeForPath')
+ ->will($this->returnValueMap([
+ [$source, $sourceNode],
+ ['/files/user0/test/sub', $parentDestinationNode],
+ ]));
+
+ $plugin->initialize($server);
+ $plugin->expects($this->once())
+ ->method('checkQuota')
+ ->with('/test/sub', 12345);
+
+ $server->emit('beforeMove', [$source, $destination]);
+
+ }
 }