diff options
Diffstat (limited to 'apps')
| -rw-r--r-- | apps/dav/bin/chunkperf.php | 4 | ||||
| -rw-r--r-- | apps/dav/composer/composer/autoload_classmap.php | 1 | ||||
| -rw-r--r-- | apps/dav/composer/composer/autoload_static.php | 1 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/File.php | 12 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/FilesPlugin.php | 54 | ||||
| -rw-r--r-- | apps/dav/lib/Connector/Sabre/Node.php | 14 | ||||
| -rw-r--r-- | apps/dav/lib/Server.php | 2 | ||||
| -rw-r--r-- | apps/dav/lib/Upload/ChunkingPlugin.php | 106 | ||||
| -rw-r--r-- | apps/dav/tests/unit/Connector/Sabre/FileTest.php | 4 | ||||
| -rw-r--r-- | apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php | 57 | ||||
| -rw-r--r-- | apps/dav/tests/unit/Upload/ChunkingPluginTest.php | 167 |
11 files changed, 302 insertions, 120 deletions
diff --git a/apps/dav/bin/chunkperf.php b/apps/dav/bin/chunkperf.php index bd976e74384..e2280b1e104 100644 --- a/apps/dav/bin/chunkperf.php +++ b/apps/dav/bin/chunkperf.php @@ -73,5 +73,7 @@ while(!feof($stream)) { $destination = pathinfo($file, PATHINFO_BASENAME); //echo "Moving $uploadUrl/.file to it's final destination $baseUri/files/$userName/$destination" . PHP_EOL; request($client, 'MOVE', "$uploadUrl/.file", null, [ - 'Destination' => "$baseUri/files/$userName/$destination" + 'Destination' => "$baseUri/files/$userName/$destination", + 'OC-Total-Length' => filesize($file), + 'X-OC-MTime' => filemtime($file) ]); diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index ddb8d7ca8d2..0687981a099 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -145,6 +145,7 @@ return array( 'OCA\\DAV\\SystemTag\\SystemTagsObjectTypeCollection' => $baseDir . '/../lib/SystemTag/SystemTagsObjectTypeCollection.php', 'OCA\\DAV\\SystemTag\\SystemTagsRelationsCollection' => $baseDir . '/../lib/SystemTag/SystemTagsRelationsCollection.php', 'OCA\\DAV\\Upload\\AssemblyStream' => $baseDir . '/../lib/Upload/AssemblyStream.php', + 'OCA\\DAV\\Upload\\ChunkingPlugin' => $baseDir . '/../lib/Upload/ChunkingPlugin.php', 'OCA\\DAV\\Upload\\FutureFile' => $baseDir . '/../lib/Upload/FutureFile.php', 'OCA\\DAV\\Upload\\RootCollection' => $baseDir . '/../lib/Upload/RootCollection.php', 'OCA\\DAV\\Upload\\UploadFolder' => $baseDir . '/../lib/Upload/UploadFolder.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 46707a93912..5c45a80e7dd 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -160,6 +160,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\SystemTag\\SystemTagsObjectTypeCollection' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagsObjectTypeCollection.php', 'OCA\\DAV\\SystemTag\\SystemTagsRelationsCollection' => __DIR__ . '/..' . '/../lib/SystemTag/SystemTagsRelationsCollection.php', 'OCA\\DAV\\Upload\\AssemblyStream' => __DIR__ . '/..' . '/../lib/Upload/AssemblyStream.php', + 'OCA\\DAV\\Upload\\ChunkingPlugin' => __DIR__ . '/..' . '/../lib/Upload/ChunkingPlugin.php', 'OCA\\DAV\\Upload\\FutureFile' => __DIR__ . '/..' . '/../lib/Upload/FutureFile.php', 'OCA\\DAV\\Upload\\RootCollection' => __DIR__ . '/..' . '/../lib/Upload/RootCollection.php', 'OCA\\DAV\\Upload\\UploadFolder' => __DIR__ . '/..' . '/../lib/Upload/UploadFolder.php', diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 32cc8b7adeb..597e6ebef90 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -589,18 +589,6 @@ class File extends Node implements IFile { throw new \Sabre\DAV\Exception($e->getMessage(), 0, $e); } - private function sanitizeMtime($mtimeFromRequest) { - // In PHP 5.X "is_numeric" returns true for strings in hexadecimal - // notation. This is no longer the case in PHP 7.X, so this check - // ensures that strings with hexadecimal notations fail too in PHP 5.X. - $isHexadecimal = is_string($mtimeFromRequest) && preg_match('/^\s*0[xX]/', $mtimeFromRequest); - if ($isHexadecimal || !is_numeric($mtimeFromRequest)) { - throw new \InvalidArgumentException('X-OC-MTime header must be an integer (unix timestamp).'); - } - - return intval($mtimeFromRequest); - } - /** * Get the checksum for this file * diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index daf9d2d2112..a3a0893259b 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -32,7 +32,7 @@ namespace OCA\DAV\Connector\Sabre; -use OC\Files\View; +use OC\AppFramework\Http\Request; use OCP\Files\ForbiddenException; use OCP\IPreview; use Sabre\DAV\Exception\Forbidden; @@ -47,7 +47,6 @@ use \Sabre\HTTP\ResponseInterface; use OCP\Files\StorageNotAvailableException; use OCP\IConfig; use OCP\IRequest; -use OCA\DAV\Upload\FutureFile; class FilesPlugin extends ServerPlugin { @@ -91,11 +90,6 @@ class FilesPlugin extends ServerPlugin { private $isPublic; /** - * @var View - */ - private $fileView; - - /** * @var bool */ private $downloadAttachment; @@ -183,7 +177,6 @@ class FilesPlugin extends ServerPlugin { } }); $this->server->on('beforeMove', [$this, 'checkMove']); - $this->server->on('beforeMove', [$this, 'beforeMoveFutureFile']); } /** @@ -258,9 +251,9 @@ class FilesPlugin extends ServerPlugin { $filename = $node->getName(); if ($this->request->isUserAgent( [ - \OC\AppFramework\Http\Request::USER_AGENT_IE, - \OC\AppFramework\Http\Request::USER_AGENT_ANDROID_MOBILE_CHROME, - \OC\AppFramework\Http\Request::USER_AGENT_FREEBOX, + Request::USER_AGENT_IE, + Request::USER_AGENT_ANDROID_MOBILE_CHROME, + Request::USER_AGENT_FREEBOX, ])) { $response->addHeader('Content-Disposition', 'attachment; filename="' . rawurlencode($filename) . '"'); } else { @@ -461,43 +454,4 @@ class FilesPlugin extends ServerPlugin { } } } - - /** - * Move handler for future file. - * - * This overrides the default move behavior to prevent Sabre - * to delete the target file before moving. Because deleting would - * lose the file id and metadata. - * - * @param string $path source path - * @param string $destination destination path - * @return bool|void false to stop handling, void to skip this handler - */ - public function beforeMoveFutureFile($path, $destination) { - $sourceNode = $this->tree->getNodeForPath($path); - if (!$sourceNode instanceof FutureFile) { - // skip handling as the source is not a chunked FutureFile - return; - } - - if (!$this->tree->nodeExists($destination)) { - // skip and let the default handler do its work - return; - } - - // do a move manually, skipping Sabre's default "delete" for existing nodes - $this->tree->move($path, $destination); - - // trigger all default events (copied from CorePlugin::move) - $this->server->emit('afterMove', [$path, $destination]); - $this->server->emit('afterUnbind', [$path]); - $this->server->emit('afterBind', [$destination]); - - $response = $this->server->httpResponse; - $response->setHeader('Content-Length', '0'); - $response->setStatus(204); - - return false; - } - } diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index 979336d86fe..b8a8209129c 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -165,6 +165,7 @@ abstract class Node implements \Sabre\DAV\INode { * Even if the modification time is set to a custom value the access time is set to now. */ public function touch($mtime) { + $mtime = $this->sanitizeMtime($mtime); $this->fileView->touch($this->path, $mtime); $this->refreshInfo(); } @@ -358,4 +359,17 @@ abstract class Node implements \Sabre\DAV\INode { public function getFileInfo() { return $this->info; } + + protected function sanitizeMtime($mtimeFromRequest) { + // In PHP 5.X "is_numeric" returns true for strings in hexadecimal + // notation. This is no longer the case in PHP 7.X, so this check + // ensures that strings with hexadecimal notations fail too in PHP 5.X. + $isHexadecimal = is_string($mtimeFromRequest) && preg_match('/^\s*0[xX]/', $mtimeFromRequest); + if ($isHexadecimal || !is_numeric($mtimeFromRequest)) { + throw new \InvalidArgumentException('X-OC-MTime header must be an integer (unix timestamp).'); + } + + return intval($mtimeFromRequest); + } + } diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 4e8a9fd0a51..dfa19467e5e 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -55,6 +55,7 @@ use OCA\DAV\DAV\CustomPropertiesBackend; use OCA\DAV\Connector\Sabre\QuotaPlugin; use OCA\DAV\Files\BrowserErrorPagePlugin; use OCA\DAV\SystemTag\SystemTagPlugin; +use OCA\DAV\Upload\ChunkingPlugin; use OCP\IRequest; use OCP\SabrePluginEvent; use Sabre\CardDAV\VCFExportPlugin; @@ -171,6 +172,7 @@ class Server { )); $this->server->addPlugin(new CopyEtagHeaderPlugin()); + $this->server->addPlugin(new ChunkingPlugin()); // allow setup of additional plugins $dispatcher->dispatch('OCA\DAV\Connector\Sabre::addPlugin', $event); diff --git a/apps/dav/lib/Upload/ChunkingPlugin.php b/apps/dav/lib/Upload/ChunkingPlugin.php new file mode 100644 index 00000000000..5768f53c2b4 --- /dev/null +++ b/apps/dav/lib/Upload/ChunkingPlugin.php @@ -0,0 +1,106 @@ +<?php +/** + * @author Thomas Müller <thomas.mueller@tmit.eu> + * + * @copyright Copyright (c) 2017, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see <http://www.gnu.org/licenses/> + * + */ + + +namespace OCA\DAV\Upload; + + +use OCA\DAV\Connector\Sabre\File; +use Sabre\DAV\Exception\BadRequest; +use Sabre\DAV\Server; +use Sabre\DAV\ServerPlugin; + +class ChunkingPlugin extends ServerPlugin { + + /** @var Server */ + private $server; + /** @var FutureFile */ + private $sourceNode; + + /** + * @inheritdoc + */ + function initialize(Server $server) { + $server->on('beforeMove', [$this, 'beforeMove']); + $this->server = $server; + } + + /** + * @param string $sourcePath source path + * @param string $destination destination path + */ + function beforeMove($sourcePath, $destination) { + $this->sourceNode = $this->server->tree->getNodeForPath($sourcePath); + if (!$this->sourceNode instanceof FutureFile) { + // skip handling as the source is not a chunked FutureFile + return; + } + + $this->verifySize(); + return $this->performMove($sourcePath, $destination); + } + + /** + * Move handler for future file. + * + * This overrides the default move behavior to prevent Sabre + * to delete the target file before moving. Because deleting would + * lose the file id and metadata. + * + * @param string $path source path + * @param string $destination destination path + * @return bool|void false to stop handling, void to skip this handler + */ + public function performMove($path, $destination) { + if (!$this->server->tree->nodeExists($destination)) { + // skip and let the default handler do its work + return; + } + + // do a move manually, skipping Sabre's default "delete" for existing nodes + $this->server->tree->move($path, $destination); + + // trigger all default events (copied from CorePlugin::move) + $this->server->emit('afterMove', [$path, $destination]); + $this->server->emit('afterUnbind', [$path]); + $this->server->emit('afterBind', [$destination]); + + $response = $this->server->httpResponse; + $response->setHeader('Content-Length', '0'); + $response->setStatus(204); + + return false; + } + + /** + * @throws BadRequest + */ + private function verifySize() { + $expectedSize = $this->server->httpRequest->getHeader('OC-Total-Length'); + if ($expectedSize === null) { + return; + } + $actualSize = $this->sourceNode->getSize(); + if ((int)$expectedSize !== $actualSize) { + throw new BadRequest("Chunks on server do not sum up to $expectedSize but to $actualSize bytes"); + } + } +} diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index 1db9b7948e3..5e7a6374206 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -78,6 +78,9 @@ class FileTest extends \Test\TestCase { parent::tearDown(); } + /** + * @return \PHPUnit_Framework_MockObject_MockObject | Storage + */ private function getMockStorage() { $storage = $this->getMockBuilder(Storage::class) ->disableOriginalConstructor() @@ -165,6 +168,7 @@ class FileTest extends \Test\TestCase { ->setConstructorArgs([['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]]) ->getMock(); \OC\Files\Filesystem::mount($storage, [], $this->user . '/'); + /** @var View | \PHPUnit_Framework_MockObject_MockObject $view */ $view = $this->getMockBuilder(View::class) ->setMethods(['getRelativePath', 'resolvePath']) ->getMock(); diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php index 3372f99e957..800bdfd3598 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php @@ -43,8 +43,6 @@ use Sabre\DAV\Tree; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; use Test\TestCase; -use OCA\DAV\Upload\FutureFile; -use OCA\DAV\Connector\Sabre\Directory; use OCP\Files\FileInfo; /** @@ -600,59 +598,4 @@ class FilesPluginTest extends TestCase { $this->assertEquals("false", $propFind->get(self::HAS_PREVIEW_PROPERTYNAME)); } - - public function testBeforeMoveFutureFileSkip() { - $node = $this->createMock(Directory::class); - - $this->tree->expects($this->any()) - ->method('getNodeForPath') - ->with('source') - ->will($this->returnValue($node)); - $this->server->httpResponse->expects($this->never()) - ->method('setStatus'); - - $this->assertNull($this->plugin->beforeMoveFutureFile('source', 'target')); - } - - public function testBeforeMoveFutureFileSkipNonExisting() { - $sourceNode = $this->createMock(FutureFile::class); - - $this->tree->expects($this->any()) - ->method('getNodeForPath') - ->with('source') - ->will($this->returnValue($sourceNode)); - $this->tree->expects($this->any()) - ->method('nodeExists') - ->with('target') - ->will($this->returnValue(false)); - $this->server->httpResponse->expects($this->never()) - ->method('setStatus'); - - $this->assertNull($this->plugin->beforeMoveFutureFile('source', 'target')); - } - - public function testBeforeMoveFutureFileMoveIt() { - $sourceNode = $this->createMock(FutureFile::class); - - $this->tree->expects($this->any()) - ->method('getNodeForPath') - ->with('source') - ->will($this->returnValue($sourceNode)); - $this->tree->expects($this->any()) - ->method('nodeExists') - ->with('target') - ->will($this->returnValue(true)); - $this->tree->expects($this->once()) - ->method('move') - ->with('source', 'target'); - - $this->server->httpResponse->expects($this->once()) - ->method('setHeader') - ->with('Content-Length', '0'); - $this->server->httpResponse->expects($this->once()) - ->method('setStatus') - ->with(204); - - $this->assertFalse($this->plugin->beforeMoveFutureFile('source', 'target')); - } } diff --git a/apps/dav/tests/unit/Upload/ChunkingPluginTest.php b/apps/dav/tests/unit/Upload/ChunkingPluginTest.php new file mode 100644 index 00000000000..3951d1f1795 --- /dev/null +++ b/apps/dav/tests/unit/Upload/ChunkingPluginTest.php @@ -0,0 +1,167 @@ +<?php +/** + * @author Thomas Müller <thomas.mueller@tmit.eu> + * + * @copyright Copyright (c) 2017, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see <http://www.gnu.org/licenses/> + * + */ + + +namespace OCA\DAV\Tests\unit\Upload; + + +use OCA\DAV\Upload\ChunkingPlugin; +use Sabre\HTTP\RequestInterface; +use Sabre\HTTP\ResponseInterface; +use Test\TestCase; +use OCA\DAV\Upload\FutureFile; +use OCA\DAV\Connector\Sabre\Directory; + +class ChunkingPluginTest extends TestCase { + + + /** + * @var \Sabre\DAV\Server | \PHPUnit_Framework_MockObject_MockObject + */ + private $server; + + /** + * @var \Sabre\DAV\Tree | \PHPUnit_Framework_MockObject_MockObject + */ + private $tree; + + /** + * @var ChunkingPlugin + */ + private $plugin; + /** @var RequestInterface | \PHPUnit_Framework_MockObject_MockObject */ + private $request; + /** @var ResponseInterface | \PHPUnit_Framework_MockObject_MockObject */ + private $response; + + public function setUp() { + parent::setUp(); + + $this->server = $this->getMockBuilder('\Sabre\DAV\Server') + ->disableOriginalConstructor() + ->getMock(); + $this->tree = $this->getMockBuilder('\Sabre\DAV\Tree') + ->disableOriginalConstructor() + ->getMock(); + + $this->server->tree = $this->tree; + $this->plugin = new ChunkingPlugin(); + + $this->request = $this->createMock(RequestInterface::class); + $this->response = $this->createMock(ResponseInterface::class); + $this->server->httpRequest = $this->request; + $this->server->httpResponse = $this->response; + + $this->plugin->initialize($this->server); + } + + public function testBeforeMoveFutureFileSkip() { + $node = $this->createMock(Directory::class); + + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with('source') + ->will($this->returnValue($node)); + $this->response->expects($this->never()) + ->method('setStatus'); + + $this->assertNull($this->plugin->beforeMove('source', 'target')); + } + + public function testBeforeMoveFutureFileSkipNonExisting() { + $sourceNode = $this->createMock(FutureFile::class); + $sourceNode->expects($this->once()) + ->method('getSize') + ->willReturn(4); + + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with('source') + ->will($this->returnValue($sourceNode)); + $this->tree->expects($this->any()) + ->method('nodeExists') + ->with('target') + ->will($this->returnValue(false)); + $this->response->expects($this->never()) + ->method('setStatus'); + $this->request->expects($this->once()) + ->method('getHeader') + ->with('OC-Total-Length') + ->willReturn(4); + + $this->assertNull($this->plugin->beforeMove('source', 'target')); + } + + public function testBeforeMoveFutureFileMoveIt() { + $sourceNode = $this->createMock(FutureFile::class); + $sourceNode->expects($this->once()) + ->method('getSize') + ->willReturn(4); + + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with('source') + ->will($this->returnValue($sourceNode)); + $this->tree->expects($this->any()) + ->method('nodeExists') + ->with('target') + ->will($this->returnValue(true)); + $this->tree->expects($this->once()) + ->method('move') + ->with('source', 'target'); + + $this->response->expects($this->once()) + ->method('setHeader') + ->with('Content-Length', '0'); + $this->response->expects($this->once()) + ->method('setStatus') + ->with(204); + $this->request->expects($this->once()) + ->method('getHeader') + ->with('OC-Total-Length') + ->willReturn('4'); + + $this->assertFalse($this->plugin->beforeMove('source', 'target')); + } + + /** + * @expectedException \Sabre\DAV\Exception\BadRequest + * @expectedExceptionMessage Chunks on server do not sum up to 4 but to 3 bytes + */ + public function testBeforeMoveSizeIsWrong() { + $sourceNode = $this->createMock(FutureFile::class); + $sourceNode->expects($this->once()) + ->method('getSize') + ->willReturn(3); + + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with('source') + ->will($this->returnValue($sourceNode)); + $this->request->expects($this->once()) + ->method('getHeader') + ->with('OC-Total-Length') + ->willReturn('4'); + + $this->assertFalse($this->plugin->beforeMove('source', 'target')); + } + +} |