diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index bec76a0323649..70cbeea1aa7f3 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -194,7 +194,7 @@ public function __construct(IRequest $request, string $baseUri) { $this->server->addPlugin(new CopyEtagHeaderPlugin()); $this->server->addPlugin(new RequestIdHeaderPlugin(\OC::$server->get(IRequest::class))); - $this->server->addPlugin(new ChunkingV2Plugin(\OCP\Server::get(ICacheFactory::class))); + $this->server->addPlugin(new ChunkingV2Plugin(\OCP\Server::get(ICacheFactory::class), \OCP\Server::get(LoggerInterface::class))); $this->server->addPlugin(new ChunkingPlugin()); // allow setup of additional plugins diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index 846b4ea42ba40..4d9b441833b98 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -1,6 +1,7 @@ -cache = $cacheFactory->createDistributed(self::CACHE_KEY); } /** * @inheritdoc */ - public function initialize(Server $server) { + public function initialize(Server $server): void { $server->on('afterMethod:MKCOL', [$this, 'afterMkcol']); $server->on('beforeMethod:PUT', [$this, 'beforePut']); $server->on('beforeMethod:DELETE', [$this, 'beforeDelete']); @@ -75,12 +74,7 @@ public function initialize(Server $server) { $this->server = $server; } - /** - * @param string $path - * @param bool $createIfNotExists - * @return FutureFile|UploadFile|ICollection|INode - */ - private function getUploadFile(string $path, bool $createIfNotExists = false) { + private function getUploadFile(string $path, bool $createIfNotExists = false): UploadFile|File { try { $actualFile = $this->server->tree->getNodeForPath($path); // Only directly upload to the target file if it is on the same storage @@ -109,6 +103,7 @@ public function afterMkcol(RequestInterface $request, ResponseInterface $respons $this->prepareUpload($request->getPath()); $this->checkPrerequisites(false); } catch (BadRequest|StorageInvalidException|NotFound $e) { + $this->logger->debug('Preconditions for chunking v2 not matched, fallback to v1', ['exception' => $e]); return true; } @@ -133,12 +128,14 @@ public function beforePut(RequestInterface $request, ResponseInterface $response $this->prepareUpload(dirname($request->getPath())); $this->checkPrerequisites(); } catch (StorageInvalidException|BadRequest|NotFound $e) { + $this->logger->debug('Preconditions for chunking v2 not matched, fallback to v1', ['exception' => $e]); return true; } [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); $chunkName = basename($request->getPath()); + // S3 Multipart upload allows up to 10000 chunks at maximum and required them to be numeric $partId = is_numeric($chunkName) ? (int)$chunkName : -1; if (!($partId >= 1 && $partId <= 10000)) { throw new BadRequest('Invalid chunk name, must be numeric between 1 and 10000'); @@ -151,7 +148,7 @@ public function beforePut(RequestInterface $request, ResponseInterface $response if ($this->uploadFolder->childExists(self::TEMP_TARGET) && $this->uploadPath) { /** @var UploadFile $tempTargetFile */ $tempTargetFile = $this->uploadFolder->getChild(self::TEMP_TARGET); - [$destinationDir, $destinationName] = Uri\split($this->uploadPath); + [$destinationDir, ] = Uri\split($this->uploadPath); /** @var Directory $destinationParent */ $destinationParent = $this->server->tree->getNodeForPath($destinationDir); $free = $destinationParent->getNode()->getFreeSpace(); @@ -173,14 +170,15 @@ public function beforePut(RequestInterface $request, ResponseInterface $response return false; } - public function beforeMove($sourcePath, $destination): bool { + public function beforeMove(string $sourcePath, string $destination): bool { try { $this->prepareUpload(dirname($sourcePath)); $this->checkPrerequisites(); - } catch (StorageInvalidException|BadRequest|NotFound|PreconditionFailed $e) { + } catch (StorageInvalidException|BadRequest|NotFound $e) { + $this->logger->debug('Preconditions for chunking v2 not matched, fallback to v1', ['exception' => $e]); return true; } - [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); + [$storage, ] = $this->getUploadStorage($this->uploadPath); $targetFile = $this->getUploadFile($this->uploadPath); @@ -239,11 +237,12 @@ public function beforeMove($sourcePath, $destination): bool { return false; } - public function beforeDelete(RequestInterface $request, ResponseInterface $response) { + public function beforeDelete(RequestInterface $request, ResponseInterface $response): bool { try { $this->prepareUpload(dirname($request->getPath())); $this->checkPrerequisites(); } catch (StorageInvalidException|BadRequest|NotFound $e) { + $this->logger->debug('Preconditions for chunking v2 not matched, fallback to v1', ['exception' => $e]); return true; } @@ -256,6 +255,8 @@ public function beforeDelete(RequestInterface $request, ResponseInterface $respo * @throws BadRequest * @throws PreconditionFailed * @throws StorageInvalidException + * @throws ContainerExceptionInterface + * @throws NotFoundExceptionInterface */ private function checkPrerequisites(bool $checkUploadMetadata = true): void { $distributedCacheConfig = \OCP\Server::get(IConfig::class)->getSystemValue('memcache.distributed', null); @@ -266,10 +267,12 @@ private function checkPrerequisites(bool $checkUploadMetadata = true): void { if (!$this->uploadFolder instanceof UploadFolder || empty($this->server->httpRequest->getHeader(self::DESTINATION_HEADER))) { throw new BadRequest('Skipping chunked file writing as the destination header was not passed'); } - if (!$this->uploadFolder->getStorage()->instanceOfStorage(IChunkedFileWrite::class)) { + /** @var ObjectStoreStorage $storage */ + $storage = $this->uploadFolder->getStorage(); + if (!$storage->instanceOfStorage(IChunkedFileWrite::class)) { throw new StorageInvalidException('Storage does not support chunked file writing'); } - if ($this->uploadFolder->getStorage()->instanceOfStorage(ObjectStoreStorage::class) && !$this->uploadFolder->getStorage()->getObjectStore() instanceof IObjectStoreMultiPartUpload) { + if ($storage->instanceOfStorage(ObjectStoreStorage::class) && !$storage->getObjectStore() instanceof IObjectStoreMultiPartUpload) { throw new StorageInvalidException('Storage does not support multi part uploads'); } @@ -300,8 +303,12 @@ protected function sanitizeMtime(string $mtimeFromRequest): int { /** * @throws NotFound */ - public function prepareUpload($path): void { - $this->uploadFolder = $this->server->tree->getNodeForPath($path); + public function prepareUpload(string $path): void { + $uploadFolder = $this->server->tree->getNodeForPath($path); + if (!$uploadFolder instanceof UploadFolder) { + throw new \RuntimeException('Unexpected node returned, should be the upload folder'); + } + $this->uploadFolder = $uploadFolder; $uploadMetadata = $this->cache->get($this->uploadFolder->getName()); $this->uploadId = $uploadMetadata[self::UPLOAD_ID] ?? null; $this->uploadPath = $uploadMetadata[self::UPLOAD_TARGET_PATH] ?? null; @@ -329,7 +336,7 @@ private function completeChunkedWrite(string $targetAbsolutePath): void { try { $uploadFileAbsolutePath = $uploadFile->getFileInfo()->getPath(); if ($uploadFileAbsolutePath !== $targetAbsolutePath) { - $uploadFile = $rootFolder->get($uploadFile->getFileInfo()->getPath()); + $uploadFile = $rootFolder->get($uploadFile->getPath()); if ($exists) { $uploadFile->copy($targetAbsolutePath); } else { diff --git a/apps/dav/lib/Upload/FutureFile.php b/apps/dav/lib/Upload/FutureFile.php index 5ef15bd2b56e4..d67414d6d73f3 100644 --- a/apps/dav/lib/Upload/FutureFile.php +++ b/apps/dav/lib/Upload/FutureFile.php @@ -49,7 +49,7 @@ public function get() { return AssemblyStream::wrap($nodes); } - public function getPath() { + public function getPath(): string { return $this->root->getFileInfo()->getInternalPath() . '/.file'; } diff --git a/apps/dav/lib/Upload/PartFile.php b/apps/dav/lib/Upload/PartFile.php index a711fe083b989..d9190fc17899b 100644 --- a/apps/dav/lib/Upload/PartFile.php +++ b/apps/dav/lib/Upload/PartFile.php @@ -1,10 +1,13 @@ root = $root; @@ -40,7 +41,7 @@ public function get() { throw new Forbidden('Permission denied to get this file'); } - public function getPath() { + public function getPath(): string { return $this->root->getFileInfo()->getInternalPath() . '/' . $this->partInfo['PartNumber']; } diff --git a/apps/dav/lib/Upload/UploadFile.php b/apps/dav/lib/Upload/UploadFile.php index ae64760e0ce07..26d8c4e321c75 100644 --- a/apps/dav/lib/Upload/UploadFile.php +++ b/apps/dav/lib/Upload/UploadFile.php @@ -27,7 +27,7 @@ public function get() { return $this->file->get(); } - public function getId() { + public function getId(): int { return $this->file->getId(); } @@ -71,7 +71,7 @@ public function getFile(): File { return $this->file; } - public function getNode() { + public function getNode(): \OCP\Files\File { return $this->file->getNode(); } } diff --git a/apps/dav/lib/Upload/UploadFolder.php b/apps/dav/lib/Upload/UploadFolder.php index 3f84f7a85974a..e3eab5ebbb29a 100644 --- a/apps/dav/lib/Upload/UploadFolder.php +++ b/apps/dav/lib/Upload/UploadFolder.php @@ -11,16 +11,14 @@ use OCA\DAV\Connector\Sabre\Directory; use OCP\Files\ObjectStore\IObjectStoreMultiPartUpload; use OCP\Files\Storage\IStorage; +use OCP\ICacheFactory; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\ICollection; class UploadFolder implements ICollection { - /** @var Directory */ - private $node; - /** @var CleanupService */ - private $cleanupService; - /** @var IStorage */ - private $storage; + private Directory $node; + private CleanupService $cleanupService; + private IStorage $storage; public function __construct(Directory $node, CleanupService $cleanupService, IStorage $storage) { $this->node = $node; @@ -66,7 +64,7 @@ public function getChildren() { /** @var ObjectStoreStorage $storage */ $objectStore = $this->storage->getObjectStore(); if ($objectStore instanceof IObjectStoreMultiPartUpload) { - $cache = \OC::$server->getMemCacheFactory()->createDistributed(ChunkingV2Plugin::CACHE_KEY); + $cache = \OCP\Server::get(ICacheFactory::class)->createDistributed(ChunkingV2Plugin::CACHE_KEY); $uploadSession = $cache->get($this->getName()); if ($uploadSession) { $uploadId = $uploadSession[ChunkingV2Plugin::UPLOAD_ID]; @@ -108,7 +106,7 @@ public function getLastModified() { return $this->node->getLastModified(); } - public function getStorage() { + public function getStorage(): IStorage { return $this->storage; } } diff --git a/apps/dav/lib/Upload/UploadHome.php b/apps/dav/lib/Upload/UploadHome.php index 3e7e3c6c986a8..8dd59604a8b4b 100644 --- a/apps/dav/lib/Upload/UploadHome.php +++ b/apps/dav/lib/Upload/UploadHome.php @@ -10,6 +10,7 @@ use OC\Files\Filesystem; use OC\Files\View; use OCA\DAV\Connector\Sabre\Directory; +use OCP\Files\Storage\IStorage; use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\ICollection; @@ -69,13 +70,13 @@ public function getLastModified() { /** * @return Directory */ - private function impl() { + private function impl(): Directory { $view = $this->getView(); $rootInfo = $view->getFileInfo(''); return new Directory($view, $rootInfo); } - private function getView() { + private function getView(): View { $rootView = new View(); $user = \OC::$server->getUserSession()->getUser(); Filesystem::initMountPoints($user->getUID()); @@ -85,7 +86,7 @@ private function getView() { return new View('/' . $user->getUID() . '/uploads'); } - private function getStorage() { + private function getStorage(): IStorage { $view = $this->getView(); $storage = $view->getFileInfo('')->getStorage(); return $storage; diff --git a/lib/private/Files/ObjectStore/ObjectStoreStorage.php b/lib/private/Files/ObjectStore/ObjectStoreStorage.php index 389f744eab41c..3494a7db8067d 100644 --- a/lib/private/Files/ObjectStore/ObjectStoreStorage.php +++ b/lib/private/Files/ObjectStore/ObjectStoreStorage.php @@ -661,10 +661,6 @@ public function startChunkedWrite(string $targetPath): string { return $this->objectStore->initiateMultipartUpload($urn); } - /** - * - * @throws GenericFileException - */ public function putChunkedWritePart( string $targetPath, string $writeToken, @@ -675,6 +671,11 @@ public function putChunkedWritePart( if (!$this->objectStore instanceof IObjectStoreMultiPartUpload) { throw new GenericFileException('Object store does not support multipart upload'); } + + if (!is_numeric($chunkId)) { + throw new GenericFileException('Chunk ID must be numeric for S3 multipart upload'); + } + $cacheEntry = $this->getCache()->get($targetPath); $urn = $this->getURN($cacheEntry->getId()); diff --git a/lib/public/Files/ObjectStore/IObjectStoreMultiPartUpload.php b/lib/public/Files/ObjectStore/IObjectStoreMultiPartUpload.php index 7989e27dfc851..c3a04da220103 100644 --- a/lib/public/Files/ObjectStore/IObjectStoreMultiPartUpload.php +++ b/lib/public/Files/ObjectStore/IObjectStoreMultiPartUpload.php @@ -5,6 +5,7 @@ * SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ + namespace OCP\Files\ObjectStore; use Aws\Result; @@ -35,6 +36,15 @@ public function abortMultipartUpload(string $urn, string $uploadId): void; /** * @since 26.0.0 + * @return array of the parts in ListParts response + * + * Sample data: + * [[ + * 'PartNumber' => 1, + * 'LastModified' => '2010-11-10T20:48:34.000Z', + * 'ETag' => '"7778aef83f66abc1fa1e8477f296d394"', + * 'Size' => 10485760, + * ]] */ public function getMultipartUploads(string $urn, string $uploadId): array; } diff --git a/lib/public/Files/Storage/IChunkedFileWrite.php b/lib/public/Files/Storage/IChunkedFileWrite.php index 1095ee7cbfc73..fd205ea276445 100644 --- a/lib/public/Files/Storage/IChunkedFileWrite.php +++ b/lib/public/Files/Storage/IChunkedFileWrite.php @@ -1,10 +1,12 @@