diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index 38a84e1484b..101c0d13935 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -86,21 +86,8 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol */ public function createFile($name, $data = null) { try { - // for chunked upload also updating a existing file is a "createFile" - // because we create all the chunks before re-assemble 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 - $chunkInfo = \OC_FileChunking::decodeName($name); - if (!$this->fileView->isCreatable($this->path) && - !$this->fileView->isUpdatable($this->path . '/' . $chunkInfo['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 (!$this->fileView->isCreatable($this->path)) { - throw new \Sabre\DAV\Exception\Forbidden(); - } + if (!$this->fileView->isCreatable($this->path)) { + throw new \Sabre\DAV\Exception\Forbidden(); } $this->fileView->verifyPath($this->path, $name); diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index ef473340478..0b9199492fe 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -41,7 +41,6 @@ use Sabre\DAV\Exception; use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\Exception\NotFound; -use Sabre\DAV\Exception\NotImplemented; use Sabre\DAV\Exception\ServiceUnavailable; use Sabre\DAV\IFile; @@ -118,16 +117,6 @@ class File extends Node implements IFile { // verify path of the target $this->verifyPath(); - // chunked handling - $chunkedHeader = $this->request->getHeader('oc-chunked'); - if ($chunkedHeader) { - try { - return $this->createFileChunked($data); - } catch (\Exception $e) { - $this->convertToSabreException($e); - } - } - /** @var Storage $partStorage */ [$partStorage] = $this->fileView->resolvePath($this->path); $needsPartFile = $partStorage->needsPartFile() && (strlen($this->path) > 1); @@ -555,135 +544,6 @@ class File extends Node implements IFile { return $storage->getDirectDownload($internalPath); } - /** - * @param resource $data - * @return null|string - * @throws Exception - * @throws BadRequest - * @throws NotImplemented - * @throws ServiceUnavailable - */ - private function createFileChunked($data) { - [$path, $name] = \Sabre\Uri\split($this->path); - - $info = \OC_FileChunking::decodeName($name); - if (empty($info)) { - throw new NotImplemented($this->l10n->t('Invalid chunk name')); - } - - $chunk_handler = new \OC_FileChunking($info); - $bytesWritten = $chunk_handler->store($info['index'], $data); - - //detect aborted upload - if ($this->request->getMethod() === 'PUT') { - $lengthHeader = $this->request->getHeader('content-length'); - if ($lengthHeader) { - $expected = (int)$lengthHeader; - if ($bytesWritten !== $expected) { - $chunk_handler->remove($info['index']); - throw new BadRequest( - $this->l10n->t( - 'Expected filesize of %1$s but read (from Nextcloud client) and wrote (to Nextcloud storage) %2$s. Could either be a network problem on the sending side or a problem writing to the storage on the server side.', - [ - $this->l10n->n('%n byte', '%n bytes', $expected), - $this->l10n->n('%n byte', '%n bytes', $bytesWritten), - ], - ) - ); - } - } - } - - if ($chunk_handler->isComplete()) { - /** @var Storage $storage */ - [$storage,] = $this->fileView->resolvePath($path); - $needsPartFile = $storage->needsPartFile(); - $partFile = null; - - $targetPath = $path . '/' . $info['name']; - /** @var \OC\Files\Storage\Storage $targetStorage */ - [$targetStorage, $targetInternalPath] = $this->fileView->resolvePath($targetPath); - - $exists = $this->fileView->file_exists($targetPath); - - try { - $this->fileView->lockFile($targetPath, ILockingProvider::LOCK_SHARED); - - $this->emitPreHooks($exists, $targetPath); - $this->fileView->changeLock($targetPath, ILockingProvider::LOCK_EXCLUSIVE); - /** @var \OC\Files\Storage\Storage $targetStorage */ - [$targetStorage, $targetInternalPath] = $this->fileView->resolvePath($targetPath); - - if ($needsPartFile) { - // we first assembly the target file as a part file - $partFile = $this->getPartFileBasePath($path . '/' . $info['name']) . '.ocTransferId' . $info['transferid'] . '.part'; - /** @var \OC\Files\Storage\Storage $targetStorage */ - [$partStorage, $partInternalPath] = $this->fileView->resolvePath($partFile); - - - $chunk_handler->file_assemble($partStorage, $partInternalPath); - - // here is the final atomic rename - $renameOkay = $targetStorage->moveFromStorage($partStorage, $partInternalPath, $targetInternalPath); - $fileExists = $targetStorage->file_exists($targetInternalPath); - if ($renameOkay === false || $fileExists === false) { - \OC::$server->get(LoggerInterface::class)->error('\OC\Files\Filesystem::rename() failed', ['app' => 'webdav']); - // only delete if an error occurred and the target file was already created - if ($fileExists) { - // set to null to avoid double-deletion when handling exception - // stray part file - $partFile = null; - $targetStorage->unlink($targetInternalPath); - } - $this->fileView->changeLock($targetPath, ILockingProvider::LOCK_SHARED); - throw new Exception($this->l10n->t('Could not rename part file assembled from chunks')); - } - } else { - // assemble directly into the final file - $chunk_handler->file_assemble($targetStorage, $targetInternalPath); - } - - // allow sync clients to send the mtime along in a header - $mtimeHeader = $this->request->getHeader('x-oc-mtime'); - if ($mtimeHeader !== '') { - $mtime = $this->sanitizeMtime($mtimeHeader); - if ($targetStorage->touch($targetInternalPath, $mtime)) { - $this->header('X-OC-MTime: accepted'); - } - } - - // since we skipped the view we need to scan and emit the hooks ourselves - $targetStorage->getUpdater()->update($targetInternalPath); - - $this->fileView->changeLock($targetPath, ILockingProvider::LOCK_SHARED); - - $this->emitPostHooks($exists, $targetPath); - - // FIXME: should call refreshInfo but can't because $this->path is not the of the final file - $info = $this->fileView->getFileInfo($targetPath); - - $checksumHeader = $this->request->getHeader('oc-checksum'); - if ($checksumHeader) { - $checksum = trim($checksumHeader); - $this->fileView->putFileInfo($targetPath, ['checksum' => $checksum]); - } elseif ($info->getChecksum() !== null && $info->getChecksum() !== '') { - $this->fileView->putFileInfo($this->path, ['checksum' => '']); - } - - $this->fileView->unlockFile($targetPath, ILockingProvider::LOCK_SHARED); - - return $info->getEtag(); - } catch (\Exception $e) { - if ($partFile !== null) { - $targetStorage->unlink($targetInternalPath); - } - $this->convertToSabreException($e); - } - } - - return null; - } - /** * Convert the given exception to a SabreException instance * diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index d1ed8d83464..3b96f67a82b 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -642,15 +642,6 @@ class FilesPlugin extends ServerPlugin { * @throws \Sabre\DAV\Exception\BadRequest */ public function sendFileIdHeader($filePath, ?\Sabre\DAV\INode $node = null) { - // chunked upload handling - if (isset($_SERVER['HTTP_OC_CHUNKED'])) { - [$path, $name] = \Sabre\Uri\split($filePath); - $info = \OC_FileChunking::decodeName($name); - if (!empty($info)) { - $filePath = $path . '/' . $info['name']; - } - } - // we get the node for the given $filePath here because in case of afterCreateFile $node is the parent folder if (!$this->server->tree->nodeExists($filePath)) { return; diff --git a/apps/dav/lib/Connector/Sabre/LockPlugin.php b/apps/dav/lib/Connector/Sabre/LockPlugin.php index 7bfbb352173..6640771dc31 100644 --- a/apps/dav/lib/Connector/Sabre/LockPlugin.php +++ b/apps/dav/lib/Connector/Sabre/LockPlugin.php @@ -42,7 +42,7 @@ class LockPlugin extends ServerPlugin { public function getLock(RequestInterface $request) { // we can't listen on 'beforeMethod:PUT' due to order of operations with setting up the tree // so instead we limit ourselves to the PUT method manually - if ($request->getMethod() !== 'PUT' || isset($_SERVER['HTTP_OC_CHUNKED'])) { + if ($request->getMethod() !== 'PUT') { return; } try { @@ -65,7 +65,7 @@ class LockPlugin extends ServerPlugin { if ($this->isLocked === false) { return; } - if ($request->getMethod() !== 'PUT' || isset($_SERVER['HTTP_OC_CHUNKED'])) { + if ($request->getMethod() !== 'PUT') { return; } try { diff --git a/apps/dav/lib/Connector/Sabre/ObjectTree.php b/apps/dav/lib/Connector/Sabre/ObjectTree.php index a7b6cadce35..7ac5b476154 100644 --- a/apps/dav/lib/Connector/Sabre/ObjectTree.php +++ b/apps/dav/lib/Connector/Sabre/ObjectTree.php @@ -46,35 +46,6 @@ class ObjectTree extends CachingTree { $this->mountManager = $mountManager; } - /** - * If the given path is a chunked file name, converts it - * to the real file name. Only applies if the OC-CHUNKED header - * is present. - * - * @param string $path chunk file path to convert - * - * @return string path to real file - */ - private function resolveChunkFile($path) { - if (isset($_SERVER['HTTP_OC_CHUNKED'])) { - // resolve to real file name to find the proper node - [$dir, $name] = \Sabre\Uri\split($path); - if ($dir === '/' || $dir === '.') { - $dir = ''; - } - - $info = \OC_FileChunking::decodeName($name); - // only replace path if it was really the chunked file - if (isset($info['transferid'])) { - // getNodePath is called for multiple nodes within a chunk - // upload call - $path = $dir . '/' . $info['name']; - $path = ltrim($path, '/'); - } - } - return $path; - } - /** * Returns the INode object for the requested path * @@ -126,9 +97,6 @@ class ObjectTree extends CachingTree { $info = null; } } else { - // resolve chunk file name to real name, if applicable - $path = $this->resolveChunkFile($path); - // read from cache try { $info = $this->fileView->getFileInfo($path); diff --git a/apps/dav/lib/Connector/Sabre/QuotaPlugin.php b/apps/dav/lib/Connector/Sabre/QuotaPlugin.php index ea6fc12e1cb..deaa4cf672b 100644 --- a/apps/dav/lib/Connector/Sabre/QuotaPlugin.php +++ b/apps/dav/lib/Connector/Sabre/QuotaPlugin.php @@ -187,26 +187,11 @@ class QuotaPlugin extends \Sabre\DAV\ServerPlugin { } $req = $this->server->httpRequest; - // If LEGACY chunked upload - if ($req->getHeader('OC-Chunked')) { - $info = \OC_FileChunking::decodeName($newName); - $chunkHandler = $this->getFileChunking($info); - // subtract the already uploaded size to see whether - // there is still enough space for the remaining chunks - $length -= $chunkHandler->getCurrentSize(); - // use target file name for free space check in case of shared files - $path = rtrim($parentPath, '/') . '/' . $info['name']; - } - // Strip any duplicate slashes $path = str_replace('//', '/', $path); $freeSpace = $this->getFreeSpace($path); if ($freeSpace >= 0 && $length > $freeSpace) { - // If LEGACY chunked upload, clean up - if (isset($chunkHandler)) { - $chunkHandler->cleanup(); - } throw new InsufficientStorage("Insufficient space in $path, $length required, $freeSpace available"); } } @@ -214,11 +199,6 @@ class QuotaPlugin extends \Sabre\DAV\ServerPlugin { return true; } - public function getFileChunking($info) { - // FIXME: need a factory for better mocking support - return new \OC_FileChunking($info); - } - public function getLength() { $req = $this->server->httpRequest; $length = $req->getHeader('X-Expected-Entity-Length'); diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index 2830ccc0f18..060f0294256 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -210,86 +210,6 @@ class FileTest extends TestCase { $this->assertEmpty($this->listPartFiles($view, ''), 'No stray part files'); } - /** - * Test putting a file using chunking - * - * @dataProvider fopenFailuresProvider - */ - public function testChunkedPutFails($thrownException, $expectedException, $checkPreviousClass = false): void { - // setup - $storage = $this->getMockBuilder(Local::class) - ->onlyMethods(['fopen']) - ->setConstructorArgs([['datadir' => \OCP\Server::get(ITempManager::class)->getTemporaryFolder()]]) - ->getMock(); - \OC\Files\Filesystem::mount($storage, [], $this->user . '/'); - /** @var View|MockObject */ - $view = $this->getMockBuilder(View::class) - ->onlyMethods(['getRelativePath', 'resolvePath']) - ->getMock(); - $view->expects($this->atLeastOnce()) - ->method('resolvePath') - ->willReturnCallback( - function ($path) use ($storage) { - return [$storage, $path]; - } - ); - - if ($thrownException !== null) { - $storage->expects($this->once()) - ->method('fopen') - ->will($this->throwException($thrownException)); - } else { - $storage->expects($this->once()) - ->method('fopen') - ->willReturn(false); - } - - $view->expects($this->any()) - ->method('getRelativePath') - ->willReturnArgument(0); - - $request = new Request([ - 'server' => [ - 'HTTP_OC_CHUNKED' => 'true' - ] - ], $this->requestId, $this->config, null); - - $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL, - 'type' => FileInfo::TYPE_FOLDER, - ], null); - $file = new \OCA\DAV\Connector\Sabre\File($view, $info, null, $request); - - // put first chunk - $file->acquireLock(ILockingProvider::LOCK_SHARED); - $this->assertNull($file->put('test data one')); - $file->releaseLock(ILockingProvider::LOCK_SHARED); - - $info = new \OC\Files\FileInfo('/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL, - 'type' => FileInfo::TYPE_FOLDER, - ], null); - $file = new \OCA\DAV\Connector\Sabre\File($view, $info, null, $request); - - // action - $caughtException = null; - try { - // last chunk - $file->acquireLock(ILockingProvider::LOCK_SHARED); - $file->put('test data two'); - $file->releaseLock(ILockingProvider::LOCK_SHARED); - } catch (\Exception $e) { - $caughtException = $e; - } - - $this->assertInstanceOf($expectedException, $caughtException); - if ($checkPreviousClass) { - $this->assertInstanceOf(get_class($thrownException), $caughtException->getPrevious()); - } - - $this->assertEmpty($this->listPartFiles($view, ''), 'No stray part files'); - } - /** * Simulate putting a file to the given path. * @@ -418,45 +338,6 @@ class FileTest extends TestCase { } } - /** - * Test putting a file with string Mtime using chunking - * @dataProvider legalMtimeProvider - */ - public function testChunkedPutLegalMtime($requestMtime, $resultMtime): void { - $request = new Request([ - 'server' => [ - 'HTTP_X_OC_MTIME' => (string)$requestMtime, - 'HTTP_OC_CHUNKED' => 'true' - ] - ], $this->requestId, $this->config, null); - - $file = 'foo.txt'; - - if ($resultMtime === null) { - $this->expectException(\Sabre\DAV\Exception::class); - } - - $this->doPut($file.'-chunking-12345-2-0', null, $request); - $this->doPut($file.'-chunking-12345-2-1', null, $request); - - if ($resultMtime !== null) { - $this->assertEquals($resultMtime, $this->getFileInfos($file)['mtime']); - } - } - - /** - * Test putting a file using chunking - */ - public function testChunkedPut(): void { - $request = new Request([ - 'server' => [ - 'HTTP_OC_CHUNKED' => 'true' - ] - ], $this->requestId, $this->config, null); - $this->assertNull($this->doPut('/test.txt-chunking-12345-2-0', null, $request)); - $this->assertNotEmpty($this->doPut('/test.txt-chunking-12345-2-1', null, $request)); - } - /** * Test that putting a file triggers create hooks */ @@ -559,83 +440,6 @@ class FileTest extends TestCase { ); } - /** - * Test that putting a file with chunks triggers create hooks - */ - public function testPutChunkedFileTriggersHooks(): void { - HookHelper::setUpHooks(); - - $request = new Request([ - 'server' => [ - 'HTTP_OC_CHUNKED' => 'true' - ] - ], $this->requestId, $this->config, null); - $this->assertNull($this->doPut('/foo.txt-chunking-12345-2-0', null, $request)); - $this->assertNotEmpty($this->doPut('/foo.txt-chunking-12345-2-1', null, $request)); - - $this->assertCount(4, HookHelper::$hookCalls); - $this->assertHookCall( - HookHelper::$hookCalls[0], - Filesystem::signal_create, - '/foo.txt' - ); - $this->assertHookCall( - HookHelper::$hookCalls[1], - Filesystem::signal_write, - '/foo.txt' - ); - $this->assertHookCall( - HookHelper::$hookCalls[2], - Filesystem::signal_post_create, - '/foo.txt' - ); - $this->assertHookCall( - HookHelper::$hookCalls[3], - Filesystem::signal_post_write, - '/foo.txt' - ); - } - - /** - * Test that putting a chunked file triggers update hooks - */ - public function testPutOverwriteChunkedFileTriggersHooks(): void { - $view = \OC\Files\Filesystem::getView(); - $view->file_put_contents('/foo.txt', 'some content that will be replaced'); - - HookHelper::setUpHooks(); - - $request = new Request([ - 'server' => [ - 'HTTP_OC_CHUNKED' => 'true' - ] - ], $this->requestId, $this->config, null); - $this->assertNull($this->doPut('/foo.txt-chunking-12345-2-0', null, $request)); - $this->assertNotEmpty($this->doPut('/foo.txt-chunking-12345-2-1', null, $request)); - - $this->assertCount(4, HookHelper::$hookCalls); - $this->assertHookCall( - HookHelper::$hookCalls[0], - Filesystem::signal_update, - '/foo.txt' - ); - $this->assertHookCall( - HookHelper::$hookCalls[1], - Filesystem::signal_write, - '/foo.txt' - ); - $this->assertHookCall( - HookHelper::$hookCalls[2], - Filesystem::signal_post_update, - '/foo.txt' - ); - $this->assertHookCall( - HookHelper::$hookCalls[3], - Filesystem::signal_post_write, - '/foo.txt' - ); - } - public static function cancellingHook($params): void { self::$hookCalls[] = [ 'signal' => Filesystem::signal_post_create, @@ -753,50 +557,6 @@ class FileTest extends TestCase { $this->assertEmpty($this->listPartFiles($view, ''), 'No stray part files'); } - /** - * Test exception during final rename in chunk upload mode - */ - public function testChunkedPutFailsFinalRename(): void { - $view = new \OC\Files\View('/' . $this->user . '/files'); - - // simulate situation where the target file is locked - $view->lockFile('/test.txt', ILockingProvider::LOCK_EXCLUSIVE); - - $request = new Request([ - 'server' => [ - 'HTTP_OC_CHUNKED' => 'true' - ] - ], $this->requestId, $this->config, null); - - $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-0', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL, - 'type' => FileInfo::TYPE_FOLDER, - ], null); - $file = new \OCA\DAV\Connector\Sabre\File($view, $info, null, $request); - $file->acquireLock(ILockingProvider::LOCK_SHARED); - $this->assertNull($file->put('test data one')); - $file->releaseLock(ILockingProvider::LOCK_SHARED); - - $info = new \OC\Files\FileInfo('/' . $this->user . '/files/test.txt-chunking-12345-2-1', $this->getMockStorage(), null, [ - 'permissions' => \OCP\Constants::PERMISSION_ALL, - 'type' => FileInfo::TYPE_FOLDER, - ], null); - $file = new \OCA\DAV\Connector\Sabre\File($view, $info, null, $request); - - // action - $thrown = false; - try { - $file->acquireLock(ILockingProvider::LOCK_SHARED); - $file->put($this->getStream('test data')); - $file->releaseLock(ILockingProvider::LOCK_SHARED); - } catch (\OCA\DAV\Connector\Sabre\Exception\FileLocked $e) { - $thrown = true; - } - - $this->assertTrue($thrown); - $this->assertEmpty($this->listPartFiles($view, ''), 'No stray part files'); - } - /** * Test put file with invalid chars */ diff --git a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php index ccdb501f7d3..f0996a17a33 100644 --- a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php @@ -127,13 +127,8 @@ class ObjectTreeTest extends \Test\TestCase { $inputFileName, $fileInfoQueryPath, $outputFileName, - $type, - $enableChunkingHeader + $type ): void { - if ($enableChunkingHeader) { - $_SERVER['HTTP_OC_CHUNKED'] = true; - } - $rootNode = $this->getMockBuilder(Directory::class) ->disableOriginalConstructor() ->getMock(); @@ -170,8 +165,6 @@ class ObjectTreeTest extends \Test\TestCase { } else { $this->assertTrue($node instanceof \OCA\DAV\Connector\Sabre\Directory); } - - unset($_SERVER['HTTP_OC_CHUNKED']); } public function nodeForPathProvider() { @@ -182,7 +175,6 @@ class ObjectTreeTest extends \Test\TestCase { 'regularfile.txt', 'regularfile.txt', 'file', - false ], // regular directory [ @@ -190,31 +182,6 @@ class ObjectTreeTest extends \Test\TestCase { 'regulardir', 'regulardir', 'dir', - false - ], - // regular file with chunking - [ - 'regularfile.txt', - 'regularfile.txt', - 'regularfile.txt', - 'file', - true - ], - // regular directory with chunking - [ - 'regulardir', - 'regulardir', - 'regulardir', - 'dir', - true - ], - // file with chunky file name - [ - 'regularfile.txt-chunking-123566789-10-1', - 'regularfile.txt', - 'regularfile.txt', - 'file', - true ], // regular file in subdir [ @@ -222,7 +189,6 @@ class ObjectTreeTest extends \Test\TestCase { 'subdir/regularfile.txt', 'regularfile.txt', 'file', - false ], // regular directory in subdir [ @@ -230,15 +196,6 @@ class ObjectTreeTest extends \Test\TestCase { 'subdir/regulardir', 'regulardir', 'dir', - false - ], - // file with chunky file name in subdir - [ - 'subdir/regularfile.txt-chunking-123566789-10-1', - 'subdir/regularfile.txt', - 'regularfile.txt', - 'file', - true ], ]; } diff --git a/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php index ed4a8b0404a..815837799fd 100644 --- a/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php @@ -143,29 +143,6 @@ class QuotaPluginTest extends TestCase { ]; } - /** - * @dataProvider quotaChunkedOkProvider - */ - public function testCheckQuotaChunkedOk($quota, $chunkTotalSize, $headers): void { - $this->init($quota, 'sub/test.txt'); - - $mockChunking = $this->getMockBuilder(\OC_FileChunking::class) - ->disableOriginalConstructor() - ->getMock(); - $mockChunking->expects($this->once()) - ->method('getCurrentSize') - ->willReturn($chunkTotalSize); - - $this->plugin->expects($this->once()) - ->method('getFileChunking') - ->willReturn($mockChunking); - - $headers['OC-CHUNKED'] = 1; - $this->server->httpRequest = new \Sabre\HTTP\Request('POST', 'dummy.file', $headers); - $result = $this->plugin->checkQuota('/sub/test.txt-chunking-12345-3-1'); - $this->assertTrue($result); - } - public function quotaChunkedFailProvider() { return [ [400, 0, ['X-EXPECTED-ENTITY-LENGTH' => '1024']], @@ -178,30 +155,6 @@ class QuotaPluginTest extends TestCase { ]; } - /** - * @dataProvider quotaChunkedFailProvider - */ - public function testCheckQuotaChunkedFail($quota, $chunkTotalSize, $headers): void { - $this->expectException(\Sabre\DAV\Exception\InsufficientStorage::class); - - $this->init($quota, 'sub/test.txt'); - - $mockChunking = $this->getMockBuilder(\OC_FileChunking::class) - ->disableOriginalConstructor() - ->getMock(); - $mockChunking->expects($this->once()) - ->method('getCurrentSize') - ->willReturn($chunkTotalSize); - - $this->plugin->expects($this->once()) - ->method('getFileChunking') - ->willReturn($mockChunking); - - $headers['OC-CHUNKED'] = 1; - $this->server->httpRequest = new \Sabre\HTTP\Request('POST', 'dummy.file', $headers); - $this->plugin->checkQuota('/sub/test.txt-chunking-12345-3-1'); - } - private function buildFileViewMock($quota, $checkedPath) { // mock filesysten $view = $this->getMockBuilder(View::class) diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php index 286bba7d196..60decd4c846 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php @@ -74,115 +74,4 @@ class UploadTest extends RequestTestCase { $result = $this->request($view, $user, 'pass', 'PUT', '/foo.txt', 'asd'); $this->assertEquals(Http::STATUS_LOCKED, $result->getStatus()); } - - public function testChunkedUpload(): void { - $user = $this->getUniqueID(); - $view = $this->setupUser($user, 'pass'); - - $this->assertFalse($view->file_exists('foo.txt')); - $response = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-0', 'asd', ['OC-Chunked' => '1']); - - $this->assertEquals(201, $response->getStatus()); - $this->assertFalse($view->file_exists('foo.txt')); - - $response = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-1', 'bar', ['OC-Chunked' => '1']); - - $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); - $this->assertTrue($view->file_exists('foo.txt')); - - $this->assertEquals('asdbar', $view->file_get_contents('foo.txt')); - - $info = $view->getFileInfo('foo.txt'); - $this->assertInstanceOf('\OC\Files\FileInfo', $info); - $this->assertEquals(6, $info->getSize()); - } - - public function testChunkedUploadOverWrite(): void { - $user = $this->getUniqueID(); - $view = $this->setupUser($user, 'pass'); - - $view->file_put_contents('foo.txt', 'bar'); - $response = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-0', 'asd', ['OC-Chunked' => '1']); - - $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); - $this->assertEquals('bar', $view->file_get_contents('foo.txt')); - - $response = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-1', 'bar', ['OC-Chunked' => '1']); - - $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); - - $this->assertEquals('asdbar', $view->file_get_contents('foo.txt')); - - $info = $view->getFileInfo('foo.txt'); - $this->assertInstanceOf('\OC\Files\FileInfo', $info); - $this->assertEquals(6, $info->getSize()); - } - - public function testChunkedUploadOutOfOrder(): void { - $user = $this->getUniqueID(); - $view = $this->setupUser($user, 'pass'); - - $this->assertFalse($view->file_exists('foo.txt')); - $response = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-1', 'bar', ['OC-Chunked' => '1']); - - $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); - $this->assertFalse($view->file_exists('foo.txt')); - - $response = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-0', 'asd', ['OC-Chunked' => '1']); - - $this->assertEquals(201, $response->getStatus()); - $this->assertTrue($view->file_exists('foo.txt')); - - $this->assertEquals('asdbar', $view->file_get_contents('foo.txt')); - - $info = $view->getFileInfo('foo.txt'); - $this->assertInstanceOf('\OC\Files\FileInfo', $info); - $this->assertEquals(6, $info->getSize()); - } - - public function testChunkedUploadOutOfOrderReadLocked(): void { - $user = $this->getUniqueID(); - $view = $this->setupUser($user, 'pass'); - - $this->assertFalse($view->file_exists('foo.txt')); - - $view->lockFile('/foo.txt', ILockingProvider::LOCK_SHARED); - - try { - $response = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-1', 'bar', ['OC-Chunked' => '1']); - } catch (\OCA\DAV\Connector\Sabre\Exception\FileLocked $e) { - $this->fail('Didn\'t expect locked error for the first chunk on read lock'); - return; - } - - $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); - $this->assertFalse($view->file_exists('foo.txt')); - - // last chunk should trigger the locked error since it tries to assemble - $result = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-0', 'asd', ['OC-Chunked' => '1']); - $this->assertEquals(Http::STATUS_LOCKED, $result->getStatus()); - } - - public function testChunkedUploadOutOfOrderWriteLocked(): void { - $user = $this->getUniqueID(); - $view = $this->setupUser($user, 'pass'); - - $this->assertFalse($view->file_exists('foo.txt')); - - $view->lockFile('/foo.txt', ILockingProvider::LOCK_EXCLUSIVE); - - try { - $response = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-1', 'bar', ['OC-Chunked' => '1']); - } catch (\OCA\DAV\Connector\Sabre\Exception\FileLocked $e) { - $this->fail('Didn\'t expect locked error for the first chunk on write lock'); // maybe forbid this in the future for write locks only? - return; - } - - $this->assertEquals(Http::STATUS_CREATED, $response->getStatus()); - $this->assertFalse($view->file_exists('foo.txt')); - - // last chunk should trigger the locked error since it tries to assemble - $result = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-0', 'asd', ['OC-Chunked' => '1']); - $this->assertEquals(Http::STATUS_LOCKED, $result->getStatus()); - } } diff --git a/build/integration/dav_features/webdav-related.feature b/build/integration/dav_features/webdav-related.feature index 2aaa917c075..fdf633bd580 100644 --- a/build/integration/dav_features/webdav-related.feature +++ b/build/integration/dav_features/webdav-related.feature @@ -279,33 +279,6 @@ Feature: webdav-related When Sending a "PROPFIND" to "/remote.php/webdav/welcome.txt" with requesttoken Then the HTTP status code should be "207" - Scenario: Upload chunked file asc - Given user "user0" exists - And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChunkedFile.txt" - And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChunkedFile.txt" - And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChunkedFile.txt" - When As an "user0" - And Downloading file "/myChunkedFile.txt" - Then Downloaded content should be "AAAAABBBBBCCCCC" - - Scenario: Upload chunked file desc - Given user "user0" exists - And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChunkedFile.txt" - And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChunkedFile.txt" - And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChunkedFile.txt" - When As an "user0" - And Downloading file "/myChunkedFile.txt" - Then Downloaded content should be "AAAAABBBBBCCCCC" - - Scenario: Upload chunked file random - Given user "user0" exists - And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChunkedFile.txt" - And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChunkedFile.txt" - And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChunkedFile.txt" - When As an "user0" - And Downloading file "/myChunkedFile.txt" - Then Downloaded content should be "AAAAABBBBBCCCCC" - Scenario: A file that is not shared does not have a share-types property Given user "user0" exists And user "user0" created a folder "/test" diff --git a/build/integration/features/bootstrap/ChecksumsContext.php b/build/integration/features/bootstrap/ChecksumsContext.php index 5661015e714..7909d077d7c 100644 --- a/build/integration/features/bootstrap/ChecksumsContext.php +++ b/build/integration/features/bootstrap/ChecksumsContext.php @@ -211,31 +211,4 @@ class ChecksumsContext implements \Behat\Behat\Context\Context { throw new \Exception("Expected no checksum header but got ".$this->response->getHeader('OC-Checksum')[0]); } } - - /** - * @Given user :user uploads chunk file :num of :total with :data to :destination with checksum :checksum - * @param string $user - * @param int $num - * @param int $total - * @param string $data - * @param string $destination - * @param string $checksum - */ - public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination, $checksum) { - $num -= 1; - $this->response = $this->client->put( - $this->baseUrl . '/remote.php/webdav' . $destination . '-chunking-42-'.$total.'-'.$num, - [ - 'auth' => [ - $user, - $this->getPasswordForUser($user) - ], - 'body' => $data, - 'headers' => [ - 'OC-Checksum' => $checksum, - 'OC-Chunked' => '1', - ] - ] - ); - } } diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index c2de207284d..852317f76a3 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -683,21 +683,6 @@ trait WebDav { } } - /** - * @Given user :user uploads chunk file :num of :total with :data to :destination - * @param string $user - * @param int $num - * @param int $total - * @param string $data - * @param string $destination - */ - public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination) { - $num -= 1; - $data = \GuzzleHttp\Psr7\Utils::streamFor($data); - $file = $destination . '-chunking-42-' . $total . '-' . $num; - $this->makeDavRequest($user, 'PUT', $file, ['OC-Chunked' => '1'], $data, "uploads"); - } - /** * @Given user :user uploads bulked files :name1 with :content1 and :name2 with :content2 and :name3 with :content3 * @param string $user diff --git a/build/integration/federation_features/federated.feature b/build/integration/federation_features/federated.feature index 37c43c1191e..81a3d3abd02 100644 --- a/build/integration/federation_features/federated.feature +++ b/build/integration/federation_features/federated.feature @@ -247,40 +247,6 @@ Feature: federated #And Downloading file "/PARENT (2)/textfile0.txt" with range "bytes=0-8" #Then Downloaded content should be "BLABLABLA" - Scenario: Overwrite a federated shared file as recipient using old chunking - Given Using server "REMOTE" - And user "user1" exists - And user "user2" exists - And Using server "LOCAL" - And user "user0" exists - And User "user0" from server "LOCAL" shares "/textfile0.txt" with user "user1" from server "REMOTE" - And User "user1" from server "REMOTE" accepts last pending share - And Using server "REMOTE" - And As an "user1" - #And user "user1" uploads chunk file "1" of "3" with "AAAAA" to "/textfile0 (2).txt" - #And user "user1" uploads chunk file "2" of "3" with "BBBBB" to "/textfile0 (2).txt" - #And user "user1" uploads chunk file "3" of "3" with "CCCCC" to "/textfile0 (2).txt" - #When Downloading file "/textfile0 (2).txt" with range "bytes=0-4" - #Then Downloaded content should be "AAAAA" - - Scenario: Overwrite a federated shared folder as recipient using old chunking - Given Using server "REMOTE" - And user "user1" exists - And user "user2" exists - And Using server "LOCAL" - And user "user0" exists - And User "user0" from server "LOCAL" shares "/PARENT" with user "user1" from server "REMOTE" - And User "user1" from server "REMOTE" accepts last pending share - And Using server "REMOTE" - And As an "user1" - #And user "user1" uploads chunk file "1" of "3" with "AAAAA" to "/PARENT (2)/textfile0.txt" - #And user "user1" uploads chunk file "2" of "3" with "BBBBB" to "/PARENT (2)/textfile0.txt" - #And user "user1" uploads chunk file "3" of "3" with "CCCCC" to "/PARENT (2)/textfile0.txt" - #When Downloading file "/PARENT (2)/textfile0.txt" with range "bytes=3-13" - #Then Downloaded content should be "AABBBBBCCCC" - - - Scenario: List federated share from another server not accepted yet Given Using server "LOCAL" And user "user0" exists diff --git a/build/integration/files_features/checksums.feature b/build/integration/files_features/checksums.feature index bc70cfc09b7..d797c7a503e 100644 --- a/build/integration/files_features/checksums.feature +++ b/build/integration/files_features/checksums.feature @@ -61,19 +61,3 @@ Feature: checksums When user "user0" uploads file "data/textfile.txt" to "/myChecksumFile.txt" And user "user0" downloads the file "/myChecksumFile.txt" Then The OC-Checksum header should not be there - - Scenario: Uploading a chunked file with checksum should return the checksum in the propfind - Given user "user0" exists - And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - When user "user0" request the checksum of "/myChecksumFile.txt" via propfind - Then The webdav checksum should match "MD5:e892fdd61a74bc89cd05673cc2e22f88" - - Scenario: Uploading a chunked file with checksum should return the checksum in the download header - Given user "user0" exists - And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChecksumFile.txt" with checksum "MD5:e892fdd61a74bc89cd05673cc2e22f88" - When user "user0" downloads the file "/myChecksumFile.txt" - Then The header checksum should match "MD5:e892fdd61a74bc89cd05673cc2e22f88" diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0837fe2ab62..17479170f0d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1909,7 +1909,6 @@ return array( 'OC_API' => $baseDir . '/lib/private/legacy/OC_API.php', 'OC_App' => $baseDir . '/lib/private/legacy/OC_App.php', 'OC_Defaults' => $baseDir . '/lib/private/legacy/OC_Defaults.php', - 'OC_FileChunking' => $baseDir . '/lib/private/legacy/OC_FileChunking.php', 'OC_Files' => $baseDir . '/lib/private/legacy/OC_Files.php', 'OC_Helper' => $baseDir . '/lib/private/legacy/OC_Helper.php', 'OC_Hook' => $baseDir . '/lib/private/legacy/OC_Hook.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index b7f11b43266..a5b707220d2 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1942,7 +1942,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC_API' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_API.php', 'OC_App' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_App.php', 'OC_Defaults' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_Defaults.php', - 'OC_FileChunking' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_FileChunking.php', 'OC_Files' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_Files.php', 'OC_Helper' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_Helper.php', 'OC_Hook' => __DIR__ . '/../../..' . '/lib/private/legacy/OC_Hook.php', diff --git a/lib/private/legacy/OC_FileChunking.php b/lib/private/legacy/OC_FileChunking.php deleted file mode 100644 index 7600d810657..00000000000 --- a/lib/private/legacy/OC_FileChunking.php +++ /dev/null @@ -1,161 +0,0 @@ -.*)-chunking-(?P\d+)-(?P\d+)-(?P\d+)/', $name, $matches); - return $matches; - } - - /** - * @param string[] $info - */ - public function __construct($info) { - $this->info = $info; - $this->ttl = \OC::$server->getConfig()->getSystemValueInt('cache_chunk_gc_ttl', 86400); - } - - public function getPrefix() { - $name = $this->info['name']; - $transferid = $this->info['transferid']; - - return $name.'-chunking-'.$transferid.'-'; - } - - protected function getCache() { - if (!isset($this->cache)) { - $this->cache = new \OC\Cache\File(); - } - return $this->cache; - } - - /** - * Stores the given $data under the given $key - the number of stored bytes is returned - * - * @param string $index - * @param resource $data - * @return int - */ - public function store($index, $data) { - $cache = $this->getCache(); - $name = $this->getPrefix().$index; - $cache->set($name, $data, $this->ttl); - - return $cache->size($name); - } - - public function isComplete() { - $prefix = $this->getPrefix(); - $cache = $this->getCache(); - $chunkcount = (int)$this->info['chunkcount']; - - for ($i = ($chunkcount - 1); $i >= 0; $i--) { - if (!$cache->hasKey($prefix.$i)) { - return false; - } - } - - return true; - } - - /** - * Assembles the chunks into the file specified by the path. - * Chunks are deleted afterwards. - * - * @param resource $f target path - * - * @return integer assembled file size - * - * @throws \OC\InsufficientStorageException when file could not be fully - * assembled due to lack of free space - */ - public function assemble($f) { - $cache = $this->getCache(); - $prefix = $this->getPrefix(); - $count = 0; - for ($i = 0; $i < $this->info['chunkcount']; $i++) { - $chunk = $cache->get($prefix.$i); - // remove after reading to directly save space - $cache->remove($prefix.$i); - $count += fwrite($f, $chunk); - // let php release the memory to work around memory exhausted error with php 5.6 - $chunk = null; - } - - return $count; - } - - /** - * Returns the size of the chunks already present - * @return integer size in bytes - */ - public function getCurrentSize() { - $cache = $this->getCache(); - $prefix = $this->getPrefix(); - $total = 0; - for ($i = 0; $i < $this->info['chunkcount']; $i++) { - $total += $cache->size($prefix.$i); - } - return $total; - } - - /** - * Removes all chunks which belong to this transmission - */ - public function cleanup() { - $cache = $this->getCache(); - $prefix = $this->getPrefix(); - for ($i = 0; $i < $this->info['chunkcount']; $i++) { - $cache->remove($prefix.$i); - } - } - - /** - * Removes one specific chunk - * @param string $index - */ - public function remove($index) { - $cache = $this->getCache(); - $prefix = $this->getPrefix(); - $cache->remove($prefix.$index); - } - - /** - * Assembles the chunks into the file specified by the path. - * Also triggers the relevant hooks and proxies. - * - * @param \OC\Files\Storage\Storage $storage storage - * @param string $path target path relative to the storage - * @return bool true on success or false if file could not be created - * - * @throws \OC\ServerNotAvailableException - */ - public function file_assemble($storage, $path) { - // use file_put_contents as method because that best matches what this function does - if (\OC\Files\Filesystem::isValidPath($path)) { - $target = $storage->fopen($path, 'w'); - if ($target) { - $count = $this->assemble($target); - fclose($target); - return $count > 0; - } else { - return false; - } - } - return false; - } -} diff --git a/tests/lib/FileChunkingTest.php b/tests/lib/FileChunkingTest.php deleted file mode 100644 index b69154ea1c3..00000000000 --- a/tests/lib/FileChunkingTest.php +++ /dev/null @@ -1,58 +0,0 @@ -getMockBuilder(\OC_FileChunking::class) - ->setMethods(['getCache']) - ->setConstructorArgs([[ - 'name' => 'file', - 'transferid' => '42', - 'chunkcount' => $total, - ]]) - ->getMock(); - - $cache = $this->createMock(ICache::class); - - $cache->expects($this->atLeastOnce()) - ->method('hasKey') - ->willReturnCallback(function ($key) use ($present) { - $data = explode('-', $key); - return in_array($data[3], $present); - }); - - $fileChunking->method('getCache')->willReturn($cache); - - $this->assertEquals($expected, $fileChunking->isComplete()); - } -}