From c3ac545f72225329f552a38fc2d9f52cb3aa1180 Mon Sep 17 00:00:00 2001 From: Josh Date: Mon, 10 Nov 2025 13:21:16 -0500 Subject: [PATCH] refactor(upload): enhance internal validation, error handling, child existence check, and docs in UploadHome Signed-off-by: Josh --- apps/dav/lib/Upload/UploadHome.php | 89 +++++++++++++++++++++++++----- 1 file changed, 74 insertions(+), 15 deletions(-) diff --git a/apps/dav/lib/Upload/UploadHome.php b/apps/dav/lib/Upload/UploadHome.php index 4042f1c4101..cdcd5ba2f75 100644 --- a/apps/dav/lib/Upload/UploadHome.php +++ b/apps/dav/lib/Upload/UploadHome.php @@ -1,7 +1,7 @@ Internal: /principals/users/alice + * External: /remote.php/dav/uploads/token_xyz/ --> Internal: /principals/shares/token_xyz + */ class UploadHome implements ICollection { private string $uid; private ?Folder $uploadFolder = null; + /** + * @throws NotFound If the principal is unsupported/invalid or the share token is not found or invalid + * @throws Forbidden If the session is unauthenticated + */ public function __construct( private readonly array $principalInfo, private readonly CleanupService $cleanupService, @@ -28,15 +44,38 @@ class UploadHome implements ICollection { private readonly \OCP\Share\IManager $shareManager, ) { [$prefix, $name] = \Sabre\Uri\split($principalInfo['uri']); - if ($prefix === 'principals/shares') { - $this->uid = $this->shareManager->getShareByToken($name)->getShareOwner(); - } else { + + // Validate required components of the principal + if (empty($name) || !in_array($prefix, ['principals/users', 'principals/shares'], true)) { + throw new NotFound('Invalid or unsupported principal URI'); + } + + // Handle user principal + if ($prefix === 'principals/users') { + // Authenticated user principal: $name provided in URI is ignored, but must have a valid logged-in session. $user = $this->userSession->getUser(); if (!$user) { throw new Forbidden('Not logged in'); } - + // TODO: (non-security just general robustness/expected behavior): Compare the URI $name to $user/$uid? $this->uid = $user->getUID(); + return; + } + + // Handle share principal + if ($prefix === 'principals/shares') { + // Share principal: use share token ($name) from URI to determine owner UID. + try { + $share = $this->shareManager->getShareByToken($name); + $owner = $share->getShareOwner(); + if (empty($owner) || !is_string($owner)) { + throw new NotFound('Share token not found or invalid'); + } + $this->uid = $owner; + return; + } catch (ShareNotFound $e) { + throw new NotFound('Share token not found or invalid'); + } } } @@ -45,7 +84,7 @@ class UploadHome implements ICollection { } public function createDirectory($name) { - $this->impl()->createDirectory($name); + $this->getUploadDirectory()->createDirectory($name); // Add a cleanup job $this->cleanupService->addJob($this->uid, $name); @@ -53,10 +92,10 @@ class UploadHome implements ICollection { public function getChild($name): UploadFolder { return new UploadFolder( - $this->impl()->getChild($name), + $this->getUploadDirectory()->getChild($name), $this->cleanupService, $this->getStorage(), - $this->uid, + $this->uid ); } @@ -66,17 +105,21 @@ class UploadHome implements ICollection { $node, $this->cleanupService, $this->getStorage(), - $this->uid, + $this->uid ); - }, $this->impl()->getChildren()); + }, $this->getUploadDirectory()->getChildren()); } public function childExists($name): bool { - return !is_null($this->getChild($name)); + try { + return $this->getUploadDirectory()->childExists($name); + } catch (\Throwable $e) { + return false; + } } public function delete() { - $this->impl()->delete(); + $this->getUploadDirectory()->delete(); } public function getName() { @@ -89,9 +132,17 @@ class UploadHome implements ICollection { } public function getLastModified() { - return $this->impl()->getLastModified(); + return $this->getUploadDirectory()->getLastModified(); } + /** + * Returns the Nextcloud Folder object representing the effective user's uploads folder, creating it if missing. + * Lazily fetches and caches the result for repeated use. + * + * @return Folder The Nextcloud Folder instance representing the user's uploads directory. + * @throws \Exception If the uploads path exists as a file instead of a folder. + * + */ private function getUploadFolder(): Folder { if ($this->uploadFolder === null) { $path = '/' . $this->uid . '/uploads'; @@ -108,13 +159,21 @@ class UploadHome implements ICollection { return $this->uploadFolder; } - private function impl(): Directory { + /** + * Returns the SabreDAV Directory node representing the effective user's uploads folder, creating it if missing. + * + * @return OCA\DAV\Connector\Sabre\Directory + */ + private function getUploadDirectory(): Directory { $folder = $this->getUploadFolder(); $view = new View($folder->getPath()); return new Directory($view, $folder); } - private function getStorage() { + /** + * Returns the Nextcloud storage backend for the effective user's uploads folder. + */ + private function getStorage(): \OCP\Files\Storage\IStorage { return $this->getUploadFolder()->getStorage(); } }