]> source.dussan.org Git - nextcloud-server.git/commitdiff
Upload abortion is now detected within the OC_Connector_Sabre_File::put()
authorThomas Müller <thomas.mueller@tmit.eu>
Tue, 8 Jul 2014 07:44:46 +0000 (09:44 +0200)
committerThomas Müller <thomas.mueller@tmit.eu>
Tue, 8 Jul 2014 09:09:59 +0000 (11:09 +0200)
OC_Connector_Sabre_AbortedUploadDetectionPlugin is pointless

Adding unit test testUploadAbort()

apps/files/appinfo/remote.php
apps/files_sharing/publicwebdav.php
lib/private/connector/sabre/aborteduploaddetectionplugin.php [deleted file]
lib/private/connector/sabre/file.php
tests/lib/connector/sabre/aborteduploaddetectionplugin.php [deleted file]
tests/lib/connector/sabre/file.php

index dd5c470431a556498903c3f43241eb6d57fcb335..3ba25085bad3a0499091f7f57127256804ca9fdd 100644 (file)
@@ -53,7 +53,6 @@ $server->subscribeEvent('beforeMethod', function () use ($server, $objectTree) {
        $rootDir = new OC_Connector_Sabre_Directory($view, $rootInfo);
        $objectTree->init($rootDir, $view, $mountManager);
 
-       $server->addPlugin(new OC_Connector_Sabre_AbortedUploadDetectionPlugin($view));
        $server->addPlugin(new OC_Connector_Sabre_QuotaPlugin($view));
 }, 30); // priority 30: after auth (10) and acl(20), before lock(50) and handling the request
 
index 684edd976705b0165d363528fd8c7aa87be4e20b..03e43967a4042873901aa855be8eb7db327d5179 100644 (file)
@@ -67,7 +67,6 @@ $server->subscribeEvent('beforeMethod', function () use ($server, $objectTree, $
        $mountManager = \OC\Files\Filesystem::getMountManager();
        $objectTree->init($root, $view, $mountManager);
 
-       $server->addPlugin(new OC_Connector_Sabre_AbortedUploadDetectionPlugin($view));
        $server->addPlugin(new OC_Connector_Sabre_QuotaPlugin($view));
 }, 30); // priority 30: after auth (10) and acl(20), before lock(50) and handling the request
 
diff --git a/lib/private/connector/sabre/aborteduploaddetectionplugin.php b/lib/private/connector/sabre/aborteduploaddetectionplugin.php
deleted file mode 100644 (file)
index b569f9a..0000000
+++ /dev/null
@@ -1,98 +0,0 @@
-<?php
-/**
- * Copyright (c) 2013 Thomas Müller <thomas.mueller@tmit.eu>
- * 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;
-
-       /**
-        * @var \OC\Files\View
-        */
-       private $fileView;
-
-       /**
-        * @param \OC\Files\View $view
-        */
-       public function __construct($view) {
-               $this->fileView = $view;
-       }
-
-       /**
-        * 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
-        */
-       public function initialize(\Sabre\DAV\Server $server) {
-
-               $this->server = $server;
-
-               $server->subscribeEvent('afterCreateFile', array($this, 'verifyContentLength'), 10);
-               $server->subscribeEvent('afterWriteContent', array($this, 'verifyContentLength'), 10);
-       }
-
-       /**
-        * @param string $filePath
-        * @param \Sabre\DAV\INode $node
-        * @throws \Sabre\DAV\Exception\BadRequest
-        */
-       public function verifyContentLength($filePath, \Sabre\DAV\INode $node = null) {
-
-               // we should only react on PUT which is used for upload
-               // e.g. with LOCK this will not work, but LOCK uses createFile() as well
-               if ($this->server->httpRequest->getMethod() !== 'PUT') {
-                       return;
-               }
-
-               // ownCloud chunked upload will be handled in its own plugin
-               $chunkHeader = $this->server->httpRequest->getHeader('OC-Chunked');
-               if ($chunkHeader) {
-                       return;
-               }
-
-               // compare expected and actual size
-               $expected = $this->getLength();
-               if (!$expected) {
-                       return;
-               }
-               $actual = $this->fileView->filesize($filePath);
-               if ($actual != $expected) {
-                       $this->fileView->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;
-       }
-}
index 7591cc5c0669416bdb748b430cac24ec35235c95..ece6885f24f0030e2e5449c5f8d5bdacd384084c 100644 (file)
@@ -71,13 +71,13 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements \Sabre\
                }
 
                // mark file as partial while uploading (ignored by the scanner)
-               $partpath = $this->path . '.ocTransferId' . rand() . '.part';
+               $partFilePath = $this->path . '.ocTransferId' . rand() . '.part';
 
                try {
-                       $putOkay = $this->fileView->file_put_contents($partpath, $data);
+                       $putOkay = $this->fileView->file_put_contents($partFilePath, $data);
                        if ($putOkay === false) {
                                \OC_Log::write('webdav', '\OC\Files\Filesystem::file_put_contents() failed', \OC_Log::ERROR);
-                               $this->fileView->unlink($partpath);
+                               $this->fileView->unlink($partFilePath);
                                // because we have no clue about the cause we can only throw back a 500/Internal Server Error
                                throw new \Sabre\DAV\Exception('Could not write file contents');
                        }
@@ -102,13 +102,22 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements \Sabre\
                        throw new OC_Connector_Sabre_Exception_FileLocked($e->getMessage(), $e->getCode(), $e);
                }
 
+               // double check if the file was fully received
+               // compare expected and actual size
+               $expected = $_SERVER['CONTENT_LENGTH'];
+               $actual = $this->fileView->filesize($partFilePath);
+               if ($actual != $expected) {
+                       $this->fileView->unlink($partFilePath);
+                       throw new \Sabre\DAV\Exception\BadRequest('expected filesize ' . $expected . ' got ' . $actual);
+               }
+
                // rename to correct path
                try {
-                       $renameOkay = $this->fileView->rename($partpath, $this->path);
+                       $renameOkay = $this->fileView->rename($partFilePath, $this->path);
                        $fileExists = $this->fileView->file_exists($this->path);
                        if ($renameOkay === false || $fileExists === false) {
                                \OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR);
-                               $this->fileView->unlink($partpath);
+                               $this->fileView->unlink($partFilePath);
                                throw new \Sabre\DAV\Exception('Could not rename part file to final file');
                        }
                }
@@ -259,5 +268,4 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements \Sabre\
 
                return null;
        }
-
 }
diff --git a/tests/lib/connector/sabre/aborteduploaddetectionplugin.php b/tests/lib/connector/sabre/aborteduploaddetectionplugin.php
deleted file mode 100644 (file)
index 7e9f70d..0000000
+++ /dev/null
@@ -1,100 +0,0 @@
-<?php
-
-/**
- * Copyright (c) 2013 Thomas Müller <thomas.mueller@tmit.eu>
- * 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;
-
-       private function init($view) {
-               $this->server = new \Sabre\DAV\Server();
-               $this->plugin = new OC_Connector_Sabre_AbortedUploadDetectionPlugin($view);
-               $this->plugin->initialize($this->server);
-       }
-
-       /**
-        * @dataProvider lengthProvider
-        */
-       public function testLength($expected, $headers) {
-               $this->init(null);
-
-               $this->server->httpRequest = new \Sabre\HTTP\Request($headers);
-               $length = $this->plugin->getLength();
-               $this->assertEquals($expected, $length);
-       }
-
-       /**
-        * @dataProvider verifyContentLengthProvider
-        */
-       public function testVerifyContentLength($method, $fileSize, $headers) {
-               $this->init($this->buildFileViewMock($fileSize));
-
-               $headers['REQUEST_METHOD'] = $method;
-               $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($method, $fileSize, $headers) {
-               $view = $this->buildFileViewMock($fileSize);
-               $this->init($view);
-               // we expect unlink to be called
-               $view->expects($this->once())->method('unlink');
-
-               $headers['REQUEST_METHOD'] = $method;
-               $this->server->httpRequest = new Sabre\HTTP\Request($headers);
-               $this->plugin->verifyContentLength('foo.txt');
-       }
-
-       public function verifyContentLengthProvider() {
-               return array(
-                       array('PUT', 1024, array()),
-                       array('PUT', 1024, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')),
-                       array('PUT', 512, array('HTTP_CONTENT_LENGTH' => '512')),
-                       array('LOCK', 1024, array()),
-                       array('LOCK', 1024, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')),
-                       array('LOCK', 512, array('HTTP_CONTENT_LENGTH' => '512')),
-               );
-       }
-
-       public function verifyContentLengthFailedProvider() {
-               return array(
-                       array('PUT', 1025, array('HTTP_X_EXPECTED_ENTITY_LENGTH' => '1024')),
-                       array('PUT', 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 filesystem
-               $view = $this->getMock('\OC\Files\View', array('filesize', 'unlink'), array(), '', false);
-               $view->expects($this->any())->method('filesize')->withAnyParameters()->will($this->returnValue($fileSize));
-
-               return $view;
-       }
-
-}
index 3dd5b328f4677d3d6b1d395f365ba3bbb3139000..ba4e775813b16e54fb481015e0e719e8ea3d369b 100644 (file)
@@ -29,7 +29,7 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase {
                $file = new OC_Connector_Sabre_File($view, $info);
 
                // action
-               $etag = $file->put('test data');
+               $file->put('test data');
        }
 
        /**
@@ -37,7 +37,9 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase {
         */
        public function testSimplePutFailsOnRename() {
                // setup
-               $view = $this->getMock('\OC\Files\View', array('file_put_contents', 'rename', 'getRelativePath'), array(), '', false);
+               $view = $this->getMock('\OC\Files\View',
+                       array('file_put_contents', 'rename', 'getRelativePath', 'filesize'),
+                       array(), '', false);
                $view->expects($this->any())
                        ->method('file_put_contents')
                        ->withAnyParameters()
@@ -46,10 +48,14 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase {
                        ->method('rename')
                        ->withAnyParameters()
                        ->will($this->returnValue(false));
-
                $view->expects($this->any())
                        ->method('getRelativePath')
                        ->will($this->returnValue('/test.txt'));
+               $view->expects($this->any())
+                       ->method('filesize')
+                       ->will($this->returnValue(123456));
+
+               $_SERVER['CONTENT_LENGTH'] = 123456;
 
                $info = new \OC\Files\FileInfo('/test.txt', null, null, array(
                        'permissions' => \OCP\PERMISSION_ALL
@@ -58,7 +64,7 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase {
                $file = new OC_Connector_Sabre_File($view, $info);
 
                // action
-               $etag = $file->put('test data');
+               $file->put('test data');
        }
 
        /**
@@ -81,7 +87,7 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase {
                $file = new OC_Connector_Sabre_File($view, $info);
 
                // action
-               $etag = $file->put('test data');
+               $file->put('test data');
        }
 
        /**
@@ -102,4 +108,39 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase {
                $file = new OC_Connector_Sabre_File($view, $info);
                $file->setName('/super*star.txt');
        }
+
+       /**
+        * @expectedException \Sabre\DAV\Exception\BadRequest
+        */
+       public function testUploadAbort() {
+               // setup
+               $view = $this->getMock('\OC\Files\View',
+                       array('file_put_contents', 'rename', 'getRelativePath', 'filesize'),
+                       array(), '', false);
+               $view->expects($this->any())
+                       ->method('file_put_contents')
+                       ->withAnyParameters()
+                       ->will($this->returnValue(true));
+               $view->expects($this->any())
+                       ->method('rename')
+                       ->withAnyParameters()
+                       ->will($this->returnValue(false));
+               $view->expects($this->any())
+                       ->method('getRelativePath')
+                       ->will($this->returnValue('/test.txt'));
+               $view->expects($this->any())
+                       ->method('filesize')
+                       ->will($this->returnValue(123456));
+
+               $_SERVER['CONTENT_LENGTH'] = 12345;
+
+               $info = new \OC\Files\FileInfo('/test.txt', null, null, array(
+                       'permissions' => \OCP\PERMISSION_ALL
+               ));
+
+               $file = new OC_Connector_Sabre_File($view, $info);
+
+               // action
+               $file->put('test data');
+       }
 }