From 0696dc8fc18d1a0c649486baf93f2c242a240692 Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 2 Jun 2026 10:24:05 -0400 Subject: [PATCH 1/8] refactor(ChunkingV2Plugin): split up getUploadFile Signed-off-by: Josh --- apps/dav/lib/Upload/ChunkingV2Plugin.php | 61 +++++++++++++++++------- 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index 117bdb835d265..604fa2e56e4a0 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -93,25 +93,54 @@ protected function beforeGet(RequestInterface $request) { * @param bool $createIfNotExists * @return FutureFile|UploadFile|ICollection|INode */ - private function getUploadFile(string $path, bool $createIfNotExists = false) { + private function resolveChunkWriteTargetFile(string $path, bool $createIfNotExists = false) { + if (!$this->uploadFolder instanceof UploadFolder) { + throw new \LogicException('Upload folder not initialized'); + } + + $directTarget = $this->tryGetDirectTargetFile($path); + if ($directTarget !== null) { + return $directTarget; + } + + return $this->getOrCreateStagingFile($createIfNotExists); + } + + private function tryGetDirectTargetFile(string $path): ?File { try { $actualFile = $this->server->tree->getNodeForPath($path); - // Only directly upload to the target file if it is on the same storage - // There may be further potential to optimize here by also uploading - // to other storages directly. This would require to also carefully pick - // the storage/path used in getStorage() - if ($actualFile instanceof File && $this->uploadFolder->getStorage()->getId() === $actualFile->getNode()->getStorage()->getId()) { - return $actualFile; - } } catch (NotFound $e) { - // If there is no target file we upload to the upload folder first + // No target file exists yet; fall back to staging via the upload folder. + return null; } - // Use file in the upload directory that will be copied or moved afterwards - if ($createIfNotExists) { + if (!$actualFile instanceof File) { + return null; + } + + $uploadStorage = $this->uploadFolder->getStorage(); + $actualNode = $actualFile->getNode(); + + // Direct chunked writes are only safe when both nodes share the same + // storage backend; otherwise use the staging file in the upload folder. + // + // This is a conservative optimization: we only bypass the staging file when + // the destination already lives on the same storage as the upload folder. + // Broader direct-write support may be possible, but only if getUploadStorage() + // is changed to return the correct storage/path for that target backend. + if ($uploadStorage->getId() !== $actualNode->getStorage()->getId()) { + return null; + } + + return $actualFile; + } + + private function getOrCreateStagingFile(bool $createIfNotExists) { + if ($createIfNotExists && !$this->uploadFolder->childExists(self::TEMP_TARGET)) { $this->uploadFolder->createFile(self::TEMP_TARGET); } + // Use file in the upload directory that will be copied or moved afterwards /** @var UploadFile $uploadFile */ $uploadFile = $this->uploadFolder->getChild(self::TEMP_TARGET); return $uploadFile->getFile(); @@ -126,7 +155,7 @@ public function afterMkcol(RequestInterface $request, ResponseInterface $respons } $this->uploadPath = $this->server->calculateUri($this->server->httpRequest->getHeader(self::DESTINATION_HEADER)); - $targetFile = $this->getUploadFile($this->uploadPath, true); + $targetFile = $this->resolveChunkWriteTargetFile($this->uploadPath, true); [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); $this->uploadId = $storage->startChunkedWrite($storagePath); @@ -157,7 +186,7 @@ public function beforePut(RequestInterface $request, ResponseInterface $response throw new BadRequest('Invalid chunk name, must be numeric between 1 and 10000'); } - $uploadFile = $this->getUploadFile($this->uploadPath); + $uploadFile = $this->resolveChunkWriteTargetFile($this->uploadPath); $tempTargetFile = null; $additionalSize = (int)$request->getHeader('Content-Length'); @@ -195,7 +224,7 @@ public function beforeMove($sourcePath, $destination): bool { } [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); - $targetFile = $this->getUploadFile($this->uploadPath); + $targetFile = $this->resolveChunkWriteTargetFile($this->uploadPath); [$destinationDir, $destinationName] = Uri\split($destination); /** @var Directory $destinationParent */ @@ -297,7 +326,7 @@ private function checkPrerequisites(bool $checkUploadMetadata = true): void { */ private function getUploadStorage(string $targetPath): array { $storage = $this->uploadFolder->getStorage(); - $targetFile = $this->getUploadFile($targetPath); + $targetFile = $this->resolveChunkWriteTargetFile($targetPath); return [$storage, $targetFile->getInternalPath()]; } @@ -320,7 +349,7 @@ public function prepareUpload($path): void { } private function completeChunkedWrite(string $targetAbsolutePath): void { - $uploadFile = $this->getUploadFile($this->uploadPath)->getNode(); + $uploadFile = $this->resolveChunkWriteTargetFile($this->uploadPath)->getNode(); [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); $rootFolder = \OCP\Server::get(IRootFolder::class); From 2f23b14df80f10f4c154ae7f1befca90c8d1ddac Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 2 Jun 2026 11:38:03 -0400 Subject: [PATCH 2/8] refactor(dav): resolve everything needed for chunk writing in one pass Introduces a helper that replaces getUploadFile + getUploadStorage and resolves everything needed for chunk writing in one pass. Signed-off-by: Josh --- apps/dav/lib/Upload/ChunkingV2Plugin.php | 146 +++++++++++++++-------- 1 file changed, 96 insertions(+), 50 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index 604fa2e56e4a0..8f58cf2225b0b 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -89,28 +89,49 @@ protected function beforeGet(RequestInterface $request) { } /** - * @param string $path - * @param bool $createIfNotExists - * @return FutureFile|UploadFile|ICollection|INode + * Resolve the file and storage/path tuple used for chunk writes. + * + * Direct writes are only used when the destination file already exists on the + * same storage as the upload folder. Otherwise the staging file in the upload + * folder is used and finalized with copy/move afterwards. + * + * FIXME: Verify 'file' return type; old code had probably overly broad `@return FutureFile|UploadFile|ICollection|INode` + * + * @return array{ + * file: File, + * storage: \OCP\Files\Storage\IStorage, + * storagePath: string, + * isDirect: bool + * } */ - private function resolveChunkWriteTargetFile(string $path, bool $createIfNotExists = false) { + private function resolveChunkWriteTarget(string $path, bool $createIfNotExists = false): array { if (!$this->uploadFolder instanceof UploadFolder) { throw new \LogicException('Upload folder not initialized'); } - $directTarget = $this->tryGetDirectTargetFile($path); + $directTarget = $this->tryResolveDirectChunkWriteTarget($path); if ($directTarget !== null) { return $directTarget; } - return $this->getOrCreateStagingFile($createIfNotExists); + return $this->resolveStagingChunkWriteTarget($createIfNotExists); } - private function tryGetDirectTargetFile(string $path): ?File { + /** + * @return array{ + * file: File, + * storage: \OCP\Files\Storage\IStorage, + * storagePath: string, + * isDirect: bool + * }|null + */ + private function tryResolveDirectChunkWriteTarget(string $path): ?array { + $uploadStorage = $this->uploadFolder->getStorage(); + try { $actualFile = $this->server->tree->getNodeForPath($path); } catch (NotFound $e) { - // No target file exists yet; fall back to staging via the upload folder. + // No target file exists yet; fall back to staging. return null; } @@ -118,24 +139,35 @@ private function tryGetDirectTargetFile(string $path): ?File { return null; } - $uploadStorage = $this->uploadFolder->getStorage(); $actualNode = $actualFile->getNode(); - - // Direct chunked writes are only safe when both nodes share the same - // storage backend; otherwise use the staging file in the upload folder. - // - // This is a conservative optimization: we only bypass the staging file when - // the destination already lives on the same storage as the upload folder. - // Broader direct-write support may be possible, but only if getUploadStorage() - // is changed to return the correct storage/path for that target backend. + // This is a conservative optimization: only bypass staging when the + // destination already lives on the same storage as the upload folder. + // Broader direct-write support may be possible, but only if this + // resolver is extended to return the correct storage/path pair for + // that backend. if ($uploadStorage->getId() !== $actualNode->getStorage()->getId()) { return null; } - return $actualFile; + return [ + 'file' => $actualFile, + 'storage' => $uploadStorage, + 'storagePath' => $actualFile->getInternalPath(), + 'isDirect' => true, + ]; } - private function getOrCreateStagingFile(bool $createIfNotExists) { + /** + * @return array{ + * file: File, + * storage: \OCP\Files\Storage\IStorage, + * storagePath: string, + * isDirect: bool + * } + */ + private function resolveStagingChunkWriteTarget(bool $createIfNotExists): array { + $uploadStorage = $this->uploadFolder->getStorage(); + if ($createIfNotExists && !$this->uploadFolder->childExists(self::TEMP_TARGET)) { $this->uploadFolder->createFile(self::TEMP_TARGET); } @@ -143,7 +175,14 @@ private function getOrCreateStagingFile(bool $createIfNotExists) { // Use file in the upload directory that will be copied or moved afterwards /** @var UploadFile $uploadFile */ $uploadFile = $this->uploadFolder->getChild(self::TEMP_TARGET); - return $uploadFile->getFile(); + $file = $uploadFile->getFile(); + + return [ + 'file' => $file, + 'storage' => $uploadStorage, + 'storagePath' => $file->getInternalPath(), + 'isDirect' => false, + ]; } public function afterMkcol(RequestInterface $request, ResponseInterface $response): bool { @@ -155,15 +194,14 @@ public function afterMkcol(RequestInterface $request, ResponseInterface $respons } $this->uploadPath = $this->server->calculateUri($this->server->httpRequest->getHeader(self::DESTINATION_HEADER)); - $targetFile = $this->resolveChunkWriteTargetFile($this->uploadPath, true); - [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); + $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath, true); - $this->uploadId = $storage->startChunkedWrite($storagePath); + $this->uploadId = $resolvedTarget['storage']->startChunkedWrite($resolvedTarget['storagePath']); $this->cache->set($this->uploadFolder->getName(), [ self::UPLOAD_ID => $this->uploadId, self::UPLOAD_TARGET_PATH => $this->uploadPath, - self::UPLOAD_TARGET_ID => $targetFile->getId(), + self::UPLOAD_TARGET_ID => $resolvedTarget['file']->getId(), ], 86400); $response->setStatus(Http::STATUS_CREATED); @@ -178,7 +216,11 @@ public function beforePut(RequestInterface $request, ResponseInterface $response return true; } - [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); + $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath); + $storage = $resolvedTarget['storage']; + $storagePath = $resolvedTarget['storagePath']; + $uploadFile = $resolvedTarget['file']; + $isDirect = $resolvedTarget['isDirect']; $chunkName = basename($request->getPath()); $partId = is_numeric($chunkName) ? (int)$chunkName : -1; @@ -186,11 +228,10 @@ public function beforePut(RequestInterface $request, ResponseInterface $response throw new BadRequest('Invalid chunk name, must be numeric between 1 and 10000'); } - $uploadFile = $this->resolveChunkWriteTargetFile($this->uploadPath); $tempTargetFile = null; $additionalSize = (int)$request->getHeader('Content-Length'); - if ($this->uploadFolder->childExists(self::TEMP_TARGET) && $this->uploadPath) { + if (!$isDirect && $this->uploadPath) { /** @var UploadFile $tempTargetFile */ $tempTargetFile = $this->uploadFolder->getChild(self::TEMP_TARGET); [$destinationDir, $destinationName] = Uri\split($this->uploadPath); @@ -222,28 +263,39 @@ public function beforeMove($sourcePath, $destination): bool { } catch (StorageInvalidException|BadRequest|NotFound|PreconditionFailed $e) { return true; } - [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); - $targetFile = $this->resolveChunkWriteTargetFile($this->uploadPath); + $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath); + $storage = $resolvedTarget['storage']; + $storagePath = $resolvedTarget['storagePath']; + $targetFile = $resolvedTarget['file']; [$destinationDir, $destinationName] = Uri\split($destination); /** @var Directory $destinationParent */ $destinationParent = $this->server->tree->getNodeForPath($destinationDir); $destinationExists = $destinationParent->childExists($destinationName); - // allow sync clients to send the modification and creation time along in a header $updateFileInfo = []; - if ($this->server->httpRequest->getHeader('X-OC-MTime') !== null) { - $updateFileInfo['mtime'] = $this->sanitizeMtime($this->server->httpRequest->getHeader('X-OC-MTime')); + + // allow sync clients to send the modification time along in a header + $headerMTime = $this->server->httpRequest->getHeader('X-OC-MTime'); + if ($headerMTime !== null) { + $updateFileInfo['mtime'] = $this->sanitizeMtime($headerMTime); $this->server->httpResponse->setHeader('X-OC-MTime', 'accepted'); } - if ($this->server->httpRequest->getHeader('X-OC-CTime') !== null) { - $updateFileInfo['creation_time'] = $this->sanitizeMtime($this->server->httpRequest->getHeader('X-OC-CTime')); + + // allow sync clients to send the creation time along in a header + $headerCTime = $this->server->httpRequest->getHeader('X-OC-CTime'); + if ($headerCTime !== null) { + $updateFileInfo['creation_time'] = $this->sanitizeMtime($headerCTime); $this->server->httpResponse->setHeader('X-OC-CTime', 'accepted'); } + $updateFileInfo['mimetype'] = \OCP\Server::get(IMimeTypeDetector::class)->detectPath($destinationName); - if ($storage->instanceOfStorage(ObjectStoreStorage::class) && $storage->getObjectStore() instanceof IObjectStoreMultiPartUpload) { + if ( + $storage->instanceOfStorage(ObjectStoreStorage::class) + && $storage->getObjectStore() instanceof IObjectStoreMultiPartUpload + ) { /** @var ObjectStoreStorage $storage */ /** @var IObjectStoreMultiPartUpload $objectStore */ $objectStore = $storage->getObjectStore(); @@ -288,8 +340,9 @@ public function beforeDelete(RequestInterface $request, ResponseInterface $respo return true; } - [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); - $storage->cancelChunkedWrite($storagePath, $this->uploadId); + $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath); + $resolvedTarget['storage']->cancelChunkedWrite($resolvedTarget['storagePath'], $this->uploadId); + return true; } @@ -321,15 +374,6 @@ private function checkPrerequisites(bool $checkUploadMetadata = true): void { } } - /** - * @return array [IStorage, string] - */ - private function getUploadStorage(string $targetPath): array { - $storage = $this->uploadFolder->getStorage(); - $targetFile = $this->resolveChunkWriteTargetFile($targetPath); - return [$storage, $targetFile->getInternalPath()]; - } - protected function sanitizeMtime(string $mtimeFromRequest): int { if (!is_numeric($mtimeFromRequest)) { throw new InvalidArgumentException('X-OC-MTime header must be an integer (unix timestamp).'); @@ -349,8 +393,11 @@ public function prepareUpload($path): void { } private function completeChunkedWrite(string $targetAbsolutePath): void { - $uploadFile = $this->resolveChunkWriteTargetFile($this->uploadPath)->getNode(); - [$storage, $storagePath] = $this->getUploadStorage($this->uploadPath); + $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath); + $uploadFile = $resolvedTarget['file']->getNode(); + $storage = $resolvedTarget['storage']; + $storagePath = $resolvedTarget['storagePath']; + $isDirect = $resolvedTarget['isDirect']; $rootFolder = \OCP\Server::get(IRootFolder::class); $exists = $rootFolder->nodeExists($targetAbsolutePath); @@ -368,8 +415,7 @@ private function completeChunkedWrite(string $targetAbsolutePath): void { // If the file was not uploaded to the user storage directly we need to copy/move it try { - $uploadFileAbsolutePath = $uploadFile->getFileInfo()->getPath(); - if ($uploadFileAbsolutePath !== $targetAbsolutePath) { + if (!$isDirect) { $uploadFile = $rootFolder->get($uploadFile->getFileInfo()->getPath()); if ($exists) { $uploadFile->copy($targetAbsolutePath); From 0f82e96155d949f440c289abc2e5f5be0586b500 Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 2 Jun 2026 12:05:49 -0400 Subject: [PATCH 3/8] chore(dav): switch to keyed destructuring in ChunkingV2Plugin Signed-off-by: Josh --- apps/dav/lib/Upload/ChunkingV2Plugin.php | 31 +++++++++++++----------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index 8f58cf2225b0b..22b247cbcfc72 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -194,14 +194,19 @@ public function afterMkcol(RequestInterface $request, ResponseInterface $respons } $this->uploadPath = $this->server->calculateUri($this->server->httpRequest->getHeader(self::DESTINATION_HEADER)); - $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath, true); - $this->uploadId = $resolvedTarget['storage']->startChunkedWrite($resolvedTarget['storagePath']); + [ + 'file' => $uploadFile, + 'storage' => $storage, + 'storagePath' => $storagePath, + ] = $this->resolveChunkWriteTarget($this->uploadPath, true); + + $this->uploadId = $storage->startChunkedWrite($storagePath); $this->cache->set($this->uploadFolder->getName(), [ self::UPLOAD_ID => $this->uploadId, self::UPLOAD_TARGET_PATH => $this->uploadPath, - self::UPLOAD_TARGET_ID => $resolvedTarget['file']->getId(), + self::UPLOAD_TARGET_ID => $uploadFile->getId(), ], 86400); $response->setStatus(Http::STATUS_CREATED); @@ -216,11 +221,12 @@ public function beforePut(RequestInterface $request, ResponseInterface $response return true; } - $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath); - $storage = $resolvedTarget['storage']; - $storagePath = $resolvedTarget['storagePath']; - $uploadFile = $resolvedTarget['file']; - $isDirect = $resolvedTarget['isDirect']; + [ + 'file' => $uploadFile, + 'storage' => $storage, + 'storagePath' => $storagePath, + 'isDirect' => $isDirect, + ] = $this->resolveChunkWriteTarget($this->uploadPath); $chunkName = basename($request->getPath()); $partId = is_numeric($chunkName) ? (int)$chunkName : -1; @@ -264,10 +270,7 @@ public function beforeMove($sourcePath, $destination): bool { return true; } - $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath); - $storage = $resolvedTarget['storage']; - $storagePath = $resolvedTarget['storagePath']; - $targetFile = $resolvedTarget['file']; + ['file' => $targetFile, 'storage' => $storage] = $this->resolveChunkWriteTarget($this->uploadPath); [$destinationDir, $destinationName] = Uri\split($destination); /** @var Directory $destinationParent */ @@ -340,8 +343,8 @@ public function beforeDelete(RequestInterface $request, ResponseInterface $respo return true; } - $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath); - $resolvedTarget['storage']->cancelChunkedWrite($resolvedTarget['storagePath'], $this->uploadId); + ['storage' => $storage, 'storagePath' => $storagePath] = $this->resolveChunkWriteTarget($this->uploadPath); + $storage->cancelChunkedWrite($storagePath, $this->uploadId); return true; } From a5e6fef758c3d152ad441bdb406cefc17e17cbd3 Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 2 Jun 2026 13:01:16 -0400 Subject: [PATCH 4/8] docs(dav): clarifying comments and code formatting in ChunkingV2 Signed-off-by: Josh --- apps/dav/lib/Upload/ChunkingV2Plugin.php | 133 +++++++++++++++++++---- 1 file changed, 112 insertions(+), 21 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index 22b247cbcfc72..efda0ee845061 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -41,26 +41,27 @@ use Sabre\HTTP\ResponseInterface; use Sabre\Uri; +/** + * Sabre server plugin that coordinates chunked WebDAV uploads and finalization. + * + * Uploads are written either directly to the target file or to a staging file + * in the upload folder, depending on the target storage backend. + */ class ChunkingV2Plugin extends ServerPlugin { - /** @var Server */ - private $server; - /** @var UploadFolder */ - private $uploadFolder; - /** @var ICache */ - private $cache; - + private Server $server; + private UploadFolder $uploadFolder; + private ICache $cache; private ?string $uploadId = null; private ?string $uploadPath = null; private const TEMP_TARGET = '.target'; + private const DESTINATION_HEADER = 'Destination'; public const CACHE_KEY = 'chunking-v2'; public const UPLOAD_TARGET_PATH = 'upload-target-path'; public const UPLOAD_TARGET_ID = 'upload-target-id'; public const UPLOAD_ID = 'upload-id'; - private const DESTINATION_HEADER = 'Destination'; - public function __construct(ICacheFactory $cacheFactory) { $this->cache = $cacheFactory->createDistributed(self::CACHE_KEY); } @@ -92,8 +93,8 @@ protected function beforeGet(RequestInterface $request) { * Resolve the file and storage/path tuple used for chunk writes. * * Direct writes are only used when the destination file already exists on the - * same storage as the upload folder. Otherwise the staging file in the upload - * folder is used and finalized with copy/move afterwards. + * same storage as the upload folder. Otherwise chunks are written to the + * staging file in the upload folder and finalized with copy/move afterwards. * * FIXME: Verify 'file' return type; old code had probably overly broad `@return FutureFile|UploadFile|ICollection|INode` * @@ -118,6 +119,11 @@ private function resolveChunkWriteTarget(string $path, bool $createIfNotExists = } /** + * Try to resolve the destination as a direct chunk-write target. + * + * This only succeeds when the destination exists as a file and is backed by + * the same storage as the upload folder. + * * @return array{ * file: File, * storage: \OCP\Files\Storage\IStorage, @@ -158,6 +164,11 @@ private function tryResolveDirectChunkWriteTarget(string $path): ?array { } /** + * Resolve the staging file used when direct chunk writes are not possible. + * + * The staging file lives in the upload folder and is copied or moved to the + * final destination after the chunked write has been completed. + * * @return array{ * file: File, * storage: \OCP\Files\Storage\IStorage, @@ -185,6 +196,12 @@ private function resolveStagingChunkWriteTarget(bool $createIfNotExists): array ]; } + /** + * Initialize chunked-upload metadata after the upload collection is created. + * + * This resolves the write target, starts the backend chunked-write session, + * and stores the upload metadata in distributed cache for subsequent requests. + */ public function afterMkcol(RequestInterface $request, ResponseInterface $response): bool { try { $this->prepareUpload($request->getPath()); @@ -193,7 +210,8 @@ public function afterMkcol(RequestInterface $request, ResponseInterface $respons return true; } - $this->uploadPath = $this->server->calculateUri($this->server->httpRequest->getHeader(self::DESTINATION_HEADER)); + $headerDestination = $this->server->httpRequest->getHeader(self::DESTINATION_HEADER); + $this->uploadPath = $this->server->calculateUri($headerDestination); [ 'file' => $uploadFile, @@ -213,6 +231,15 @@ public function afterMkcol(RequestInterface $request, ResponseInterface $respons return true; } + /** + * Store one uploaded chunk in the active chunked-write session. + * + * For staged uploads, this also updates the temporary target file metadata so + * size and propagation stay consistent across chunk requests. + * + * @throws BadRequest + * @throws InsufficientStorage + */ public function beforePut(RequestInterface $request, ResponseInterface $response): bool { try { $this->prepareUpload(dirname($request->getPath())); @@ -235,14 +262,17 @@ public function beforePut(RequestInterface $request, ResponseInterface $response } $tempTargetFile = null; - $additionalSize = (int)$request->getHeader('Content-Length'); + + // For staged uploads, track the temporary target size separately so we can + // reject requests that would exceed the destination's available free space. if (!$isDirect && $this->uploadPath) { /** @var UploadFile $tempTargetFile */ $tempTargetFile = $this->uploadFolder->getChild(self::TEMP_TARGET); [$destinationDir, $destinationName] = Uri\split($this->uploadPath); /** @var Directory $destinationParent */ $destinationParent = $this->server->tree->getNodeForPath($destinationDir); + $free = $destinationParent->getNode()->getFreeSpace(); $newSize = $tempTargetFile->getSize() + $additionalSize; if ($free >= 0 && ($tempTargetFile->getSize() > $free || $newSize > $free)) { @@ -251,17 +281,37 @@ public function beforePut(RequestInterface $request, ResponseInterface $response } $stream = $request->getBodyAsStream(); - $storage->putChunkedWritePart($storagePath, $this->uploadId, (string)$partId, $stream, $additionalSize); + $storage->putChunkedWritePart( + $storagePath, + $this->uploadId, + (string)$partId, + $stream, + $additionalSize + ); + + $storage->getCache()->update( + $uploadFile->getId(), + ['size' => $uploadFile->getSize() + $additionalSize] + ); - $storage->getCache()->update($uploadFile->getId(), ['size' => $uploadFile->getSize() + $additionalSize]); if ($tempTargetFile) { - $storage->getPropagator()->propagateChange($tempTargetFile->getInternalPath(), time(), $additionalSize); + $storage->getPropagator()->propagateChange( + $tempTargetFile->getInternalPath(), + time(), + $additionalSize + ); } $response->setStatus(201); return false; } + /** + * Finalize a chunked upload when the upload collection is moved to its target. + * + * Completes the backend chunked write, applies file metadata, and removes the + * temporary upload folder when the source is still a future file. + */ public function beforeMove($sourcePath, $destination): bool { try { $this->prepareUpload(dirname($sourcePath)); @@ -295,6 +345,8 @@ public function beforeMove($sourcePath, $destination): bool { $updateFileInfo['mimetype'] = \OCP\Server::get(IMimeTypeDetector::class)->detectPath($destinationName); + // For multipart object-store uploads, determine the final size from uploaded + // parts before completing the write so free-space checks reflect the real size. if ( $storage->instanceOfStorage(ObjectStoreStorage::class) && $storage->getObjectStore() instanceof IObjectStoreMultiPartUpload @@ -350,6 +402,11 @@ public function beforeDelete(RequestInterface $request, ResponseInterface $respo } /** + * Validate that the current request can use chunking v2. + * + * This checks cache availability, upload-folder capabilities, destination + * headers, and cached upload metadata from previous requests. + * * @throws BadRequest * @throws PreconditionFailed * @throws StorageInvalidException @@ -357,35 +414,56 @@ public function beforeDelete(RequestInterface $request, ResponseInterface $respo private function checkPrerequisites(bool $checkUploadMetadata = true): void { $distributedCacheConfig = \OCP\Server::get(IConfig::class)->getSystemValue('memcache.distributed', null); - if ($distributedCacheConfig === null || (!$this->cache instanceof Redis && !$this->cache instanceof Memcached)) { + if ( + $distributedCacheConfig === null + || (!$this->cache instanceof Redis && !$this->cache instanceof Memcached) + ) { throw new BadRequest('Skipping chunking v2 since no proper distributed cache is available'); } - if (!$this->uploadFolder instanceof UploadFolder || empty($this->server->httpRequest->getHeader(self::DESTINATION_HEADER))) { + + 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)) { throw new StorageInvalidException('Storage does not support chunked file writing'); } - if ($this->uploadFolder->getStorage()->instanceOfStorage(ObjectStoreStorage::class) && !$this->uploadFolder->getStorage()->getObjectStore() instanceof IObjectStoreMultiPartUpload) { + + if ( + $this->uploadFolder->getStorage()->instanceOfStorage(ObjectStoreStorage::class) + && !$this->uploadFolder->getStorage()->getObjectStore() instanceof IObjectStoreMultiPartUpload + ) { throw new StorageInvalidException('Storage does not support multi part uploads'); } if ($checkUploadMetadata) { if ($this->uploadId === null || $this->uploadPath === null) { - throw new PreconditionFailed('Missing metadata for chunked upload. The distributed cache does not hold the information of previous requests.'); + throw new PreconditionFailed( + 'Missing metadata for chunked upload. The distributed cache does not hold the information of previous requests.' + ); } } } protected function sanitizeMtime(string $mtimeFromRequest): int { if (!is_numeric($mtimeFromRequest)) { - throw new InvalidArgumentException('X-OC-MTime header must be an integer (unix timestamp).'); + throw new InvalidArgumentException('X-OC-MTime/CTime header must be an integer (unix timestamp).'); } return (int)$mtimeFromRequest; } /** + * Load the upload folder and cached metadata for the current upload request. + * + * Populates the upload folder as well as the cached upload ID and destination + * path needed by subsequent chunk, move, and delete requests. + * + * FIXME: make private? + * * @throws NotFound */ public function prepareUpload($path): void { @@ -395,6 +473,13 @@ public function prepareUpload($path): void { $this->uploadPath = $uploadMetadata[self::UPLOAD_TARGET_PATH] ?? null; } + /** + * Complete the backend chunked write and materialize the final target file. + * + * When the upload was staged in the upload folder, the completed file is + * copied or moved to the requested destination after the backend write + * finishes. + */ private function completeChunkedWrite(string $targetAbsolutePath): void { $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath); $uploadFile = $resolvedTarget['file']->getNode(); @@ -433,6 +518,9 @@ private function completeChunkedWrite(string $targetAbsolutePath): void { } } + /** + * Emit filesystem hooks before the completed upload is materialized. + */ private function emitPreHooks(string $target, bool $exists): void { $hookPath = $this->getHookPath($target); if (!$exists) { @@ -449,6 +537,9 @@ private function emitPreHooks(string $target, bool $exists): void { ]); } + /** + * Emit filesystem hooks after the completed upload has been materialized. + */ private function emitPostHooks(string $target, bool $exists): void { $hookPath = $this->getHookPath($target); if (!$exists) { From 6524ea7d57396925cf0eca697cb9aeb84ba9b5a3 Mon Sep 17 00:00:00 2001 From: Josh Date: Tue, 2 Jun 2026 13:23:39 -0400 Subject: [PATCH 5/8] docs(dav): clarify ChunkingV2Plugin::afterMkcol() scope is more than metadata Signed-off-by: Josh --- apps/dav/lib/Upload/ChunkingV2Plugin.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index efda0ee845061..afdfc1f442a2f 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -197,10 +197,10 @@ private function resolveStagingChunkWriteTarget(bool $createIfNotExists): array } /** - * Initialize chunked-upload metadata after the upload collection is created. + * Start a chunked upload after the upload collection is created. * - * This resolves the write target, starts the backend chunked-write session, - * and stores the upload metadata in distributed cache for subsequent requests. + * Resolves the upload target, starts the backend chunked-write session, + * and stores upload metadata in distributed cache for subsequent chunk requests. */ public function afterMkcol(RequestInterface $request, ResponseInterface $response): bool { try { From f19b09dab03b7425fb45fe3995219ec68761c126 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 3 Jun 2026 08:23:58 -0400 Subject: [PATCH 6/8] refactor(dav): avoid using $uploadFile naming in ChunkingV2 unless an actual UploadFile Signed-off-by: Josh --- apps/dav/lib/Upload/ChunkingV2Plugin.php | 43 +++++++++++++----------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index afdfc1f442a2f..9943b41d8ab1f 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -186,10 +186,10 @@ private function resolveStagingChunkWriteTarget(bool $createIfNotExists): array // Use file in the upload directory that will be copied or moved afterwards /** @var UploadFile $uploadFile */ $uploadFile = $this->uploadFolder->getChild(self::TEMP_TARGET); - $file = $uploadFile->getFile(); + $targetFile = $uploadFile->getFile(); return [ - 'file' => $file, + 'file' => $targetFile, 'storage' => $uploadStorage, 'storagePath' => $file->getInternalPath(), 'isDirect' => false, @@ -214,7 +214,7 @@ public function afterMkcol(RequestInterface $request, ResponseInterface $respons $this->uploadPath = $this->server->calculateUri($headerDestination); [ - 'file' => $uploadFile, + 'file' => $targetFile, 'storage' => $storage, 'storagePath' => $storagePath, ] = $this->resolveChunkWriteTarget($this->uploadPath, true); @@ -224,7 +224,7 @@ public function afterMkcol(RequestInterface $request, ResponseInterface $respons $this->cache->set($this->uploadFolder->getName(), [ self::UPLOAD_ID => $this->uploadId, self::UPLOAD_TARGET_PATH => $this->uploadPath, - self::UPLOAD_TARGET_ID => $uploadFile->getId(), + self::UPLOAD_TARGET_ID => $targetFile->getId(), ], 86400); $response->setStatus(Http::STATUS_CREATED); @@ -249,7 +249,7 @@ public function beforePut(RequestInterface $request, ResponseInterface $response } [ - 'file' => $uploadFile, + 'file' => $targetFile, 'storage' => $storage, 'storagePath' => $storagePath, 'isDirect' => $isDirect, @@ -290,8 +290,8 @@ public function beforePut(RequestInterface $request, ResponseInterface $response ); $storage->getCache()->update( - $uploadFile->getId(), - ['size' => $uploadFile->getSize() + $additionalSize] + $targetFile->getId(), + ['size' => $targetFile->getSize() + $additionalSize] ); if ($tempTargetFile) { @@ -481,39 +481,42 @@ public function prepareUpload($path): void { * finishes. */ private function completeChunkedWrite(string $targetAbsolutePath): void { - $resolvedTarget = $this->resolveChunkWriteTarget($this->uploadPath); - $uploadFile = $resolvedTarget['file']->getNode(); - $storage = $resolvedTarget['storage']; - $storagePath = $resolvedTarget['storagePath']; - $isDirect = $resolvedTarget['isDirect']; + [ + 'file' => $targetFile, + 'storage' => $storage, + 'storagePath' => $storagePath, + 'isDirect' => $isDirect, + ] = $this->resolveChunkWriteTarget($this->uploadPath); + + $targetNode = $targetFile->getNode(); $rootFolder = \OCP\Server::get(IRootFolder::class); $exists = $rootFolder->nodeExists($targetAbsolutePath); - $uploadFile->lock(ILockingProvider::LOCK_SHARED); + $targetNode->lock(ILockingProvider::LOCK_SHARED); $this->emitPreHooks($targetAbsolutePath, $exists); try { - $uploadFile->changeLock(ILockingProvider::LOCK_EXCLUSIVE); + $targetNode->changeLock(ILockingProvider::LOCK_EXCLUSIVE); $storage->completeChunkedWrite($storagePath, $this->uploadId); - $uploadFile->changeLock(ILockingProvider::LOCK_SHARED); + $targetNode->changeLock(ILockingProvider::LOCK_SHARED); } catch (Exception $e) { - $uploadFile->unlock(ILockingProvider::LOCK_EXCLUSIVE); + $targetNode->unlock(ILockingProvider::LOCK_EXCLUSIVE); throw $e; } // If the file was not uploaded to the user storage directly we need to copy/move it try { if (!$isDirect) { - $uploadFile = $rootFolder->get($uploadFile->getFileInfo()->getPath()); + $stagingNode = $rootFolder->get($targetNode->getFileInfo()->getPath()); if ($exists) { - $uploadFile->copy($targetAbsolutePath); + $stagingNode->copy($targetAbsolutePath); } else { - $uploadFile->move($targetAbsolutePath); + $stagingNode->move($targetAbsolutePath); } } $this->emitPostHooks($targetAbsolutePath, $exists); } catch (Exception $e) { - $uploadFile->unlock(ILockingProvider::LOCK_SHARED); + $targetNode->unlock(ILockingProvider::LOCK_SHARED); throw $e; } } From 2dce7cd0e45e4c54ff51f0a88dbcd57e0dfca9c7 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 3 Jun 2026 08:42:07 -0400 Subject: [PATCH 7/8] chore(dav): fixup ChunkingV2Plugin variable mismatch Signed-off-by: Josh --- apps/dav/lib/Upload/ChunkingV2Plugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index 9943b41d8ab1f..e693a1e623ed1 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -191,7 +191,7 @@ private function resolveStagingChunkWriteTarget(bool $createIfNotExists): array return [ 'file' => $targetFile, 'storage' => $uploadStorage, - 'storagePath' => $file->getInternalPath(), + 'storagePath' => $targetFile->getInternalPath(), 'isDirect' => false, ]; } From 6c7b4c4714263124b1355e4bcdea73ab3db12d98 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 3 Jun 2026 19:21:12 -0400 Subject: [PATCH 8/8] fix(dav): ignore non-upload requests in ChunkingV2Plugin The refactor added typing for $this->uploadHome, which brought out this existing behavior. Previously invalid node types could be assigned, then eventually filtered out later. This adds an explicit earlier check to ignore non-upload requests. Signed-off-by: Josh --- apps/dav/lib/Upload/ChunkingV2Plugin.php | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index e693a1e623ed1..54d73d0169c70 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -242,7 +242,9 @@ public function afterMkcol(RequestInterface $request, ResponseInterface $respons */ public function beforePut(RequestInterface $request, ResponseInterface $response): bool { try { - $this->prepareUpload(dirname($request->getPath())); + if ($this->prepareUpload(dirname($request->getPath())) === null) { + return true; + } $this->checkPrerequisites(); } catch (StorageInvalidException|BadRequest|NotFound $e) { return true; @@ -314,7 +316,9 @@ public function beforePut(RequestInterface $request, ResponseInterface $response */ public function beforeMove($sourcePath, $destination): bool { try { - $this->prepareUpload(dirname($sourcePath)); + if ($this->prepareUpload(dirname($sourcePath)) === null) { + return true; + } $this->checkPrerequisites(); } catch (StorageInvalidException|BadRequest|NotFound|PreconditionFailed $e) { return true; @@ -389,7 +393,9 @@ public function beforeMove($sourcePath, $destination): bool { public function beforeDelete(RequestInterface $request, ResponseInterface $response) { try { - $this->prepareUpload(dirname($request->getPath())); + if ($this->prepareUpload(dirname($request->getPath()))) { + return true; + } $this->checkPrerequisites(); } catch (StorageInvalidException|BadRequest|NotFound $e) { return true; @@ -466,11 +472,18 @@ protected function sanitizeMtime(string $mtimeFromRequest): int { * * @throws NotFound */ - public function prepareUpload($path): void { - $this->uploadFolder = $this->server->tree->getNodeForPath($path); + public function prepareUpload($path): ?UploadFolder { + $node = $this->server->tree->getNodeForPath($path); + if (!$node instanceof UploadFolder) { + return null; + } + + $this->uploadFolder = $node; $uploadMetadata = $this->cache->get($this->uploadFolder->getName()); $this->uploadId = $uploadMetadata[self::UPLOAD_ID] ?? null; $this->uploadPath = $uploadMetadata[self::UPLOAD_TARGET_PATH] ?? null; + + return $node; } /**