[Pkg-owncloud-commits] [owncloud] 23/70: Upload abortion is now detected within the OC_Connector_Sabre_File::put()
David Prévot
taffit at moszumanska.debian.org
Thu Aug 21 17:40:01 UTC 2014
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to annotated tag v6.0.5RC1
in repository owncloud.
commit a8ec0a7ccc55df2b72009b49a30fa0da9d99a31d
Author: Thomas Müller <thomas.mueller at tmit.eu>
Date: Tue Jul 8 09:44:46 2014 +0200
Upload abortion is now detected within the OC_Connector_Sabre_File::put()
OC_Connector_Sabre_AbortedUploadDetectionPlugin is pointless
Adding unit test testUploadAbort()
Backport of ea269f0 from master
---
apps/files/appinfo/remote.php | 1 -
.../sabre/aborteduploaddetectionplugin.php | 107 ---------------------
lib/private/connector/sabre/file.php | 26 +++--
.../sabre/aborteduploaddetectionplugin.php | 101 -------------------
tests/lib/connector/sabre/file.php | 52 +++++++++-
5 files changed, 64 insertions(+), 223 deletions(-)
diff --git a/apps/files/appinfo/remote.php b/apps/files/appinfo/remote.php
index ef22fe9..78eb1a1 100644
--- a/apps/files/appinfo/remote.php
+++ b/apps/files/appinfo/remote.php
@@ -49,7 +49,6 @@ $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_FilesPlugin());
-$server->addPlugin(new OC_Connector_Sabre_AbortedUploadDetectionPlugin());
$server->addPlugin(new OC_Connector_Sabre_QuotaPlugin());
$server->addPlugin(new OC_Connector_Sabre_MaintenancePlugin());
$server->addPlugin(new OC_Connector_Sabre_ExceptionLoggerPlugin('webdav'));
diff --git a/lib/private/connector/sabre/aborteduploaddetectionplugin.php b/lib/private/connector/sabre/aborteduploaddetectionplugin.php
deleted file mode 100644
index 10cca64..0000000
--- a/lib/private/connector/sabre/aborteduploaddetectionplugin.php
+++ /dev/null
@@ -1,107 +0,0 @@
-<?php
-/**
- * Copyright (c) 2013 Thomas Müller <thomas.mueller at 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;
-
- /**
- * 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
- */
- 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 $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->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/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php
index bc528cd..aa93d25 100644
--- a/lib/private/connector/sabre/file.php
+++ b/lib/private/connector/sabre/file.php
@@ -64,20 +64,20 @@ 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 . '.ocTransferId' . rand() . '.part';
+ $partFilePath = $this->path . '.ocTransferId' . rand() . '.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() . '.part';
+ if (dirname($partFilePath) === 'Shared') {
+ $partFilePath = pathinfo($partFilePath, PATHINFO_FILENAME) . rand() . '.part';
}
try {
- $putOkay = $fs->file_put_contents($partpath, $data);
+ $putOkay = $fs->file_put_contents($partFilePath, $data);
if ($putOkay === false) {
\OC_Log::write('webdav', '\OC\Files\Filesystem::file_put_contents() failed', \OC_Log::ERROR);
- $fs->unlink($partpath);
+ $fs->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,14 +102,23 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D
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 = $fs->filesize($partFilePath);
+ if ($actual != $expected) {
+ $fs->unlink($partFilePath);
+ throw new \Sabre_DAV_Exception_BadRequest('expected filesize ' . $expected . ' got ' . $actual);
+ }
+
// rename to correct path
try {
- $renameOkay = $fs->rename($partpath, $this->path);
+ $renameOkay = $fs->rename($partFilePath, $this->path);
$fileExists = $fs->file_exists($this->path);
if ($renameOkay === false || $fileExists === false) {
\OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR);
- $fs->unlink($partpath);
- throw new Sabre_DAV_Exception('Could not rename part file to final file');
+ $fs->unlink($partFilePath);
+ throw new \Sabre_DAV_Exception('Could not rename part file to final file');
}
}
catch (\OCP\Files\LockNotAcquiredException $e) {
@@ -272,5 +281,4 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D
return null;
}
-
}
diff --git a/tests/lib/connector/sabre/aborteduploaddetectionplugin.php b/tests/lib/connector/sabre/aborteduploaddetectionplugin.php
deleted file mode 100644
index 201f126..0000000
--- a/tests/lib/connector/sabre/aborteduploaddetectionplugin.php
+++ /dev/null
@@ -1,101 +0,0 @@
-<?php
-/**
- * Copyright (c) 2013 Thomas Müller <thomas.mueller at 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;
-
- 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($method, $fileSize, $headers)
- {
- $this->plugin->fileView = $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)
- {
- $this->plugin->fileView = $this->buildFileViewMock($fileSize);
-
- // we expect unlink to be called
- $this->plugin->fileView->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;
- }
-
-}
diff --git a/tests/lib/connector/sabre/file.php b/tests/lib/connector/sabre/file.php
index e1fed03..1928b50 100644
--- a/tests/lib/connector/sabre/file.php
+++ b/tests/lib/connector/sabre/file.php
@@ -18,7 +18,7 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase {
$file->fileView->expects($this->any())->method('file_put_contents')->withAnyParameters()->will($this->returnValue(false));
// action
- $etag = $file->put('test data');
+ $file->put('test data');
}
/**
@@ -27,12 +27,26 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase {
public function testSimplePutFailsOnRename() {
// setup
$file = new OC_Connector_Sabre_File('/test.txt');
- $file->fileView = $this->getMock('\OC\Files\View', array('file_put_contents', 'rename'), array(), '', FALSE);
- $file->fileView->expects($this->any())->method('file_put_contents')->withAnyParameters()->will($this->returnValue(true));
- $file->fileView->expects($this->any())->method('rename')->withAnyParameters()->will($this->returnValue(false));
+ $file->fileView = $this->getMock('\OC\Files\View',
+ array('file_put_contents', 'rename', 'filesize'),
+ array(), '', FALSE);
+ $file->fileView->expects($this->any())
+ ->method('file_put_contents')
+ ->withAnyParameters()
+ ->will($this->returnValue(true));
+ $file->fileView->expects($this->any())
+ ->method('rename')
+ ->withAnyParameters()
+ ->will($this->returnValue(false));
+ $file->fileView->expects($this->any())
+ ->method('filesize')
+ ->withAnyParameters()
+ ->will($this->returnValue(123456));
+
+ $_SERVER['CONTENT_LENGTH'] = 123456;
// action
- $etag = $file->put('test data');
+ $file->put('test data');
}
/**
@@ -42,4 +56,32 @@ class Test_OC_Connector_Sabre_File extends PHPUnit_Framework_TestCase {
$file = new OC_Connector_Sabre_File('Shared');
$file->delete();
}
+
+ /**
+ * @expectedException Sabre_DAV_Exception_BadRequest
+ */
+ public function testUploadAbort() {
+ // setup
+ $file = new OC_Connector_Sabre_File('/test.txt');
+ $file->fileView = $this->getMock('\OC\Files\View',
+ array('file_put_contents', 'rename', 'filesize'),
+ array(), '', FALSE);
+ $file->fileView->expects($this->any())
+ ->method('file_put_contents')
+ ->withAnyParameters()
+ ->will($this->returnValue(true));
+ $file->fileView->expects($this->any())
+ ->method('rename')
+ ->withAnyParameters()
+ ->will($this->returnValue(false));
+ $file->fileView->expects($this->any())
+ ->method('filesize')
+ ->withAnyParameters()
+ ->will($this->returnValue(123456));
+
+ $_SERVER['CONTENT_LENGTH'] = 12345;
+
+ // action
+ $file->put('test data');
+ }
}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list