diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index 1c06a9774074a..1ea426a228c75 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -290,14 +290,58 @@ public function beforeMove($sourcePath, $destination): bool { public function beforeDelete(RequestInterface $request, ResponseInterface $response) { try { - $this->prepareUpload(dirname($request->getPath())); - $this->checkPrerequisites(); - } catch (StorageInvalidException|BadRequest|NotFound $e) { + // DELETE targets the upload folder itself, unlike PUT and MOVE, + // which target a child within the upload folder. + $this->prepareUpload($request->getPath()); + } catch (NotFound) { + // Not an upload folder handled by this plugin. Let normal DAV + // processing produce the appropriate result. + return true; + } + + if (!$this->uploadFolder instanceof UploadFolder) { + return true; + } + + // No v2 metadata means this request cannot be handled as a v2 upload. + // It may be a Chunking v1 upload, or the v2 session may have already + // expired. In either case, there is no backend write token to cancel. + // Let normal DAV deletion handle the upload folder. + if ($this->uploadId === null && $this->uploadPath === null) { return true; } + // Partially present metadata indicates a corrupt/inconsistent v2 + // session. Do not delete the remaining upload state silently. + if ($this->uploadId === null || $this->uploadPath === null) { + throw new PreconditionFailed( + 'Incomplete metadata for chunked upload' + ); + } + + $storage = $this->uploadFolder->getStorage(); + if (!$storage->instanceOfStorage(IChunkedFileWrite::class)) { + throw new StorageInvalidException( + 'Storage does not support chunked file writing' + ); + } + + if ( + $storage->instanceOfStorage(ObjectStoreStorage::class) + && !$storage->getObjectStore() instanceof IObjectStoreMultiPartUpload + ) { + throw new StorageInvalidException( + 'Storage does not support multi part uploads' + ); + } + [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); $storage->cancelChunkedWrite($storagePath, $this->uploadId); + + // The backend session no longer exists. Avoid attempting to cancel it + // again if ordinary DAV deletion subsequently has to be retried. + $this->cache->remove($this->uploadFolder->getName()); + return true; } diff --git a/apps/dav/tests/unit/Upload/ChunkingV2PluginTest.php b/apps/dav/tests/unit/Upload/ChunkingV2PluginTest.php index 25a032b433182..3cabc72d02848 100644 --- a/apps/dav/tests/unit/Upload/ChunkingV2PluginTest.php +++ b/apps/dav/tests/unit/Upload/ChunkingV2PluginTest.php @@ -9,15 +9,26 @@ namespace OCA\DAV\Tests\unit\Upload; +use OC\Files\ObjectStore\ObjectStoreStorage; use OCA\DAV\Connector\Sabre\Directory; +use OCA\DAV\Connector\Sabre\File as DavFile; use OCA\DAV\Upload\ChunkingV2Plugin; use OCA\DAV\Upload\FutureFile; use OCA\DAV\Upload\UploadFile; +use OCA\DAV\Upload\UploadFolder; +use OCP\Files\File; +use OCP\Files\GenericFileException; +use OCP\Files\ObjectStore\IObjectStore; +use OCP\Files\Storage\IChunkedFileWrite; +use OCP\Files\Storage\IStorage; +use OCP\Files\StorageInvalidException; use OCP\ICache; use OCP\ICacheFactory; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Sabre\DAV\Exception\MethodNotAllowed; use Sabre\DAV\Exception\NotFound; +use Sabre\DAV\Exception\PreconditionFailed; use Sabre\DAV\Server; use Sabre\DAV\Tree; use Sabre\HTTP\RequestInterface; @@ -35,6 +46,7 @@ class ChunkingV2PluginTest extends TestCase { private $request; /** @var ResponseInterface | MockObject */ private $response; + private ICache&MockObject $cache; protected function setUp(): void { parent::setUp(); @@ -47,8 +59,12 @@ protected function setUp(): void { ->getMock(); $this->server->tree = $this->tree; + $this->cache = $this->createMock(ICache::class); + $cacheFactory = $this->createMock(ICacheFactory::class); - $cacheFactory->method('createDistributed')->willReturn($this->createMock(ICache::class)); + $cacheFactory->method('createDistributed') + ->with(ChunkingV2Plugin::CACHE_KEY) + ->willReturn($this->cache); $this->plugin = new ChunkingV2Plugin($cacheFactory); @@ -102,4 +118,453 @@ public function testBeforeGetAllowsRegularNode(): void { $this->assertTrue($this->plugin->forbiddenMethod($this->request)); } + + public function testBeforeDeleteCancelsV2UploadWithoutDestination(): void { + $uploadPath = 'uploads/admin/upload-id'; + $targetPath = 'files/admin/file.zip'; + $targetInternalPath = 'files/file.zip'; + $uploadId = 'multipart-upload-id'; + + $storage = $this->createMock(IChunkedFileWrite::class); + $storage->method('instanceOfStorage') + ->willReturnCallback( + static fn (string $class): bool => $class === IChunkedFileWrite::class, + ); + $storage->method('getId')->willReturn('storage-id'); + + $uploadFolder = $this->createUploadFolder($storage); + $targetFile = $this->createTargetFile($storage, $targetInternalPath); + + $this->request->expects($this->once()) + ->method('getPath') + ->willReturn($uploadPath); + $this->request->expects($this->never()) + ->method('getHeader'); + + $this->tree->expects($this->exactly(2)) + ->method('getNodeForPath') + ->willReturnCallback( + static function (string $path) use ( + $uploadPath, + $uploadFolder, + $targetPath, + $targetFile, + ) { + return match ($path) { + $uploadPath => $uploadFolder, + $targetPath => $targetFile, + default => throw new \LogicException("Unexpected path: $path"), + }; + }, + ); + + $this->cache->expects($this->once()) + ->method('get') + ->with('upload-id') + ->willReturn([ + ChunkingV2Plugin::UPLOAD_ID => $uploadId, + ChunkingV2Plugin::UPLOAD_TARGET_PATH => $targetPath, + ChunkingV2Plugin::UPLOAD_TARGET_ID => 42, + ]); + + $canceled = false; + $storage->expects($this->once()) + ->method('cancelChunkedWrite') + ->with($targetInternalPath, $uploadId) + ->willReturnCallback( + static function () use (&$canceled): void { + $canceled = true; + }, + ); + $this->cache->expects($this->once()) + ->method('remove') + ->with('upload-id') + ->willReturnCallback( + function () use (&$canceled): bool { + $this->assertTrue( + $canceled, + 'The cache entry must not be removed before the backend write is canceled', + ); + return true; + }, + ); + + $this->assertTrue( + $this->plugin->beforeDelete($this->request, $this->response), + ); + } + + public function testBeforeDeleteAllowsDavToHandleMissingPath(): void { + $uploadPath = 'uploads/admin/missing-upload'; + + $this->request->method('getPath')->willReturn($uploadPath); + + $this->tree->expects($this->once()) + ->method('getNodeForPath') + ->with($uploadPath) + ->willThrowException(new NotFound()); + + $this->cache->expects($this->never()) + ->method('get'); + $this->cache->expects($this->never()) + ->method('remove'); + + $this->assertTrue( + $this->plugin->beforeDelete($this->request, $this->response), + ); + } + + public function testBeforeDeleteAllowsDavToHandleNonUploadFolder(): void { + $path = 'files/admin/folder'; + $directory = $this->createMock(Directory::class); + $directory->method('getName')->willReturn('folder'); + + $this->request->method('getPath')->willReturn($path); + + $this->tree->expects($this->once()) + ->method('getNodeForPath') + ->with($path) + ->willReturn($directory); + + $this->cache->expects($this->once()) + ->method('get') + ->with('folder') + ->willReturn(null); + $this->cache->expects($this->never()) + ->method('remove'); + + $this->assertTrue( + $this->plugin->beforeDelete($this->request, $this->response), + ); + } + + public function testBeforeDeleteAllowsDavToHandleUploadWithoutV2Metadata(): void { + $uploadPath = 'uploads/admin/upload-id'; + + $storage = $this->createMock(IStorage::class); + $uploadFolder = $this->createUploadFolder($storage); + + $this->request->method('getPath')->willReturn($uploadPath); + $this->tree->expects($this->once()) + ->method('getNodeForPath') + ->with($uploadPath) + ->willReturn($uploadFolder); + + $this->cache->expects($this->once()) + ->method('get') + ->with('upload-id') + ->willReturn(null); + $this->cache->expects($this->never()) + ->method('remove'); + + $storage->expects($this->never()) + ->method('instanceOfStorage'); + + $this->assertTrue( + $this->plugin->beforeDelete($this->request, $this->response), + ); + } + + public static function incompleteUploadMetadataProvider(): array { + return [ + 'missing upload ID' => [[ + ChunkingV2Plugin::UPLOAD_TARGET_PATH => 'files/admin/file.zip', + ChunkingV2Plugin::UPLOAD_TARGET_ID => 42, + ]], + 'missing upload target path' => [[ + ChunkingV2Plugin::UPLOAD_ID => 'multipart-upload-id', + ChunkingV2Plugin::UPLOAD_TARGET_ID => 42, + ]], + ]; + } + + /** + * @param array $metadata + */ + #[DataProvider('incompleteUploadMetadataProvider')] + public function testBeforeDeleteRejectsIncompleteUploadMetadata( + array $metadata, + ): void { + $uploadPath = 'uploads/admin/upload-id'; + + $storage = $this->createMock(IChunkedFileWrite::class); + $storage->expects($this->never()) + ->method('cancelChunkedWrite'); + + $uploadFolder = $this->createUploadFolder($storage); + + $this->request->method('getPath')->willReturn($uploadPath); + $this->tree->expects($this->once()) + ->method('getNodeForPath') + ->with($uploadPath) + ->willReturn($uploadFolder); + + $this->cache->expects($this->once()) + ->method('get') + ->with('upload-id') + ->willReturn($metadata); + $this->cache->expects($this->never()) + ->method('remove'); + + $this->expectException(PreconditionFailed::class); + $this->expectExceptionMessage('Incomplete metadata for chunked upload'); + + $this->plugin->beforeDelete($this->request, $this->response); + } + + public function testBeforeDeleteRejectsStorageWithoutChunkedWriteSupport(): void { + $uploadPath = 'uploads/admin/upload-id'; + + $storage = $this->createMock(IStorage::class); + $storage->expects($this->once()) + ->method('instanceOfStorage') + ->with(IChunkedFileWrite::class) + ->willReturn(false); + + $uploadFolder = $this->createUploadFolder($storage); + + $this->request->method('getPath')->willReturn($uploadPath); + $this->tree->expects($this->once()) + ->method('getNodeForPath') + ->with($uploadPath) + ->willReturn($uploadFolder); + + $this->cache->expects($this->once()) + ->method('get') + ->with('upload-id') + ->willReturn([ + ChunkingV2Plugin::UPLOAD_ID => 'multipart-upload-id', + ChunkingV2Plugin::UPLOAD_TARGET_PATH => 'files/admin/file.zip', + ChunkingV2Plugin::UPLOAD_TARGET_ID => 42, + ]); + $this->cache->expects($this->never()) + ->method('remove'); + + $this->expectException(StorageInvalidException::class); + $this->expectExceptionMessage( + 'Storage does not support chunked file writing', + ); + + $this->plugin->beforeDelete($this->request, $this->response); + } + + public function testBeforeDeleteRejectsObjectStorageWithoutMultipartSupport(): void { + $uploadPath = 'uploads/admin/upload-id'; + + $objectStore = $this->createMock(IObjectStore::class); + $storage = $this->createMock(ObjectStoreStorage::class); + $storage->method('instanceOfStorage') + ->willReturnCallback( + static fn (string $class): bool => in_array($class, [ + IChunkedFileWrite::class, + ObjectStoreStorage::class, + ], true), + ); + $storage->method('getObjectStore')->willReturn($objectStore); + $storage->expects($this->never()) + ->method('cancelChunkedWrite'); + + $uploadFolder = $this->createUploadFolder($storage); + + $this->request->method('getPath')->willReturn($uploadPath); + $this->tree->expects($this->once()) + ->method('getNodeForPath') + ->with($uploadPath) + ->willReturn($uploadFolder); + + $this->cache->expects($this->once()) + ->method('get') + ->with('upload-id') + ->willReturn([ + ChunkingV2Plugin::UPLOAD_ID => 'multipart-upload-id', + ChunkingV2Plugin::UPLOAD_TARGET_PATH => 'files/admin/file.zip', + ChunkingV2Plugin::UPLOAD_TARGET_ID => 42, + ]); + $this->cache->expects($this->never()) + ->method('remove'); + + $this->expectException(StorageInvalidException::class); + $this->expectExceptionMessage( + 'Storage does not support multi part uploads', + ); + + $this->plugin->beforeDelete($this->request, $this->response); + } + + public function testBeforeDeletePropagatesCancellationFailure(): void { + $uploadPath = 'uploads/admin/upload-id'; + $targetPath = 'files/admin/file.zip'; + $targetInternalPath = 'files/file.zip'; + $uploadId = 'multipart-upload-id'; + + $storage = $this->createMock(IChunkedFileWrite::class); + $storage->method('instanceOfStorage') + ->willReturnCallback( + static fn (string $class): bool => $class === IChunkedFileWrite::class, + ); + $storage->method('getId')->willReturn('storage-id'); + $storage->expects($this->once()) + ->method('cancelChunkedWrite') + ->with($targetInternalPath, $uploadId) + ->willThrowException(new GenericFileException('Cancellation failed')); + + $uploadFolder = $this->createUploadFolder($storage); + $targetFile = $this->createTargetFile($storage, $targetInternalPath); + + $this->request->method('getPath')->willReturn($uploadPath); + $this->tree->expects($this->exactly(2)) + ->method('getNodeForPath') + ->willReturnCallback( + static fn (string $path) => match ($path) { + $uploadPath => $uploadFolder, + $targetPath => $targetFile, + default => throw new \LogicException("Unexpected path: $path"), + }, + ); + + $this->cache->expects($this->once()) + ->method('get') + ->with('upload-id') + ->willReturn([ + ChunkingV2Plugin::UPLOAD_ID => $uploadId, + ChunkingV2Plugin::UPLOAD_TARGET_PATH => $targetPath, + ChunkingV2Plugin::UPLOAD_TARGET_ID => 42, + ]); + $this->cache->expects($this->never()) + ->method('remove'); + + $this->expectException(GenericFileException::class); + $this->expectExceptionMessage('Cancellation failed'); + + $this->plugin->beforeDelete($this->request, $this->response); + } + + public function testBeforeDeleteCancelsV2UploadUsingTemporaryTarget(): void { + $uploadPath = 'uploads/admin/upload-id'; + $targetPath = 'files/admin/new-file.zip'; + $targetInternalPath = 'uploads/upload-id/.target'; + $uploadId = 'multipart-upload-id'; + + $storage = $this->createMock(IChunkedFileWrite::class); + $storage->method('instanceOfStorage') + ->willReturnCallback( + static fn (string $class): bool => $class === IChunkedFileWrite::class, + ); + $storage->expects($this->once()) + ->method('cancelChunkedWrite') + ->with($targetInternalPath, $uploadId); + + $uploadFolder = $this->createUploadFolder($storage); + + $temporaryDavFile = $this->createMock(DavFile::class); + $temporaryDavFile->method('getInternalPath') + ->willReturn($targetInternalPath); + + $temporaryUploadFile = $this->createMock(UploadFile::class); + $temporaryUploadFile->method('getFile') + ->willReturn($temporaryDavFile); + + $uploadFolder->expects($this->once()) + ->method('getChild') + ->with('.target') + ->willReturn($temporaryUploadFile); + + $this->request->method('getPath')->willReturn($uploadPath); + + $this->tree->expects($this->exactly(2)) + ->method('getNodeForPath') + ->willReturnCallback( + static fn (string $path) => match ($path) { + $uploadPath => $uploadFolder, + $targetPath => throw new NotFound('Upload target not found'), + default => throw new \LogicException("Unexpected path: $path"), + }, + ); + + $this->cache->expects($this->once()) + ->method('get') + ->with('upload-id') + ->willReturn([ + ChunkingV2Plugin::UPLOAD_ID => $uploadId, + ChunkingV2Plugin::UPLOAD_TARGET_PATH => $targetPath, + ChunkingV2Plugin::UPLOAD_TARGET_ID => 42, + ]); + $this->cache->expects($this->once()) + ->method('remove') + ->with('upload-id'); + + $this->assertTrue( + $this->plugin->beforeDelete($this->request, $this->response), + ); + } + + public function testBeforeDeletePropagatesMissingTemporaryTarget(): void { + $uploadPath = 'uploads/admin/upload-id'; + $targetPath = 'files/admin/new-file.zip'; + + $storage = $this->createMock(IChunkedFileWrite::class); + $storage->method('instanceOfStorage') + ->willReturnCallback( + static fn (string $class): bool => $class === IChunkedFileWrite::class, + ); + $storage->expects($this->never()) + ->method('cancelChunkedWrite'); + + $uploadFolder = $this->createUploadFolder($storage); + $uploadFolder->expects($this->once()) + ->method('getChild') + ->with('.target') + ->willThrowException(new NotFound('Temporary upload target not found')); + + $this->request->method('getPath')->willReturn($uploadPath); + + $this->tree->expects($this->exactly(2)) + ->method('getNodeForPath') + ->willReturnCallback( + static fn (string $path) => match ($path) { + $uploadPath => $uploadFolder, + $targetPath => throw new NotFound('Destination does not exist'), + default => throw new \LogicException("Unexpected path: $path"), + }, + ); + + $this->cache->expects($this->once()) + ->method('get') + ->with('upload-id') + ->willReturn([ + ChunkingV2Plugin::UPLOAD_ID => 'multipart-upload-id', + ChunkingV2Plugin::UPLOAD_TARGET_PATH => $targetPath, + ChunkingV2Plugin::UPLOAD_TARGET_ID => 42, + ]); + $this->cache->expects($this->never()) + ->method('remove'); + + $this->expectException(NotFound::class); + $this->expectExceptionMessage('Temporary upload target not found'); + + $this->plugin->beforeDelete($this->request, $this->response); + } + + private function createUploadFolder(IStorage $storage): UploadFolder { + $uploadFolder = $this->createMock(UploadFolder::class); + $uploadFolder->method('getName')->willReturn('upload-id'); + $uploadFolder->method('getStorage')->willReturn($storage); + + return $uploadFolder; + } + + private function createTargetFile( + IStorage $storage, + string $internalPath, + ): DavFile { + $node = $this->createMock(File::class); + $node->method('isUpdateable')->willReturn(true); + $node->method('getStorage')->willReturn($storage); + + $file = $this->createMock(DavFile::class); + $file->method('getNode')->willReturn($node); + $file->method('getInternalPath')->willReturn($internalPath); + + return $file; + } }