From 09b64535a9099bdf9c71fa96b3aab2e49206ffde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 25 Sep 2013 17:00:20 +0200 Subject: [PATCH 1/6] fixing copyright and add class documentation --- apps/files/appinfo/remote.php | 1 + .../sabre/aborteduploaddetectionplugin.php | 105 ++++++++++++++++++ lib/connector/sabre/directory.php | 13 --- 3 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 lib/connector/sabre/aborteduploaddetectionplugin.php diff --git a/apps/files/appinfo/remote.php b/apps/files/appinfo/remote.php index 9b114ca2e3..0c1f2e6580 100644 --- a/apps/files/appinfo/remote.php +++ b/apps/files/appinfo/remote.php @@ -48,6 +48,7 @@ $defaults = new OC_Defaults(); $server->addPlugin(new Sabre_DAV_Auth_Plugin($authBackend, $defaults->getName())); $server->addPlugin(new Sabre_DAV_Locks_Plugin($lockBackend)); $server->addPlugin(new Sabre_DAV_Browser_Plugin(false)); // Show something in the Browser, but no upload +$server->addPlugin(new OC_Connector_Sabre_AbortedUploadDetectionPlugin()); $server->addPlugin(new OC_Connector_Sabre_QuotaPlugin()); $server->addPlugin(new OC_Connector_Sabre_MaintenancePlugin()); diff --git a/lib/connector/sabre/aborteduploaddetectionplugin.php b/lib/connector/sabre/aborteduploaddetectionplugin.php new file mode 100644 index 0000000000..745c4a9942 --- /dev/null +++ b/lib/connector/sabre/aborteduploaddetectionplugin.php @@ -0,0 +1,105 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +/** + * Class OC_Connector_Sabre_AbortedUploadDetectionPlugin + * + * This plugin will verify if the uploaded data has been stored completely. + * This is done by comparing the content length of the request with the file size on storage. + */ +class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPlugin { + + /** + * Reference to main server object + * + * @var Sabre_DAV_Server + */ + private $server; + + /** + * 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; + + $server->subscribeEvent('afterCreateFile', array($this, 'afterCreateFile'), 10); + $server->subscribeEvent('afterWriteContent', array($this, 'afterWriteContent'), 10); + } + + function afterCreateFile($path, Sabre_DAV_ICollection $parent) { + + $this->verifyContentLength($path); + + } + + function afterWriteContent($path, Sabre_DAV_IFile $node) { + $path = $path .'/'.$node->getName(); + $this->verifyContentLength($path); + } + + function verifyContentLength($filePath) { + + // ownCloud chunked upload will be handled in it's own plugin + $chunkHeader = $this->server->httpRequest->getHeader('OC-Chunked'); + if ($chunkHeader) { + return; + } + + // compare expected and actual size + $expected = $this->getLength(); + $actual = $this->getFileView()->filesize($filePath); + if ($actual != $expected) { + $this->getFileView()->unlink($filePath); + throw new Sabre_DAV_Exception_BadRequest('expected filesize ' . $expected . ' got ' . $actual); + } + + } + + /** + * @return string + */ + public function getLength() + { + $req = $this->server->httpRequest; + $length = $req->getHeader('X-Expected-Entity-Length'); + if (!$length) { + $length = $req->getHeader('Content-Length'); + } + + return $length; + } + + /** + * @return \OC\Files\View + */ + public function getFileView() + { + if (is_null($this->fileView)) { + // initialize fileView + $this->fileView = \OC\Files\Filesystem::getView(); + } + + return $this->fileView; + } +} diff --git a/lib/connector/sabre/directory.php b/lib/connector/sabre/directory.php index 3181a4b310..29374f7a6c 100644 --- a/lib/connector/sabre/directory.php +++ b/lib/connector/sabre/directory.php @@ -74,19 +74,6 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa \OC\Files\Filesystem::file_put_contents($partpath, $data); - //detect aborted upload - if (isset ($_SERVER['REQUEST_METHOD']) && $_SERVER['REQUEST_METHOD'] === 'PUT' ) { - if (isset($_SERVER['CONTENT_LENGTH'])) { - $expected = $_SERVER['CONTENT_LENGTH']; - $actual = \OC\Files\Filesystem::filesize($partpath); - if ($actual != $expected) { - \OC\Files\Filesystem::unlink($partpath); - throw new Sabre_DAV_Exception_BadRequest( - 'expected filesize ' . $expected . ' got ' . $actual); - } - } - } - // rename to correct path \OC\Files\Filesystem::rename($partpath, $newPath); From 5e27ac4b1ade3a78941ced1eef5aaa2d2df0cedf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 25 Sep 2013 17:17:29 +0200 Subject: [PATCH 2/6] $path already contains the full path to the file --- lib/connector/sabre/aborteduploaddetectionplugin.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/connector/sabre/aborteduploaddetectionplugin.php b/lib/connector/sabre/aborteduploaddetectionplugin.php index 745c4a9942..1173ff2f9a 100644 --- a/lib/connector/sabre/aborteduploaddetectionplugin.php +++ b/lib/connector/sabre/aborteduploaddetectionplugin.php @@ -54,7 +54,6 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl } function afterWriteContent($path, Sabre_DAV_IFile $node) { - $path = $path .'/'.$node->getName(); $this->verifyContentLength($path); } From 0c44cdd4eaa82687bce4dbbb24338b16a3f4ed13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 25 Sep 2013 17:28:45 +0200 Subject: [PATCH 3/6] remove unneccessary code --- .../sabre/aborteduploaddetectionplugin.php | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/connector/sabre/aborteduploaddetectionplugin.php b/lib/connector/sabre/aborteduploaddetectionplugin.php index 1173ff2f9a..74c26f41b6 100644 --- a/lib/connector/sabre/aborteduploaddetectionplugin.php +++ b/lib/connector/sabre/aborteduploaddetectionplugin.php @@ -43,21 +43,16 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl $this->server = $server; - $server->subscribeEvent('afterCreateFile', array($this, 'afterCreateFile'), 10); - $server->subscribeEvent('afterWriteContent', array($this, 'afterWriteContent'), 10); + $server->subscribeEvent('afterCreateFile', array($this, 'verifyContentLength'), 10); + $server->subscribeEvent('afterWriteContent', array($this, 'verifyContentLength'), 10); } - function afterCreateFile($path, Sabre_DAV_ICollection $parent) { - - $this->verifyContentLength($path); - - } - - function afterWriteContent($path, Sabre_DAV_IFile $node) { - $this->verifyContentLength($path); - } - - function verifyContentLength($filePath) { + /** + * @param $filePath + * @param Sabre_DAV_INode $node + * @throws Sabre_DAV_Exception_BadRequest + */ + public function verifyContentLength($filePath, Sabre_DAV_INode $node = null) { // ownCloud chunked upload will be handled in it's own plugin $chunkHeader = $this->server->httpRequest->getHeader('OC-Chunked'); @@ -67,6 +62,9 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl // compare expected and actual size $expected = $this->getLength(); + if (!$expected) { + return; + } $actual = $this->getFileView()->filesize($filePath); if ($actual != $expected) { $this->getFileView()->unlink($filePath); From 3fa5271f10369645564f19a5706e23e0660bbbc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 25 Sep 2013 17:34:28 +0200 Subject: [PATCH 4/6] adding unit tests --- .../sabre/aborteduploaddetectionplugin.php | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 tests/lib/connector/sabre/aborteduploaddetectionplugin.php diff --git a/tests/lib/connector/sabre/aborteduploaddetectionplugin.php b/tests/lib/connector/sabre/aborteduploaddetectionplugin.php new file mode 100644 index 0000000000..8237bdbd9e --- /dev/null +++ b/tests/lib/connector/sabre/aborteduploaddetectionplugin.php @@ -0,0 +1,93 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +class Test_OC_Connector_Sabre_AbortedUploadDetectionPlugin extends PHPUnit_Framework_TestCase { + + /** + * @var Sabre_DAV_Server + */ + private $server; + + /** + * @var OC_Connector_Sabre_AbortedUploadDetectionPlugin + */ + private $plugin; + + public function setUp() { + $this->server = new Sabre_DAV_Server(); + $this->plugin = new OC_Connector_Sabre_AbortedUploadDetectionPlugin(); + $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); + } + + /** + * @dataProvider verifyContentLengthProvider + */ + public function testVerifyContentLength($fileSize, $headers) + { + $this->plugin->fileView = $this->buildFileViewMock($fileSize); + + $this->server->httpRequest = new Sabre_HTTP_Request($headers); + $this->plugin->verifyContentLength('foo.txt'); + $this->assertTrue(true); + } + + /** + * @dataProvider verifyContentLengthFailedProvider + * @expectedException Sabre_DAV_Exception_BadRequest + */ + public function testVerifyContentLengthFailed($fileSize, $headers) + { + $this->plugin->fileView = $this->buildFileViewMock($fileSize); + + $this->server->httpRequest = new Sabre_HTTP_Request($headers); + $this->plugin->verifyContentLength('foo.txt'); + } + + public function verifyContentLengthProvider() { + return array( + array(1024, array()), + array(1024, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')), + array(512, array('HTTP_CONTENT_LENGTH' => '512')), + ); + } + + public function verifyContentLengthFailedProvider() { + return array( + array(1025, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')), + array(525, array('HTTP_CONTENT_LENGTH' => '512')), + ); + } + + 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_X_EXPECTED_ENTITY_LENGTH' => '2048', 'HTTP_CONTENT_LENGTH' => '1024')), + ); + } + + private function buildFileViewMock($fileSize) { + // mock filesysten + $view = $this->getMock('\OC\Files\View', array('filesize', 'unlink'), array(), '', FALSE); + $view->expects($this->any())->method('filesize')->withAnyParameters()->will($this->returnValue($fileSize)); + + return $view; + } + +} From 826c6bec8f9a8137e0a2e7660389c728b59f95d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Wed, 25 Sep 2013 17:41:16 +0200 Subject: [PATCH 5/6] expect unlinkto be called --- tests/lib/connector/sabre/aborteduploaddetectionplugin.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/lib/connector/sabre/aborteduploaddetectionplugin.php b/tests/lib/connector/sabre/aborteduploaddetectionplugin.php index 8237bdbd9e..bef0e4c4d7 100644 --- a/tests/lib/connector/sabre/aborteduploaddetectionplugin.php +++ b/tests/lib/connector/sabre/aborteduploaddetectionplugin.php @@ -54,6 +54,10 @@ class Test_OC_Connector_Sabre_AbortedUploadDetectionPlugin extends PHPUnit_Frame { $this->plugin->fileView = $this->buildFileViewMock($fileSize); + // we expect unlink to be called + $this->plugin->fileView->expects($this->once())->method('unlink'); + + $this->server->httpRequest = new Sabre_HTTP_Request($headers); $this->plugin->verifyContentLength('foo.txt'); } From aaba0d83b5e9f8b67a739e74172da9464fa562b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Mon, 30 Sep 2013 10:03:07 +0200 Subject: [PATCH 6/6] fixing PHPDoc & typo --- lib/connector/sabre/aborteduploaddetectionplugin.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/connector/sabre/aborteduploaddetectionplugin.php b/lib/connector/sabre/aborteduploaddetectionplugin.php index 74c26f41b6..15dca3a680 100644 --- a/lib/connector/sabre/aborteduploaddetectionplugin.php +++ b/lib/connector/sabre/aborteduploaddetectionplugin.php @@ -37,7 +37,6 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl * This method should set up the requires event subscriptions. * * @param Sabre_DAV_Server $server - * @return void */ public function initialize(Sabre_DAV_Server $server) { @@ -54,7 +53,7 @@ class OC_Connector_Sabre_AbortedUploadDetectionPlugin extends Sabre_DAV_ServerPl */ public function verifyContentLength($filePath, Sabre_DAV_INode $node = null) { - // ownCloud chunked upload will be handled in it's own plugin + // ownCloud chunked upload will be handled in its own plugin $chunkHeader = $this->server->httpRequest->getHeader('OC-Chunked'); if ($chunkHeader) { return;