From 730c80ff9c83ff1b027ab804ecad599fbcaf7bd3 Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Mon, 7 Oct 2013 15:11:47 +0200 Subject: adding additional exceptions for special cases where creating a file might not be allowed --- .../connector/sabre/exception/entitytoolarge.php | 23 ++++++++++++++++++++++ .../sabre/exception/unsupportedmediatype.php | 22 +++++++++++++++++++++ lib/private/connector/sabre/file.php | 7 +++++++ 3 files changed, 52 insertions(+) create mode 100644 lib/private/connector/sabre/exception/entitytoolarge.php create mode 100644 lib/private/connector/sabre/exception/unsupportedmediatype.php (limited to 'lib/private/connector') diff --git a/lib/private/connector/sabre/exception/entitytoolarge.php b/lib/private/connector/sabre/exception/entitytoolarge.php new file mode 100644 index 00000000000..aa9b37a0496 --- /dev/null +++ b/lib/private/connector/sabre/exception/entitytoolarge.php @@ -0,0 +1,23 @@ + Date: Mon, 7 Oct 2013 17:34:21 +0200 Subject: the path for reassembling was created the wrong way --- lib/private/connector/sabre/file.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/private/connector') diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 8ffec371e3f..12d7585884e 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -58,7 +58,7 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D // chunked handling if (isset($_SERVER['HTTP_OC_CHUNKED'])) { - list(, $name) = \Sabre_DAV_URLUtil::splitPath($this->path); + list($path, $name) = \Sabre_DAV_URLUtil::splitPath($this->path); $info = OC_FileChunking::decodeName($name); if (empty($info)) { @@ -67,7 +67,7 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D $chunk_handler = new OC_FileChunking($info); $chunk_handler->store($info['index'], $data); if ($chunk_handler->isComplete()) { - $newPath = $this->path . '/' . $info['name']; + $newPath = $path . '/' . $info['name']; $chunk_handler->file_assemble($newPath); return $this->getETagPropertyForPath($newPath); } -- cgit v1.2.3 From 0293d8e04f614c330be8aac7e968eec8469fb0e5 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Oct 2013 11:26:49 +0200 Subject: If a existing file in Shared/ with update permissions gets updated we need to write the .part file to a different place because we can't create new files in the Shared folder --- lib/private/connector/sabre/file.php | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'lib/private/connector') diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 12d7585884e..43e25de40c7 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -45,7 +45,9 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D * @return string|null */ public function put($data) { + $fs = $this->getFS(); + if ($fs->file_exists($this->path) && !$fs->isUpdatable($this->path)) { throw new \Sabre_DAV_Exception_Forbidden(); @@ -58,12 +60,14 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D // chunked handling if (isset($_SERVER['HTTP_OC_CHUNKED'])) { + list($path, $name) = \Sabre_DAV_URLUtil::splitPath($this->path); $info = OC_FileChunking::decodeName($name); if (empty($info)) { throw new Sabre_DAV_Exception_NotImplemented(); } + $chunk_handler = new OC_FileChunking($info); $chunk_handler->store($info['index'], $data); if ($chunk_handler->isComplete()) { @@ -78,6 +82,13 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D // mark file as partial while uploading (ignored by the scanner) $partpath = $this->path . '.part'; + // if file is located in /Shared we write the part file to the users + // root folder because we can't create new files in /shared + // we extend the name with a random number to avoid overwriting a existing file + if (dirname($partpath) === 'Shared') { + $partpath = pathinfo($partpath, PATHINFO_FILENAME) . rand(); + } + try { $putOkay = $fs->file_put_contents($partpath, $data); if ($putOkay === false) { -- cgit v1.2.3 From dd202d9ad3a542cb4c86baad92cadb2cb975afcf Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Oct 2013 11:27:08 +0200 Subject: updating a existing large file creates new file chunks. Therefore createFile() needs to check not only if we can write to the parent folder but also if we can update the existing file" --- lib/private/connector/sabre/directory.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'lib/private/connector') diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php index af0dfd70f08..d0334780361 100644 --- a/lib/private/connector/sabre/directory.php +++ b/lib/private/connector/sabre/directory.php @@ -50,8 +50,22 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa */ public function createFile($name, $data = null) { - if (!\OC\Files\Filesystem::isCreatable($this->path)) { - throw new \Sabre_DAV_Exception_Forbidden(); + // for chunked upload also updating a existing file is a "createFile" + // because we create all the chunks before reasamble them to the existing file. + if (isset($_SERVER['HTTP_OC_CHUNKED'])) { + + // exit if we can't create a new file and we don't updatable existing file + $info = OC_FileChunking::decodeName($name); + if (!\OC\Files\Filesystem::isCreatable($this->path) && + !\OC\Files\Filesystem::isUpdatable($this->path . '/' . $info['name'])) { + throw new \Sabre_DAV_Exception_Forbidden(); + } + + } else { + // For non-chunked upload it is enough to check if we can create a new file + if (!\OC\Files\Filesystem::isCreatable($this->path)) { + throw new \Sabre_DAV_Exception_Forbidden(); + } } $path = $this->path . '/' . $name; -- cgit v1.2.3 From c77f74e1defddaa277f85bc9b6242371a13fda42 Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Tue, 8 Oct 2013 11:43:44 +0200 Subject: adding check isDeletable() on $sourcePath --- lib/private/connector/sabre/objecttree.php | 3 +++ tests/lib/connector/sabre/objecttree.php | 32 ++++++++++++++++++------------ 2 files changed, 22 insertions(+), 13 deletions(-) (limited to 'lib/private/connector') diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index 80c3840b99d..df8902f66e2 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -87,6 +87,9 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { if (!$fs->isUpdatable($destinationDir)) { throw new \Sabre_DAV_Exception_Forbidden(); } + if (!$fs->isDeletable($sourcePath)) { + throw new \Sabre_DAV_Exception_Forbidden(); + } } $renameOkay = $fs->rename($sourcePath, $destinationPath); diff --git a/tests/lib/connector/sabre/objecttree.php b/tests/lib/connector/sabre/objecttree.php index 1d76bb59676..e32f2365f95 100644 --- a/tests/lib/connector/sabre/objecttree.php +++ b/tests/lib/connector/sabre/objecttree.php @@ -15,8 +15,9 @@ use Sabre_DAV_Exception_Forbidden; class TestDoubleFileView extends \OC\Files\View{ - public function __construct($updatables, $canRename = true) { + public function __construct($updatables, $deletables, $canRename = true) { $this->updatables = $updatables; + $this->deletables = $deletables; $this->canRename = $canRename; } @@ -24,6 +25,10 @@ class TestDoubleFileView extends \OC\Files\View{ return $this->updatables[$path]; } + public function isDeletable($path) { + return $this->deletables[$path]; + } + public function rename($path1, $path2) { return $this->canRename; } @@ -35,31 +40,32 @@ class ObjectTree extends PHPUnit_Framework_TestCase { * @dataProvider moveFailedProvider * @expectedException Sabre_DAV_Exception_Forbidden */ - public function testMoveFailed($source, $dest, $updatables) { - $this->moveTest($source, $dest, $updatables); + public function testMoveFailed($source, $dest, $updatables, $deletables) { + $this->moveTest($source, $dest, $updatables, $deletables); } /** * @dataProvider moveSuccessProvider */ - public function testMoveSuccess($source, $dest, $updatables) { - $this->moveTest($source, $dest, $updatables); + public function testMoveSuccess($source, $dest, $updatables, $deletables) { + $this->moveTest($source, $dest, $updatables, $deletables); $this->assertTrue(true); } function moveFailedProvider() { return array( - array('a/b', 'a/c', array('a' => false, 'a/b' => false, 'a/c' => false)), - array('a/b', 'b/b', array('a' => false, 'a/b' => false, 'b' => false, 'b/b' => false)), - array('a/b', 'b/b', array('a' => false, 'a/b' => true, 'b' => false, 'b/b' => false)), - array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => false, 'b/b' => false)), + array('a/b', 'a/c', array('a' => false, 'a/b' => false, 'a/c' => false), array()), + array('a/b', 'b/b', array('a' => false, 'a/b' => false, 'b' => false, 'b/b' => false), array()), + array('a/b', 'b/b', array('a' => false, 'a/b' => true, 'b' => false, 'b/b' => false), array()), + array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => false, 'b/b' => false), array()), + array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => true, 'b/b' => false), array('a/b' => false)), ); } function moveSuccessProvider() { return array( - array('a/b', 'a/c', array('a' => false, 'a/b' => true, 'a/c' => false)), - array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => true, 'b/b' => false)), + array('a/b', 'a/c', array('a' => false, 'a/b' => true, 'a/c' => false), array()), + array('a/b', 'b/b', array('a' => true, 'a/b' => true, 'b' => true, 'b/b' => false), array('a/b' => true)), ); } @@ -68,7 +74,7 @@ class ObjectTree extends PHPUnit_Framework_TestCase { * @param $dest * @param $updatables */ - private function moveTest($source, $dest, $updatables) { + private function moveTest($source, $dest, $updatables, $deletables) { $rootDir = new OC_Connector_Sabre_Directory(''); $objectTree = $this->getMock('\OC\Connector\Sabre\ObjectTree', array('nodeExists', 'getNodeForPath'), @@ -80,7 +86,7 @@ class ObjectTree extends PHPUnit_Framework_TestCase { ->will($this->returnValue(false)); /** @var $objectTree \OC\Connector\Sabre\ObjectTree */ - $objectTree->fileView = new TestDoubleFileView($updatables); + $objectTree->fileView = new TestDoubleFileView($updatables, $deletables); $objectTree->move($source, $dest); } -- cgit v1.2.3 From 6c45fab0375bd8e6c02ba3081da7442b6d783c86 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Oct 2013 12:00:32 +0200 Subject: part file needs to have .part extension --- lib/private/connector/sabre/file.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/private/connector') diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 43e25de40c7..037dba7f37b 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -86,7 +86,7 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D // root folder because we can't create new files in /shared // we extend the name with a random number to avoid overwriting a existing file if (dirname($partpath) === 'Shared') { - $partpath = pathinfo($partpath, PATHINFO_FILENAME) . rand(); + $partpath = pathinfo($partpath, PATHINFO_FILENAME) . rand() . '.part'; } try { -- cgit v1.2.3 From caa27824a94e3af757ea33b01b5682ef963ae714 Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Tue, 8 Oct 2013 15:04:31 +0200 Subject: catch specific file exceptions and convert them to proper http status code via webdav --- lib/private/connector/sabre/file.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'lib/private/connector') diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 3c19da7c618..84154ee855a 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -87,14 +87,21 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D throw new Sabre_DAV_Exception(); } } catch (\OCP\Files\NotPermittedException $e) { - throw new Sabre_DAV_Exception_Forbidden(); + // a more general case - due to whatever reason the content could not be written + throw new Sabre_DAV_Exception_Forbidden($e->getMessage()); + } catch (\OCP\Files\EntityTooLargeException $e) { - throw new OC_Connector_Sabre_Exception_EntityTooLarge(); + // the file is too big to be stored + throw new OC_Connector_Sabre_Exception_EntityTooLarge($e->getMessage()); + } catch (\OCP\Files\InvalidContentException $e) { - throw new OC_Connector_Sabre_Exception_UnsupportedMediaType(); + // the file content is not permitted + throw new OC_Connector_Sabre_Exception_UnsupportedMediaType($e->getMessage()); + } catch (\OCP\Files\InvalidPathException $e) { - // TODO: add specific exception here - throw new Sabre_DAV_Exception_Forbidden(); + // the path for the file was not valid + // TODO: find proper http status code for this case + throw new Sabre_DAV_Exception_Forbidden($e->getMessage()); } // rename to correct path -- cgit v1.2.3 From 3f27fefdbe1342701161bf0949dd719f34dab76d Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Tue, 8 Oct 2013 15:40:42 +0200 Subject: fixing status code and formatting --- .../connector/sabre/exception/entitytoolarge.php | 19 +++++++++---------- .../sabre/exception/unsupportedmediatype.php | 16 ++++++++-------- 2 files changed, 17 insertions(+), 18 deletions(-) (limited to 'lib/private/connector') diff --git a/lib/private/connector/sabre/exception/entitytoolarge.php b/lib/private/connector/sabre/exception/entitytoolarge.php index aa9b37a0496..2bda51f2f3e 100644 --- a/lib/private/connector/sabre/exception/entitytoolarge.php +++ b/lib/private/connector/sabre/exception/entitytoolarge.php @@ -1,23 +1,22 @@ Date: Wed, 9 Oct 2013 15:35:09 +0200 Subject: due to internal implementations touch will always be successful - $mtime will be stored in the cache from desktop client perspective it is necessary to set the mtime under every condition --- lib/private/connector/sabre/node.php | 6 ------ 1 file changed, 6 deletions(-) (limited to 'lib/private/connector') diff --git a/lib/private/connector/sabre/node.php b/lib/private/connector/sabre/node.php index fa27abb381a..c38e9f86375 100644 --- a/lib/private/connector/sabre/node.php +++ b/lib/private/connector/sabre/node.php @@ -147,12 +147,6 @@ abstract class OC_Connector_Sabre_Node implements Sabre_DAV_INode, Sabre_DAV_IPr * Even if the modification time is set to a custom value the access time is set to now. */ public function touch($mtime) { - - // touch is only allowed if the update privilege is granted - if (!\OC\Files\Filesystem::isUpdatable($this->path)) { - throw new \Sabre_DAV_Exception_Forbidden(); - } - \OC\Files\Filesystem::touch($this->path, $mtime); } -- cgit v1.2.3