diff options
author | Anna Larch <anna@nextcloud.com> | 2023-02-07 15:55:55 +0100 |
---|---|---|
committer | Julius Härtl <jus@bitgrid.net> | 2023-02-17 19:18:37 +0100 |
commit | 5d4efb4d5fd8e4389856df5d94c3b92c7019e603 (patch) | |
tree | be191327ae1ad02a9db395dbf22c03a085b053ed /apps/dav | |
parent | d6a3ebc79f405f5294803ce04386832e526e447b (diff) | |
download | nextcloud-server-5d4efb4d5fd8e4389856df5d94c3b92c7019e603.tar.gz nextcloud-server-5d4efb4d5fd8e4389856df5d94c3b92c7019e603.zip |
Do not set up filesystem on every call
Also remove old Oc_FileChunking logis that produced GC- collectable chunks
Signed-off-by: Anna Larch <anna@nextcloud.com>
Diffstat (limited to 'apps/dav')
-rw-r--r-- | apps/dav/lib/Connector/Sabre/Directory.php | 75 | ||||
-rw-r--r-- | apps/dav/lib/Connector/Sabre/File.php | 135 | ||||
-rw-r--r-- | apps/dav/lib/Connector/Sabre/FilesPlugin.php | 9 | ||||
-rw-r--r-- | apps/dav/lib/Connector/Sabre/LockPlugin.php | 4 | ||||
-rw-r--r-- | apps/dav/lib/Connector/Sabre/ObjectTree.php | 38 | ||||
-rw-r--r-- | apps/dav/lib/Connector/Sabre/QuotaPlugin.php | 17 | ||||
-rw-r--r-- | apps/dav/tests/unit/Connector/Sabre/FileTest.php | 221 | ||||
-rw-r--r-- | apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php | 60 | ||||
-rw-r--r-- | apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php | 91 | ||||
-rw-r--r-- | apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php | 2 | ||||
-rw-r--r-- | apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php | 110 |
11 files changed, 47 insertions, 715 deletions
diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index f4b1ee62190..4459daea869 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -48,6 +48,7 @@ use OCP\Files\StorageNotAvailableException; use OCP\Lock\ILockingProvider; use OCP\Lock\LockedException; use Psr\Log\LoggerInterface; +use Sabre\DAV\Exception; use Sabre\DAV\Exception\BadRequest; use Sabre\DAV\Exception\Locked; use Sabre\DAV\Exception\NotFound; @@ -102,33 +103,19 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol * @param string $name Name of the file * @param resource|string $data Initial payload * @return null|string - * @throws Exception\EntityTooLarge * @throws Exception\UnsupportedMediaType * @throws FileLocked * @throws InvalidPath - * @throws \Sabre\DAV\Exception - * @throws \Sabre\DAV\Exception\BadRequest - * @throws \Sabre\DAV\Exception\Forbidden - * @throws \Sabre\DAV\Exception\ServiceUnavailable + * @throws Exception + * @throws BadRequest + * @throws Exception\Forbidden + * @throws ServiceUnavailable */ 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(); - } + // For non-chunked upload it is enough to check if we can create a new file + if (!$this->fileView->isCreatable($this->path)) { + throw new Exception\Forbidden(); } $this->fileView->verifyPath($this->path, $name); @@ -153,8 +140,8 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol $this->fileView->unlockFile($path . '.upload.part', ILockingProvider::LOCK_EXCLUSIVE); $node->releaseLock(ILockingProvider::LOCK_SHARED); return $result; - } catch (\OCP\Files\StorageNotAvailableException $e) { - throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage(), $e->getCode(), $e); + } catch (StorageNotAvailableException $e) { + throw new ServiceUnavailable($e->getMessage()); } catch (InvalidPathException $ex) { throw new InvalidPath($ex->getMessage(), false, $ex); } catch (ForbiddenException $ex) { @@ -170,22 +157,22 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol * @param string $name * @throws FileLocked * @throws InvalidPath - * @throws \Sabre\DAV\Exception\Forbidden - * @throws \Sabre\DAV\Exception\ServiceUnavailable + * @throws Exception\Forbidden + * @throws ServiceUnavailable */ public function createDirectory($name) { try { if (!$this->info->isCreatable()) { - throw new \Sabre\DAV\Exception\Forbidden(); + throw new Exception\Forbidden(); } $this->fileView->verifyPath($this->path, $name); $newPath = $this->path . '/' . $name; if (!$this->fileView->mkdir($newPath)) { - throw new \Sabre\DAV\Exception\Forbidden('Could not create directory ' . $newPath); + throw new Exception\Forbidden('Could not create directory ' . $newPath); } - } catch (\OCP\Files\StorageNotAvailableException $e) { - throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); + } catch (StorageNotAvailableException $e) { + throw new ServiceUnavailable($e->getMessage()); } catch (InvalidPathException $ex) { throw new InvalidPath($ex->getMessage()); } catch (ForbiddenException $ex) { @@ -203,7 +190,7 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol * @return \Sabre\DAV\INode * @throws InvalidPath * @throws \Sabre\DAV\Exception\NotFound - * @throws \Sabre\DAV\Exception\ServiceUnavailable + * @throws ServiceUnavailable */ public function getChild($name, $info = null) { if (!$this->info->isReadable()) { @@ -216,12 +203,12 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol try { $this->fileView->verifyPath($this->path, $name); $info = $this->fileView->getFileInfo($path); - } catch (\OCP\Files\StorageNotAvailableException $e) { - throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage()); + } catch (StorageNotAvailableException $e) { + throw new ServiceUnavailable($e->getMessage()); } catch (InvalidPathException $ex) { throw new InvalidPath($ex->getMessage()); } catch (ForbiddenException $e) { - throw new \Sabre\DAV\Exception\Forbidden(); + throw new Exception\Forbidden(); } } @@ -298,17 +285,17 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol * * @return void * @throws FileLocked - * @throws \Sabre\DAV\Exception\Forbidden + * @throws Exception\Forbidden */ public function delete() { if ($this->path === '' || $this->path === '/' || !$this->info->isDeletable()) { - throw new \Sabre\DAV\Exception\Forbidden(); + throw new Exception\Forbidden(); } try { if (!$this->fileView->rmdir($this->path)) { // assume it wasn't possible to remove due to permission issue - throw new \Sabre\DAV\Exception\Forbidden(); + throw new Exception\Forbidden(); } } catch (ForbiddenException $ex) { throw new Forbidden($ex->getMessage(), $ex->getRetry()); @@ -343,7 +330,7 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol } catch (\OCP\Files\NotFoundException $e) { $logger->warning("error while getting quota into", ['exception' => $e]); return [0, 0]; - } catch (\OCP\Files\StorageNotAvailableException $e) { + } catch (StorageNotAvailableException $e) { $logger->warning("error while getting quota into", ['exception' => $e]); return [0, 0]; } catch (NotPermittedException $e) { @@ -375,7 +362,7 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol * @throws ServiceUnavailable * @throws Forbidden * @throws FileLocked - * @throws \Sabre\DAV\Exception\Forbidden + * @throws Exception\Forbidden */ public function moveInto($targetName, $fullSourcePath, INode $sourceNode) { if (!$sourceNode instanceof Node) { @@ -399,7 +386,7 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol // at getNodeForPath we also check the path for isForbiddenFileOrDir // with that we have covered both source and destination if ($sourceNode instanceof Directory && $targetNodeExists) { - throw new \Sabre\DAV\Exception\Forbidden('Could not copy directory ' . $sourceNode->getName() . ', target exists'); + throw new Exception\Forbidden('Could not copy directory ' . $sourceNode->getName() . ', target exists'); } [$sourceDir,] = \Sabre\Uri\split($sourceNode->getPath()); @@ -420,11 +407,11 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol if ($targetNodeExists || $sameFolder) { // note that renaming a share mount point is always allowed if (!$this->fileView->isUpdatable($destinationDir) && !$isMovableMount) { - throw new \Sabre\DAV\Exception\Forbidden(); + throw new Exception\Forbidden(); } } else { if (!$this->fileView->isCreatable($destinationDir)) { - throw new \Sabre\DAV\Exception\Forbidden(); + throw new Exception\Forbidden(); } } @@ -432,7 +419,7 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol // moving to a different folder, source will be gone, like a deletion // note that moving a share mount point is always allowed if (!$this->fileView->isDeletable($sourcePath) && !$isMovableMount) { - throw new \Sabre\DAV\Exception\Forbidden(); + throw new Exception\Forbidden(); } } @@ -445,7 +432,7 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol $renameOkay = $this->fileView->rename($sourcePath, $destinationPath); if (!$renameOkay) { - throw new \Sabre\DAV\Exception\Forbidden(''); + throw new Exception\Forbidden(''); } } catch (StorageNotAvailableException $e) { throw new ServiceUnavailable($e->getMessage()); @@ -465,7 +452,7 @@ class Directory extends \OCA\DAV\Connector\Sabre\Node implements \Sabre\DAV\ICol $sourcePath = $sourceNode->getPath(); if (!$this->fileView->isCreatable($this->getPath())) { - throw new \Sabre\DAV\Exception\Forbidden(); + throw new Exception\Forbidden(); } try { diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index b0f17417d21..075026142ec 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -148,15 +148,6 @@ class File extends Node implements IFile { // verify path of the target $this->verifyPath(); - // chunked handling - if (isset($_SERVER['HTTP_OC_CHUNKED'])) { - 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); @@ -578,132 +569,6 @@ class File extends Node implements IFile { } /** - * @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 (isset($_SERVER['REQUEST_METHOD']) && $_SERVER['REQUEST_METHOD'] === 'PUT') { - if (isset($_SERVER['CONTENT_LENGTH'])) { - $expected = (int)$_SERVER['CONTENT_LENGTH']; - 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 - if (isset($this->request->server['HTTP_X_OC_MTIME'])) { - $mtime = $this->sanitizeMtime($this->request->server['HTTP_X_OC_MTIME']); - 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); - - if (isset($this->request->server['HTTP_OC_CHECKSUM'])) { - $checksum = trim($this->request->server['HTTP_OC_CHECKSUM']); - $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 * * @param \Exception $e diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index a6c9b8b4ebe..4a5c071848c 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -577,15 +577,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 6305b0ec138..1f3c5211986 100644 --- a/apps/dav/lib/Connector/Sabre/LockPlugin.php +++ b/apps/dav/lib/Connector/Sabre/LockPlugin.php @@ -61,7 +61,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 { @@ -84,7 +84,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 c129371e376..8a147d7396f 100644 --- a/apps/dav/lib/Connector/Sabre/ObjectTree.php +++ b/apps/dav/lib/Connector/Sabre/ObjectTree.php @@ -68,35 +68,6 @@ class ObjectTree extends CachingTree { } /** - * 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 * * @param string $path @@ -147,9 +118,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); @@ -159,12 +127,6 @@ class ObjectTree extends CachingTree { } } catch (StorageNotAvailableException $e) { throw new \Sabre\DAV\Exception\ServiceUnavailable('Storage is temporarily not available', 0, $e); - } catch (StorageInvalidException $e) { - throw new \Sabre\DAV\Exception\NotFound('Storage ' . $path . ' is invalid'); - } catch (LockedException $e) { - throw new \Sabre\DAV\Exception\Locked(); - } catch (ForbiddenException $e) { - throw new \Sabre\DAV\Exception\Forbidden(); } } diff --git a/apps/dav/lib/Connector/Sabre/QuotaPlugin.php b/apps/dav/lib/Connector/Sabre/QuotaPlugin.php index ddf4b2773e0..2b233d00437 100644 --- a/apps/dav/lib/Connector/Sabre/QuotaPlugin.php +++ b/apps/dav/lib/Connector/Sabre/QuotaPlugin.php @@ -193,31 +193,14 @@ class QuotaPlugin extends \Sabre\DAV\ServerPlugin { $parentPath = ''; } $req = $this->server->httpRequest; - 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']; - } $freeSpace = $this->getFreeSpace($path); if ($freeSpace >= 0 && $length > $freeSpace) { - if (isset($chunkHandler)) { - $chunkHandler->cleanup(); - } throw new InsufficientStorage("Insufficient space in $path, $length required, $freeSpace available"); } } 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 8d72fb13b78..fd75508398f 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -73,7 +73,6 @@ class FileTest extends TestCase { protected function setUp(): void { parent::setUp(); - unset($_SERVER['HTTP_OC_CHUNKED']); unset($_SERVER['CONTENT_LENGTH']); unset($_SERVER['REQUEST_METHOD']); @@ -91,7 +90,6 @@ class FileTest extends TestCase { protected function tearDown(): void { $userManager = \OC::$server->getUserManager(); $userManager->get($this->user)->delete(); - unset($_SERVER['HTTP_OC_CHUNKED']); parent::tearDown(); } @@ -235,81 +233,6 @@ class FileTest extends TestCase { } /** - * Test putting a file using chunking - * - * @dataProvider fopenFailuresProvider - */ - public function testChunkedPutFails($thrownException, $expectedException, $checkPreviousClass = false): void { - // setup - $storage = $this->getMockBuilder(Local::class) - ->setMethods(['fopen']) - ->setConstructorArgs([['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]]) - ->getMock(); - \OC\Files\Filesystem::mount($storage, [], $this->user . '/'); - $view = $this->getMockBuilder(View::class) - ->setMethods(['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); - - $_SERVER['HTTP_OC_CHUNKED'] = true; - - $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); - - // 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); - - // 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. * * @param string $path path to put the file into @@ -438,41 +361,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' => $requestMtime, - ] - ], $this->requestId, $this->config, null); - - $_SERVER['HTTP_OC_CHUNKED'] = true; - $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 { - $_SERVER['HTTP_OC_CHUNKED'] = true; - $this->assertNull($this->doPut('/test.txt-chunking-12345-2-0')); - $this->assertNotEmpty($this->doPut('/test.txt-chunking-12345-2-1')); - } - - /** * Test that putting a file triggers create hooks */ public function testPutSingleFileTriggersHooks(): void { @@ -574,75 +462,6 @@ class FileTest extends TestCase { ); } - /** - * Test that putting a file with chunks triggers create hooks - */ - public function testPutChunkedFileTriggersHooks(): void { - HookHelper::setUpHooks(); - - $_SERVER['HTTP_OC_CHUNKED'] = true; - $this->assertNull($this->doPut('/foo.txt-chunking-12345-2-0')); - $this->assertNotEmpty($this->doPut('/foo.txt-chunking-12345-2-1')); - - $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(); - - $_SERVER['HTTP_OC_CHUNKED'] = true; - $this->assertNull($this->doPut('/foo.txt-chunking-12345-2-0')); - $this->assertNotEmpty($this->doPut('/foo.txt-chunking-12345-2-1')); - - $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, @@ -756,46 +575,6 @@ class FileTest extends TestCase { } /** - * 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); - - $_SERVER['HTTP_OC_CHUNKED'] = true; - - $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); - $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); - - // 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 */ public function testSimplePutInvalidChars(): void { diff --git a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php index d219888ef15..86de3d81334 100644 --- a/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/ObjectTreeTest.php @@ -148,13 +148,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(); @@ -191,8 +186,6 @@ class ObjectTreeTest extends \Test\TestCase { } else { $this->assertTrue($node instanceof \OCA\DAV\Connector\Sabre\Directory); } - - unset($_SERVER['HTTP_OC_CHUNKED']); } public function nodeForPathProvider() { @@ -202,94 +195,67 @@ class ObjectTreeTest extends \Test\TestCase { 'regularfile.txt', 'regularfile.txt', 'regularfile.txt', - 'file', - false + 'file' ], // regular directory [ 'regulardir', 'regulardir', 'regulardir', - 'dir', - false + 'dir' ], // regular file with chunking [ 'regularfile.txt', 'regularfile.txt', 'regularfile.txt', - 'file', - true + 'file' ], // 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 + 'dir' ], // regular file in subdir [ 'subdir/regularfile.txt', 'subdir/regularfile.txt', 'regularfile.txt', - 'file', - false + 'file' ], // regular directory in subdir [ 'subdir/regulardir', '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 + 'dir' ], ]; } public function testGetNodeForPathInvalidPath(): void { - $this->expectException(\OCA\DAV\Connector\Sabre\Exception\InvalidPath::class); - $path = '/foo\bar'; - - $storage = new Temporary([]); - + $rootNode = $this->getMockBuilder(Directory::class) + ->disableOriginalConstructor() + ->getMock(); + $mountManager = $this->createMock(IMountManager::class); $view = $this->getMockBuilder(View::class) ->setMethods(['resolvePath']) ->getMock(); + + $this->expectException(\OCA\DAV\Connector\Sabre\Exception\InvalidPath::class); $view->expects($this->once()) ->method('resolvePath') ->willReturnCallback(function ($path) use ($storage) { return [$storage, ltrim($path, '/')]; }); - $rootNode = $this->getMockBuilder(Directory::class) - ->disableOriginalConstructor() - ->getMock(); - $mountManager = $this->createMock(IMountManager::class); - $tree = new \OCA\DAV\Connector\Sabre\ObjectTree(); $tree->init($rootNode, $view, $mountManager); - $tree->getNodeForPath($path); } diff --git a/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php index 4a9ca159bbd..75f799ada71 100644 --- a/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/QuotaPluginTest.php @@ -52,10 +52,7 @@ class QuotaPluginTest extends TestCase { private function init($quota, $checkedPath = ''): void { $view = $this->buildFileViewMock($quota, $checkedPath); $this->server = new \Sabre\DAV\Server(); - $this->plugin = $this->getMockBuilder(QuotaPlugin::class) - ->setConstructorArgs([$view]) - ->setMethods(['getFileChunking']) - ->getMock(); + $this->plugin = new QuotaPlugin($view); $this->plugin->initialize($this->server); } @@ -64,8 +61,6 @@ class QuotaPluginTest extends TestCase { */ public function testLength($expected, $headers): void { $this->init(0); - $this->plugin->expects($this->never()) - ->method('getFileChunking'); $this->server->httpRequest = new \Sabre\HTTP\Request('POST', 'dummy.file', $headers); $length = $this->plugin->getLength(); $this->assertEquals($expected, $length); @@ -76,8 +71,6 @@ class QuotaPluginTest extends TestCase { */ public function testCheckQuota($quota, $headers): void { $this->init($quota); - $this->plugin->expects($this->never()) - ->method('getFileChunking'); $this->server->httpRequest = new \Sabre\HTTP\Request('POST', 'dummy.file', $headers); $result = $this->plugin->checkQuota(''); @@ -91,8 +84,6 @@ class QuotaPluginTest extends TestCase { $this->expectException(\Sabre\DAV\Exception\InsufficientStorage::class); $this->init($quota); - $this->plugin->expects($this->never()) - ->method('getFileChunking'); $this->server->httpRequest = new \Sabre\HTTP\Request('POST', 'dummy.file', $headers); $this->plugin->checkQuota(''); @@ -103,8 +94,6 @@ class QuotaPluginTest extends TestCase { */ public function testCheckQuotaOnPath($quota, $headers): void { $this->init($quota, 'sub/test.txt'); - $this->plugin->expects($this->never()) - ->method('getFileChunking'); $this->server->httpRequest = new \Sabre\HTTP\Request('POST', 'dummy.file', $headers); $result = $this->plugin->checkQuota('/sub/test.txt'); @@ -154,84 +143,6 @@ class QuotaPluginTest extends TestCase { ]; } - public function quotaChunkedOkProvider() { - return [ - [1024, 0, ['X-EXPECTED-ENTITY-LENGTH' => '1024']], - [1024, 0, ['CONTENT-LENGTH' => '512']], - [1024, 0, ['OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512']], - // with existing chunks (allowed size = total length - chunk total size) - [400, 128, ['X-EXPECTED-ENTITY-LENGTH' => '512']], - [400, 128, ['CONTENT-LENGTH' => '512']], - [400, 128, ['OC-TOTAL-LENGTH' => '512', 'CONTENT-LENGTH' => '500']], - // \OCP\Files\FileInfo::SPACE-UNKNOWN = -2 - [-2, 0, ['X-EXPECTED-ENTITY-LENGTH' => '1024']], - [-2, 0, ['CONTENT-LENGTH' => '512']], - [-2, 0, ['OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512']], - [-2, 128, ['X-EXPECTED-ENTITY-LENGTH' => '1024']], - [-2, 128, ['CONTENT-LENGTH' => '512']], - [-2, 128, ['OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512']], - ]; - } - - /** - * @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']], - [400, 0, ['CONTENT-LENGTH' => '512']], - [400, 0, ['OC-TOTAL-LENGTH' => '1024', 'CONTENT-LENGTH' => '512']], - // with existing chunks (allowed size = total length - chunk total size) - [380, 128, ['X-EXPECTED-ENTITY-LENGTH' => '512']], - [380, 128, ['CONTENT-LENGTH' => '512']], - [380, 128, ['OC-TOTAL-LENGTH' => '512', 'CONTENT-LENGTH' => '500']], - ]; - } - - /** - * @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/RequestTestCase.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php index f6aa79eb6c4..564d8c5938c 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/RequestTestCase.php @@ -57,8 +57,6 @@ abstract class RequestTestCase extends TestCase { protected function setUp(): void { parent::setUp(); - unset($_SERVER['HTTP_OC_CHUNKED']); - $this->serverFactory = new ServerFactory( \OC::$server->getConfig(), \OC::$server->get(LoggerInterface::class), diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php index 16953d9b598..f8d5ad40b49 100644 --- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php @@ -92,114 +92,4 @@ class UploadTest extends RequestTestCase { $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()); - } } |