diff --git a/lib/Service/FilesWidgetService.php b/lib/Service/FilesWidgetService.php index ea6c679f0..6e8446b75 100644 --- a/lib/Service/FilesWidgetService.php +++ b/lib/Service/FilesWidgetService.php @@ -104,19 +104,12 @@ public function __construct( }//end __construct() /** - * List the contents of a folder for the given viewer. - * - * Resolves the configured folder (fileId preferred over path), - * applies the optional MIME filter, sorts the result, and slices - * out the requested page. Folders the viewer cannot read are - * skipped silently. + * List the visible contents of the placement's configured folder (or a + * sub-path of it) for the viewing user (REQ-FLS-003, REQ-FLS-004). * * @param string $userId Viewing user's UID. - * @param array $config Placement config — - * supports `fileId`, - * `folderPath`, - * `mimeTypeFilter`, `sortBy`, - * `sortDescending`. + * @param array $config Placement config (`fileId`, `folderPath`, + * `mimeTypeFilter`, `sortBy`, `sortDescending`). * @param array $page Optional `currentSubPath` (sub-path relative to the * configured folder, set when the user navigates into * a subfolder; null for the folder itself), `limit` @@ -134,45 +127,61 @@ public function __construct( */ public function getContentsForPlacement(string $userId, array $config, array $page=[]): array { $page += ['currentSubPath' => null, 'limit' => self::DEFAULT_LIMIT, 'cursor' => '']; - $currentSubPath = $page['currentSubPath']; $limit = (int)$page['limit']; $cursor = (string)$page['cursor']; $rootFolder = $this->resolveConfiguredFolder(userId: $userId, config: $config); - - $target = $this->descendInto( - root: $rootFolder, - subPath: ($currentSubPath ?? '/') - ); + $target = $this->descendInto(root: $rootFolder, subPath: ($page['currentSubPath'] ?? '/')); if ($target->isReadable() === false) { throw new NoAccessException(); } - $children = $target->getDirectoryListing(); + $items = $this->listVisibleItems(folder: $target, config: $config); + return $this->paginate(items: $items, limit: $this->clampLimit(limit: $limit), cursor: $cursor); + }//end getContentsForPlacement() + + /** + * The folder's children the viewer may read and the placement's MIME + * filter lets through (folders always pass), as metadata, sorted per + * the placement config. + * + * @param Folder $folder The readable folder being listed. + * @param array $config Placement config. + * + * @return list> The sorted item metadata. + */ + private function listVisibleItems(Folder $folder, array $config): array { + $children = $folder->getDirectoryListing(); $mimeFilter = $this->normaliseMimeFilter(filter: ($config['mimeTypeFilter'] ?? [])); $items = []; foreach ($children as $child) { - if ($child->isReadable() === false) { - continue; - } - - if ($child instanceof File && $this->matchesMimeFilter(node: $child, filter: $mimeFilter) === false) { - continue; + if ($this->isListed(node: $child, mimeFilter: $mimeFilter) === true) { + $items[] = $this->buildFileMetadata(node: $child); } - - $items[] = $this->buildFileMetadata(node: $child); } - $items = $this->sortItems( + return $this->sortItems( items: $items, sortBy: ($config['sortBy'] ?? 'name'), sortDescending: (bool)($config['sortDescending'] ?? false) ); + }//end listVisibleItems() - return $this->paginate(items: $items, limit: $this->clampLimit(limit: $limit), cursor: $cursor); - }//end getContentsForPlacement() + /** + * Whether a child is listed: the viewer can read it, and it is a + * folder or a file that passes the MIME filter. + * + * @param Node $node The child node. + * @param list $mimeFilter Normalised filter list. + * + * @return bool True when the child is listed. + */ + private function isListed(Node $node, array $mimeFilter): bool { + return $node->isReadable() === true + && (($node instanceof File) === false || $this->matchesMimeFilter(node: $node, filter: $mimeFilter) === true); + }//end isListed() /** * Upload one or more files into the configured folder (or a @@ -206,53 +215,114 @@ public function uploadFiles( } $root = $this->resolveConfiguredFolder(userId: $userId, config: $config); - $target = $this->descendInto(root: $root, subPath: ($currentSubPath ?? '/')); + $target = $this->writableFolder(root: $root, subPath: $currentSubPath); + return $this->storeUploads(target: $this->uploadTarget(folder: $target, config: $config), uploadedFiles: $uploadedFiles); + }//end uploadFiles() + + /** + * The sub-path folder an upload writes into, refused when the viewer + * may not write to it. + * + * @param Folder $root The configured root folder. + * @param string|null $subPath Sub-path inside the configured folder. + * + * @return Folder The writable folder. + * + * @throws FolderNotFoundException When the sub-path does not resolve. + * @throws NoAccessException When the folder is not writable. + */ + private function writableFolder(Folder $root, ?string $subPath): Folder { + $target = $this->descendInto(root: $root, subPath: ($subPath ?? '/')); if ($target->isUpdateable() === false) { throw new NoAccessException(); } + return $target; + }//end writableFolder() + + /** + * The upload destination with the placement's MIME filter. + * + * @param Folder $folder The writable destination folder. + * @param array $config Placement config. + * + * @return FilesWidgetUploadTarget The destination. + */ + private function uploadTarget(Folder $folder, array $config): FilesWidgetUploadTarget { // M3: normalise the placement's MIME type filter so we can gate // uploads against it (same filter already enforced on reads). $mimeFilter = $this->normaliseMimeFilter( filter: ($config['mimeTypeFilter'] ?? []) ); + return new FilesWidgetUploadTarget(folder: $folder, mimeFilter: $mimeFilter); + }//end uploadTarget() + + /** + * Store each uploaded entry, collecting the stored files' metadata and + * the per-entry errors, in upload order. + * + * @param FilesWidgetUploadTarget $target The destination. + * @param array $uploadedFiles Normalised `$_FILES` entries. + * + * @return array{uploaded: list>, errors: list>} + */ + private function storeUploads(FilesWidgetUploadTarget $target, array $uploadedFiles): array { $uploaded = []; $errors = []; foreach ($uploadedFiles as $entry) { - $name = (string)$entry['name']; - - $rejection = $this->rejectUpload(entry: $entry, mimeFilter: $mimeFilter); - if ($rejection !== null) { - $errors[] = [ - 'name' => $name, - 'error' => $rejection, - ]; - continue; + $outcome = $this->storeUpload(target: $target, entry: $entry); + if (is_string($outcome) === true) { + $errors[] = ['name' => (string)$entry['name'], 'error' => $outcome]; + } else { + $uploaded[] = $outcome; } + } - $safeName = $this->resolveAvailableName(folder: $target, desired: $name); - try { - $file = $this->streamToNewFile( - target: $target, - tmp: (string)$entry['tmp_name'], - safeName: $safeName - ); - $uploaded[] = $this->buildFileMetadata(node: $file); - } catch (Throwable $e) { - $errors[] = [ - 'name' => $name, - 'error' => 'storage_failed', - ]; - }//end try - }//end foreach + return ['uploaded' => $uploaded, 'errors' => $errors]; + }//end storeUploads() - return [ - 'uploaded' => $uploaded, - 'errors' => $errors, - ]; - }//end uploadFiles() + /** + * Screen and store one uploaded entry under a collision-free name. + * + * @param FilesWidgetUploadTarget $target The destination. + * @param array $entry One normalised `$_FILES` entry. + * + * @return array|string The stored file's metadata, or the + * client-facing error code. + */ + private function storeUpload(FilesWidgetUploadTarget $target, array $entry): array|string { + $rejection = $this->rejectUpload(entry: $entry, mimeFilter: $target->mimeFilter); + if ($rejection !== null) { + return $rejection; + } + + return $this->writeUpload(target: $target, entry: $entry); + }//end storeUpload() + + /** + * Write a screened upload under a collision-free name. + * + * @param FilesWidgetUploadTarget $target The destination. + * @param array $entry One screened `$_FILES` entry. + * + * @return array|string The stored file's metadata, or + * `storage_failed`. + */ + private function writeUpload(FilesWidgetUploadTarget $target, array $entry): array|string { + $safeName = $this->resolveAvailableName(folder: $target->folder, desired: (string)$entry['name']); + try { + $file = $this->streamToNewFile( + target: $target->folder, + tmp: (string)$entry['tmp_name'], + safeName: $safeName + ); + return $this->buildFileMetadata(node: $file); + } catch (Throwable $e) { + return 'storage_failed'; + } + }//end writeUpload() /** * Screen one uploaded entry before it is written to storage. @@ -272,27 +342,45 @@ public function uploadFiles( * @spec openspec/specs/files-widget/spec.md */ private function rejectUpload(array $entry, array $mimeFilter): ?string { - $name = (string)$entry['name']; - $tmp = (string)$entry['tmp_name']; - $error = (int)$entry['error']; - $size = (int)$entry['size']; - - if ($name === '' || $error !== UPLOAD_ERR_OK || $tmp === '') { - return 'upload_failed'; - } - - // M3: assert the temp file came through a real PHP upload to - // prevent local-file-inclusion via a crafted tmp_name. - if (is_uploaded_file(filename: $tmp) === false) { + if ($this->isGenuineUpload(entry: $entry) === false) { return 'upload_failed'; } // M3: reject files that exceed the hard size cap. - if ($size > self::MAX_UPLOAD_BYTES) { + if ((int)$entry['size'] > self::MAX_UPLOAD_BYTES) { return 'file_too_large'; } - // M3: honour the placement MIME type filter on writes. + return $this->rejectUploadType(tmp: (string)$entry['tmp_name'], mimeFilter: $mimeFilter); + }//end rejectUpload() + + /** + * Whether an entry is a complete upload: named, without a transport + * error, and with a temp file that came through a real PHP upload + * (M3: prevents local-file inclusion via a crafted `tmp_name`). + * + * @param array $entry One normalised `$_FILES` entry. + * + * @return bool True when the entry may be screened further. + */ + private function isGenuineUpload(array $entry): bool { + $tmp = (string)$entry['tmp_name']; + + return (string)$entry['name'] !== '' + && (int)$entry['error'] === UPLOAD_ERR_OK + && $tmp !== '' + && is_uploaded_file(filename: $tmp) === true; + }//end isGenuineUpload() + + /** + * M3: honour the placement MIME type filter on writes. + * + * @param string $tmp Path of the PHP temporary upload file. + * @param array $mimeFilter Normalised MIME filter; `[]` means allow all. + * + * @return string|null `file_type_not_allowed`, or null when allowed. + */ + private function rejectUploadType(string $tmp, array $mimeFilter): ?string { if ($mimeFilter === []) { return null; } @@ -303,7 +391,7 @@ private function rejectUpload(array $entry, array $mimeFilter): ?string { } return null; - }//end rejectUpload() + }//end rejectUploadType() /** * Stream a temporary upload into a new file under `$target`. @@ -364,10 +452,29 @@ public function deleteFile(string $userId, array $config, int $fileId): array { } $root = $this->resolveConfiguredFolder(userId: $userId, config: $config); + $node = $this->userNodeById(userId: $userId, fileId: $fileId); + $this->assertDeletableWithin(root: $root, node: $node); + $this->deleteNode(node: $node); + + return [ + 'status' => 'success', + 'fileId' => $fileId, + ]; + }//end deleteFile() - // Find the file inside the configured folder by id. The id - // search is scoped to the user folder so we do not have to - // worry about cross-storage leaks. + /** + * Find a node by id inside the viewer's own Files area. The id + * search is scoped to the user folder so we do not have to worry + * about cross-storage leaks. + * + * @param string $userId Viewing user's UID. + * @param integer $fileId The file ID. + * + * @return Node The first match. + * + * @throws FolderNotFoundException When the lookup fails or finds nothing. + */ + private function userNodeById(string $userId, int $fileId): Node { try { $userFolder = $this->rootFolder->getUserFolder(userId: $userId); $matches = $userFolder->getById(id: $fileId); @@ -379,8 +486,21 @@ public function deleteFile(string $userId, array $config, int $fileId): array { throw new FolderNotFoundException(); } - $node = $matches[0]; + return $matches[0]; + }//end userNodeById() + /** + * Refuse a delete of a node outside the configured root folder or one + * the viewer may not delete. + * + * @param Folder $root The configured root folder. + * @param Node $node The node to delete. + * + * @return void + * + * @throws NoAccessException When the delete is not allowed. + */ + private function assertDeletableWithin(Folder $root, Node $node): void { // Defence-in-depth: the resolved node must live inside the // configured root folder. Otherwise a viewer with both // placements could pass an arbitrary fileId from a different @@ -392,18 +512,24 @@ public function deleteFile(string $userId, array $config, int $fileId): array { if ($node->isDeletable() === false) { throw new NoAccessException(); } + }//end assertDeletableWithin() + /** + * Delete a node, answering any storage failure as no access. + * + * @param Node $node The node to delete. + * + * @return void + * + * @throws NoAccessException When the storage refuses the delete. + */ + private function deleteNode(Node $node): void { try { $node->delete(); } catch (Throwable $e) { throw new NoAccessException(); } - - return [ - 'status' => 'success', - 'fileId' => $fileId, - ]; - }//end deleteFile() + }//end deleteNode() /** * Build the normalised JSON-friendly file metadata blob the @@ -480,33 +606,63 @@ public function resolveConfiguredFolder(string $userId, array $config): Folder { throw new FolderNotFoundException(); } + $node = $this->configuredNode(userFolder: $userFolder, config: $config); + if ($node === null) { + throw new FolderNotFoundException(); + } + + if ($node->isReadable() === false) { + throw new NoAccessException(); + } + + return $node; + }//end resolveConfiguredFolder() + + /** + * The folder a placement config names: by `fileId` first, else by + * `folderPath`, else — for an unconfigured placement — the home folder. + * + * @param Folder $userFolder The viewer's home folder. + * @param array $config Placement config. + * + * @return Folder|null The folder, or null when the configured one is gone. + */ + private function configuredNode(Folder $userFolder, array $config): ?Folder { $fileId = $this->extractFileId(config: $config); $folderPath = (string)($config['folderPath'] ?? ''); $node = $this->resolveByFileId(userFolder: $userFolder, fileId: $fileId); - if ($node === null && $folderPath !== '') { - $node = $this->resolveByPath(userFolder: $userFolder, folderPath: $folderPath); + if ($node !== null) { + return $node; + } + + if ($folderPath !== '') { + return $this->resolveByPath(userFolder: $userFolder, folderPath: $folderPath); } // Unconfigured placement (neither a fileId nor a folderPath set): // default to the user's home folder rather than erroring, so a // freshly-added or seeded files widget shows something usable until a - // folder is chosen. A configured-but-missing folder still falls - // through to the throw below. - if ($node === null && $fileId === null && $folderPath === '') { - $node = $userFolder; - } - - if ($node === null) { - throw new FolderNotFoundException(); - } + // folder is chosen. A configured-but-missing folder still yields null. + return $this->homeWhenUnconfigured(userFolder: $userFolder, fileId: $fileId); + }//end configuredNode() - if ($node->isReadable() === false) { - throw new NoAccessException(); + /** + * The home folder for a placement without a `fileId`; null for one + * whose `fileId` no longer resolves. + * + * @param Folder $userFolder The viewer's home folder. + * @param integer|null $fileId Configured file id, or null. + * + * @return Folder|null The home folder, or null. + */ + private function homeWhenUnconfigured(Folder $userFolder, ?int $fileId): ?Folder { + if ($fileId === null) { + return $userFolder; } - return $node; - }//end resolveConfiguredFolder() + return null; + }//end homeWhenUnconfigured() /** * Resolve the placement folder by its stable `fileId`. @@ -526,13 +682,23 @@ private function resolveByFileId(Folder $userFolder, ?int $fileId): ?Folder { return null; } - $matches = $userFolder->getById(id: $fileId); + return $this->firstFolder(matches: $userFolder->getById(id: $fileId)); + }//end resolveByFileId() + + /** + * The first match when it is a folder. + * + * @param array $matches The id lookup's matches. + * + * @return Folder|null The folder, or null. + */ + private function firstFolder(array $matches): ?Folder { if (count($matches) > 0 && $matches[0] instanceof Folder) { return $matches[0]; } return null; - }//end resolveByFileId() + }//end firstFolder() /** * Resolve the placement folder by its configured path. @@ -593,24 +759,34 @@ private function descendInto(Folder $root, string $subPath): Folder { throw new FolderNotFoundException(); } - try { - $node = $root->get(path: ltrim(string: $clean, characters: '/')); - } catch (NotFoundException $e) { - throw new FolderNotFoundException(); - } catch (NotPermittedException $e) { + $node = $this->nodeBelow(root: $root, path: ltrim(string: $clean, characters: '/')); + if (($node instanceof Folder) === false || $node->isReadable() === false) { throw new FolderNotFoundException(); } - if (($node instanceof Folder) === false) { - throw new FolderNotFoundException(); - } + return $node; + }//end descendInto() - if ($node->isReadable() === false) { + /** + * The node at a relative path below a folder; missing and not + * permitted paths are both answered as not found. + * + * @param Folder $root The folder to look below. + * @param string $path The relative path. + * + * @return Node The node. + * + * @throws FolderNotFoundException When the path is missing or not permitted. + */ + private function nodeBelow(Folder $root, string $path): Node { + try { + return $root->get(path: $path); + } catch (NotFoundException $e) { + throw new FolderNotFoundException(); + } catch (NotPermittedException $e) { throw new FolderNotFoundException(); } - - return $node; - }//end descendInto() + }//end nodeBelow() /** * Coerce the placement's `fileId` config value into a positive @@ -630,15 +806,28 @@ private function extractFileId(array $config): ?int { return $value; } - if (is_string($value) === true && ctype_digit($value) === true) { - $coerced = (int)$value; - if ($coerced > 0) { - return $coerced; - } + return $this->positiveIntFromDigits(value: $value); + }//end extractFileId() + + /** + * A positive integer from a digit-only string; null for anything else. + * + * @param mixed $value The candidate value. + * + * @return integer|null The integer, or null. + */ + private function positiveIntFromDigits(mixed $value): ?int { + if (is_string($value) === false || ctype_digit($value) === false) { + return null; + } + + $coerced = (int)$value; + if ($coerced > 0) { + return $coerced; } return null; - }//end extractFileId() + }//end positiveIntFromDigits() /** * Normalise the MIME filter array — drops non-strings and empty @@ -683,19 +872,7 @@ private function matchesMimeFilter(Node $node, array $filter): bool { return true; } - $mime = strtolower(string: (string)$node->getMimetype()); - foreach ($filter as $pattern) { - if (str_ends_with(haystack: $pattern, needle: '/*') === true) { - $prefix = substr(string: $pattern, offset: 0, length: -1); - if (str_starts_with(haystack: $mime, needle: $prefix) === true) { - return true; - } - } elseif ($pattern === $mime) { - return true; - } - } - - return false; + return $this->mimeMatchesFilter(mime: strtolower(string: (string)$node->getMimetype()), filter: $filter); }//end matchesMimeFilter() /** @@ -728,8 +905,8 @@ private function detectMimeType(string $path): string { * Test a raw MIME type string against a normalised filter list. * * Accepts both exact matches (`image/png`) and wildcard prefixes - * (`image/*`) — mirrors the read-path `matchesMimeFilter` but - * operates on a raw string rather than a Node. + * (`image/*`) — shared by the read-path `matchesMimeFilter` and the + * upload screening. * * @param string $mime The detected MIME type. * @param list $filter Normalised filter list. @@ -742,12 +919,7 @@ private function mimeMatchesFilter(string $mime, array $filter): bool { } foreach ($filter as $pattern) { - if (str_ends_with(haystack: $pattern, needle: '/*') === true) { - $prefix = substr(string: $pattern, offset: 0, length: -1); - if (str_starts_with(haystack: $mime, needle: $prefix) === true) { - return true; - } - } elseif ($pattern === $mime) { + if (self::mimeMatchesPattern(mime: $mime, pattern: $pattern) === true) { return true; } } @@ -755,6 +927,23 @@ private function mimeMatchesFilter(string $mime, array $filter): bool { return false; }//end mimeMatchesFilter() + /** + * Test a MIME type against one filter entry: an exact type, or a + * `type/*` wildcard prefix. + * + * @param string $mime The MIME type. + * @param string $pattern One normalised filter entry. + * + * @return bool True on a match. + */ + private static function mimeMatchesPattern(string $mime, string $pattern): bool { + if (str_ends_with(haystack: $pattern, needle: '/*') === true) { + return str_starts_with(haystack: $mime, needle: substr(string: $pattern, offset: 0, length: -1)); + } + + return $pattern === $mime; + }//end mimeMatchesPattern() + /** * Sort an item list according to the placement config. * @@ -765,29 +954,7 @@ private function mimeMatchesFilter(string $mime, array $filter): bool { * @return list> */ private function sortItems(array $items, string $sortBy, bool $sortDescending): array { - $allowed = ['name', 'modified', 'size', 'type']; - if (in_array(needle: $sortBy, haystack: $allowed, strict: true) === false) { - $sortBy = 'name'; - } - - usort( - array: $items, - callback: function (array $a, array $b) use ($sortBy): int { - if ($sortBy === 'modified') { - return strcmp(string1: (string)($a['modifiedAt'] ?? ''), string2: (string)($b['modifiedAt'] ?? '')); - } - - if ($sortBy === 'size') { - return ((int)($a['size'] ?? 0)) <=> ((int)($b['size'] ?? 0)); - } - - if ($sortBy === 'type') { - return strcmp(string1: (string)($a['mimeType'] ?? ''), string2: (string)($b['mimeType'] ?? '')); - } - - return strnatcasecmp(string1: (string)($a['name'] ?? ''), string2: (string)($b['name'] ?? '')); - } - ); + usort(array: $items, callback: self::itemComparator(sortBy: $sortBy)); if ($sortDescending === true) { $items = array_reverse(array: $items); @@ -796,6 +963,23 @@ private function sortItems(array $items, string $sortBy, bool $sortDescending): return array_values(array: $items); }//end sortItems() + /** + * The comparator for a sort field: `modified`, `size` or `type`, and + * natural case-insensitive name order for `name` or any other value. + * + * @param string $sortBy Sort field. + * + * @return callable(array, array): int The comparator. + */ + private static function itemComparator(string $sortBy): callable { + return match ($sortBy) { + 'modified' => static fn (array $a, array $b): int => strcmp(string1: (string)($a['modifiedAt'] ?? ''), string2: (string)($b['modifiedAt'] ?? '')), + 'size' => static fn (array $a, array $b): int => ((int)($a['size'] ?? 0)) <=> ((int)($b['size'] ?? 0)), + 'type' => static fn (array $a, array $b): int => strcmp(string1: (string)($a['mimeType'] ?? ''), string2: (string)($b['mimeType'] ?? '')), + default => static fn (array $a, array $b): int => strnatcasecmp(string1: (string)($a['name'] ?? ''), string2: (string)($b['name'] ?? '')), + }; + }//end itemComparator() + /** * Slice the sorted item list into a single page using an opaque * offset cursor (REQ-FLS-003 design D3). @@ -869,32 +1053,68 @@ private function formatTimestamp(int $timestamp): string { * @return string */ private function resolveAvailableName(Folder $folder, string $desired): string { - $sanitised = str_replace(search: ['/', '\\', "\0"], replace: '_', subject: $desired); - if ($sanitised === '' || $sanitised === '.' || $sanitised === '..') { - $sanitised = 'upload'; - } - + $sanitised = self::sanitiseUploadName(desired: $desired); if ($folder->nodeExists(path: $sanitised) === false) { return $sanitised; } - $extPos = strrpos(haystack: $sanitised, needle: '.'); - $base = $sanitised; - $ext = ''; - if ($extPos !== false && $extPos !== 0) { - $base = substr(string: $sanitised, offset: 0, length: $extPos); - $ext = substr(string: $sanitised, offset: $extPos); + return $this->firstFreeNumberedName(folder: $folder, name: $sanitised); + }//end resolveAvailableName() + + /** + * Replace path separators and NUL with `_`; an empty, `.` or `..` + * result becomes `upload`. + * + * @param string $desired Desired leaf name. + * + * @return string The safe leaf name. + */ + private static function sanitiseUploadName(string $desired): string { + $sanitised = str_replace(search: ['/', '\\', "\0"], replace: '_', subject: $desired); + if ($sanitised === '' || $sanitised === '.' || $sanitised === '..') { + return 'upload'; } + return $sanitised; + }//end sanitiseUploadName() + + /** + * The first of `name (1)` … `name (999)` that is free in the folder, + * else a `name ()` variant. + * + * @param Folder $folder Target folder. + * @param string $name The taken, sanitised name. + * + * @return string The free name. + */ + private function firstFreeNumberedName(Folder $folder, string $name): string { for ($i = 1; $i <= 999; $i++) { - $candidate = $base . ' (' . $i . ')' . $ext; + $candidate = self::numberedName(name: $name, number: (string)$i); if ($folder->nodeExists(path: $candidate) === false) { return $candidate; } } - return $base . ' (' . uniqid() . ')' . $ext; - }//end resolveAvailableName() + return self::numberedName(name: $name, number: uniqid()); + }//end firstFreeNumberedName() + + /** + * Insert ` ()` before a name's extension; a leading-dot name + * or one without a dot gets it at the end. + * + * @param string $name The sanitised name. + * @param string $number The number to insert. + * + * @return string The numbered name. + */ + private static function numberedName(string $name, string $number): string { + $extPos = strrpos(haystack: $name, needle: '.'); + if ($extPos !== false && $extPos !== 0) { + return substr(string: $name, offset: 0, length: $extPos) . ' (' . $number . ')' . substr(string: $name, offset: $extPos); + } + + return $name . ' (' . $number . ')'; + }//end numberedName() /** * Walk parents of `$candidate` and return true when `$parent` is @@ -911,21 +1131,28 @@ private function isDescendantOf(Folder $parent, Node $candidate): bool { return true; } - $cursor = $candidate; - for ($depth = 0; $depth < 32; $depth++) { - try { - $next = $cursor->getParent(); - } catch (Throwable $e) { - return false; - } + return $this->hasAncestorWithId(node: $candidate, ancestorId: $parentId); + }//end isDescendantOf() - if ($next->getId() === $parentId) { - return true; + /** + * Walk at most 32 parents up from a node looking for an id; the walk + * stops at the user-folder root or on a parent that cannot be read. + * + * @param Node $node The node to start from. + * @param mixed $ancestorId The id to look for. + * + * @return bool True when a parent within reach has the id. + */ + private function hasAncestorWithId(Node $node, mixed $ancestorId): bool { + $cursor = $node; + for ($depth = 0; $depth < 32; $depth++) { + $next = $this->parentOf(node: $cursor); + if ($next === null || $next->getId() === $ancestorId) { + return $next !== null; } - // Reached the user-folder root without hitting `$parent`. - $nextPath = $next->getPath(); - if ($nextPath === '/' || $nextPath === '') { + // Reached the user-folder root without hitting the ancestor. + if (in_array(needle: $next->getPath(), haystack: ['/', ''], strict: true) === true) { return false; } @@ -933,5 +1160,20 @@ private function isDescendantOf(Folder $parent, Node $candidate): bool { } return false; - }//end isDescendantOf() + }//end hasAncestorWithId() + + /** + * A node's parent, null when it cannot be read. + * + * @param Node $node The node. + * + * @return Node|null The parent, or null. + */ + private function parentOf(Node $node): ?Node { + try { + return $node->getParent(); + } catch (Throwable $e) { + return null; + } + }//end parentOf() }//end class diff --git a/lib/Service/FilesWidgetUploadTarget.php b/lib/Service/FilesWidgetUploadTarget.php new file mode 100644 index 000000000..f564ad024 --- /dev/null +++ b/lib/Service/FilesWidgetUploadTarget.php @@ -0,0 +1,46 @@ + + * @copyright 2026 Conduction b.v. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @version GIT:auto + * @link https://conduction.nl + * + * SPDX-FileCopyrightText: 2024 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * @spec openspec/specs/files-widget/spec.md + */ + +declare(strict_types=1); + +namespace OCA\LaunchPad\Service; + +use OCP\Files\Folder; + +/** + * The folder a files-widget upload writes into and its MIME filter. + * + * @spec openspec/specs/files-widget/spec.md + */ +final class FilesWidgetUploadTarget { + /** + * Constructor. + * + * @param Folder $folder The writable destination folder. + * @param list $mimeFilter Normalised MIME filter; `[]` means allow all. + */ + public function __construct( + public readonly Folder $folder, + public readonly array $mimeFilter, + ) { + }//end __construct() +}//end class diff --git a/tests/Unit/Service/FilesWidgetAccessGoldenTest.php b/tests/Unit/Service/FilesWidgetAccessGoldenTest.php new file mode 100644 index 000000000..5f1c933ba --- /dev/null +++ b/tests/Unit/Service/FilesWidgetAccessGoldenTest.php @@ -0,0 +1,505 @@ + + * @copyright 2026 Conduction b.v. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * SPDX-FileCopyrightText: 2026 LaunchPad Contributors + * SPDX-License-Identifier: EUPL-1.2 + */ + +declare(strict_types=1); + +namespace OCA\LaunchPad\Service { + /** + * Test seam: answers true for the temp files a test registered as + * genuine uploads, and defers to PHP for every other path. + * + * @param string $filename The temp path. + * + * @return bool + */ + function is_uploaded_file(string $filename): bool { + return in_array($filename, $GLOBALS['launchpadTestUploads'] ?? [], true) || \is_uploaded_file($filename); + } +} + +namespace Unit\Service { + use OCA\LaunchPad\Exception\FolderNotFoundException; + use OCA\LaunchPad\Exception\NoAccessException; + use OCA\LaunchPad\Service\FilesWidgetService; + use OCP\Files\File; + use OCP\Files\Folder; + use OCP\Files\IRootFolder; + use OCP\Files\Node; + use OCP\Files\NotFoundException; + use OCP\Files\NotPermittedException; + use OCP\IPreview; + use OCP\IURLGenerator; + use PHPUnit\Framework\TestCase; + use RuntimeException; + + class FilesWidgetAccessGoldenTest extends TestCase { + private IRootFolder $rootFolder; + private Folder $userFolder; + private FilesWidgetService $service; + + /** @var list */ + private array $tmpFiles = []; + + protected function setUp(): void { + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->userFolder = $this->folder(id: 1, path: '/alice/files'); + $this->rootFolder->method('getUserFolder')->willReturn($this->userFolder); + $this->service = new FilesWidgetService( + $this->rootFolder, + $this->createMock(IURLGenerator::class), + $this->createMock(IPreview::class), + ); + } + + protected function tearDown(): void { + foreach ($this->tmpFiles as $tmp) { + @unlink($tmp); + } + + $GLOBALS['launchpadTestUploads'] = []; + } + + private function folder(int $id, string $path = '/x', bool $readable = true, bool $updateable = true): Folder { + $folder = $this->createMock(Folder::class); + $folder->method('getId')->willReturn($id); + $folder->method('getPath')->willReturn($path); + $folder->method('isReadable')->willReturn($readable); + $folder->method('isUpdateable')->willReturn($updateable); + $folder->method('getName')->willReturn('f' . $id); + $folder->method('getMimetype')->willReturn('httpd/unix-directory'); + + return $folder; + } + + private function file(int $id, string $name, string $mime = 'text/plain', int $size = 1, int $mtime = 0): File { + $file = $this->createMock(File::class); + $file->method('getId')->willReturn($id); + $file->method('getName')->willReturn($name); + $file->method('getMimetype')->willReturn($mime); + $file->method('getSize')->willReturn($size); + $file->method('getMTime')->willReturn($mtime); + $file->method('isReadable')->willReturn(true); + + return $file; + } + + private function names(array $result): array { + return array_column($result['items'], 'name'); + } + + /** + * fileId configs and whether they resolve by id. + * + * @return array + */ + public static function fileIds(): array { + return [ + 'int' => [12, true], + 'digit string' => ['12', true], + 'zero' => [0, false], + 'zero string' => ['0', false], + 'negative' => [-12, false], + 'text' => ['12a', false], + 'float' => [12.0, false], + ]; + } + + /** + * @dataProvider fileIds + */ + public function testFileIdResolution(mixed $fileId, bool $byId): void { + $configured = $this->folder(id: 12); + $configured->method('getDirectoryListing')->willReturn([$this->file(5, 'byid.txt')]); + $this->userFolder->method('getById')->willReturnCallback( + static fn (int $id): array => $id === 12 ? [$configured] : [] + ); + $this->userFolder->method('getDirectoryListing')->willReturn([$this->file(6, 'home.txt')]); + + $result = $this->service->getContentsForPlacement('alice', ['fileId' => $fileId]); + + $this->assertSame([$byId ? 'byid.txt' : 'home.txt'], $this->names($result)); + } + + public function testFileIdOfAFileFallsBackToThePath(): void { + $docs = $this->folder(id: 30); + $docs->method('getDirectoryListing')->willReturn([$this->file(7, 'docs.txt')]); + $this->userFolder->method('getById')->willReturn([$this->file(12, 'x.txt')]); + $this->userFolder->expects($this->once())->method('get')->with('/Docs')->willReturn($docs); + + $result = $this->service->getContentsForPlacement('alice', ['fileId' => 12, 'folderPath' => '/Docs/']); + + $this->assertSame(['docs.txt'], $this->names($result)); + } + + public function testUnresolvedFileIdWithoutPathIsNotFound(): void { + $this->userFolder->method('getById')->willReturn([]); + $this->expectException(FolderNotFoundException::class); + + $this->service->getContentsForPlacement('alice', ['fileId' => 99]); + } + + /** + * Paths that do not resolve to a folder. + * + * @return array + */ + public static function unresolvedPaths(): array { + return ['missing' => ['missing'], 'file' => ['file']]; + } + + /** + * @dataProvider unresolvedPaths + */ + public function testUnresolvedPathIsNotFound(string $case): void { + if ($case === 'missing') { + $this->userFolder->method('get')->willThrowException(new NotFoundException('x')); + } else { + $this->userFolder->method('get')->willReturn($this->file(3, 'a')); + } + + $this->expectException(FolderNotFoundException::class); + + $this->service->getContentsForPlacement('alice', ['folderPath' => 'Docs']); + } + + public function testSlashPathIsTheHomeFolder(): void { + $this->userFolder->expects($this->never())->method('get'); + $this->userFolder->method('getDirectoryListing')->willReturn([]); + + $this->assertSame(['items' => [], 'nextCursor' => null], $this->service->getContentsForPlacement('alice', ['folderPath' => '//'])); + } + + public function testUserFolderFailureIsNotFound(): void { + $rootFolder = $this->createMock(IRootFolder::class); + $rootFolder->method('getUserFolder')->willThrowException(new RuntimeException('x')); + $service = new FilesWidgetService($rootFolder, $this->createMock(IURLGenerator::class), $this->createMock(IPreview::class)); + $this->expectException(FolderNotFoundException::class); + + $service->resolveConfiguredFolder('alice', []); + } + + public function testUnreadableConfiguredFolderIsNoAccess(): void { + $this->userFolder->method('get')->willReturn($this->folder(id: 30, readable: false)); + $this->expectException(NoAccessException::class); + + $this->service->resolveConfiguredFolder('alice', ['folderPath' => 'Docs']); + } + + /** + * Sub-paths that are refused as not found. + * + * @return array + */ + public static function refusedSubPaths(): array { + return [ + 'traversal' => ['a/../b'], + 'not found' => ['nf'], + 'not permitted' => ['np'], + 'a file' => ['file'], + 'unreadable' => ['locked'], + ]; + } + + /** + * @dataProvider refusedSubPaths + */ + public function testRefusedSubPathIsNotFound(string $subPath): void { + $this->userFolder->method('get')->willReturnCallback(function (string $path) use ($subPath): Node { + $this->assertNotSame('a/../b', $subPath, 'a traversal must not be looked up'); + return match ($path) { + 'nf' => throw new NotFoundException('x'), + 'np' => throw new NotPermittedException('x'), + 'file' => $this->file(3, 'a'), + default => $this->folder(id: 4, readable: false), + }; + }); + $this->expectException(FolderNotFoundException::class); + + $this->service->getContentsForPlacement('alice', [], ['currentSubPath' => $subPath]); + } + + public function testOtherLookupErrorsPropagate(): void { + $this->userFolder->method('get')->willThrowException(new RuntimeException('boom')); + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('boom'); + + $this->service->getContentsForPlacement('alice', [], ['currentSubPath' => 'x']); + } + + public function testSubPathIsLookedUpRelativeToTheRoot(): void { + $sub = $this->folder(id: 8); + $sub->method('getDirectoryListing')->willReturn([$this->file(1, 'in-sub.txt')]); + $this->userFolder->expects($this->once())->method('get')->with('a/b')->willReturn($sub); + + $this->assertSame(['in-sub.txt'], $this->names($this->service->getContentsForPlacement('alice', [], ['currentSubPath' => '/a/b/']))); + } + + public function testUnreadableTargetIsNoAccess(): void { + $this->userFolder->method('get')->willReturn($this->folder(id: 8)); + $sub = $this->folder(id: 9, readable: true); + $root = $this->createMock(Folder::class); + $root->method('isReadable')->willReturnOnConsecutiveCalls(true, false); + $this->userFolder->method('getById')->willReturn([$root]); + $this->expectException(NoAccessException::class); + + $this->service->getContentsForPlacement('alice', ['fileId' => 4]); + } + + /** + * Sort configs and the order they give. + * + * @return array,1:list}> + */ + public static function sorts(): array { + return [ + 'default name' => [[], ['a10.txt', 'B2.png', 'b3.txt', 'f50']], + 'unknown' => [['sortBy' => 'owner'], ['a10.txt', 'B2.png', 'b3.txt', 'f50']], + 'name desc' => [['sortDescending' => true], ['f50', 'b3.txt', 'B2.png', 'a10.txt']], + 'modified' => [['sortBy' => 'modified'], ['f50', 'b3.txt', 'a10.txt', 'B2.png']], + 'size' => [['sortBy' => 'size'], ['b3.txt', 'B2.png', 'f50', 'a10.txt']], + 'type' => [['sortBy' => 'type'], ['f50', 'B2.png', 'b3.txt', 'a10.txt']], + 'mime filter' => [['mimeTypeFilter' => [' IMAGE/* ', 7, '']], ['B2.png', 'f50']], + 'exact mime' => [['mimeTypeFilter' => ['text/plain']], ['a10.txt', 'b3.txt', 'f50']], + ]; + } + + /** + * @dataProvider sorts + */ + public function testListingOrderAndFilter(array $config, array $expected): void { + $folder = $this->folder(id: 50); + $folder->method('getSize')->willReturn(5); + $folder->method('getMTime')->willReturn(10); + $hidden = $this->createMock(File::class); + $hidden->method('isReadable')->willReturn(false); + $this->userFolder->method('getDirectoryListing')->willReturn([ + $this->file(1, 'b3.txt', 'text/plain', 1, 200), + $hidden, + $this->file(2, 'B2.png', 'image/png', 3, 400), + $folder, + $this->file(3, 'a10.txt', 'text/plain', 9, 300), + ]); + + $this->assertSame($expected, $this->names($this->service->getContentsForPlacement('alice', $config))); + } + + /** + * Page windows and the names / cursor they give over five items. + * + * @return array,1:list,2:string|null}> + */ + public static function pages(): array { + return [ + 'first two' => [['limit' => 2], ['a', 'b'], '2'], + 'from two' => [['limit' => 2, 'cursor' => '2'], ['c', 'd'], '4'], + 'last' => [['limit' => 2, 'cursor' => '4'], ['e'], null], + 'bad cursor' => [['limit' => 2, 'cursor' => '-1'], ['a', 'b'], '2'], + 'zero limit' => [['limit' => 0], ['a', 'b', 'c', 'd', 'e'], null], + 'past end' => [['cursor' => '9'], [], null], + ]; + } + + /** + * @dataProvider pages + */ + public function testPaging(array $page, array $names, ?string $cursor): void { + $this->userFolder->method('getDirectoryListing')->willReturn(array_map( + fn (string $name): File => $this->file(1, $name), + ['e', 'd', 'c', 'b', 'a'] + )); + + $result = $this->service->getContentsForPlacement('alice', [], $page); + + $this->assertSame([$names, $cursor], [$this->names($result), $result['nextCursor']]); + } + + private function upload(string $name, string $contents = 'hello', bool $genuine = true, int $error = UPLOAD_ERR_OK, ?int $size = null): array { + $tmp = tempnam(sys_get_temp_dir(), 'lpup'); + file_put_contents($tmp, $contents); + $this->tmpFiles[] = $tmp; + if ($genuine === true) { + $GLOBALS['launchpadTestUploads'][] = $tmp; + } + + return ['name' => $name, 'tmp_name' => $tmp, 'size' => $size ?? strlen($contents), 'error' => $error]; + } + + public function testUploadNotAllowedIsRefusedBeforeAnyLookup(): void { + $this->rootFolder->expects($this->never())->method('getUserFolder'); + $this->expectException(NoAccessException::class); + + $this->service->uploadFiles('alice', ['allowUpload' => 0], null, []); + } + + public function testReadOnlyTargetRefusesUpload(): void { + $this->userFolder->method('get')->willReturn($this->folder(id: 8, updateable: false)); + $this->expectException(NoAccessException::class); + + $this->service->uploadFiles('alice', ['allowUpload' => true], 'sub', []); + } + + public function testUploadScreeningCodesAndNaming(): void { + $this->userFolder = $this->folder(id: 1, path: '/alice/files'); + $rootFolder = $this->createMock(IRootFolder::class); + $rootFolder->method('getUserFolder')->willReturn($this->userFolder); + $service = new FilesWidgetService($rootFolder, $this->createMock(IURLGenerator::class), $this->createMock(IPreview::class)); + $taken = ['taken.txt', 'taken (1).txt', '.env', 'noext']; + $this->userFolder->method('nodeExists')->willReturnCallback(static fn (string $path): bool => in_array($path, $taken, true)); + $written = []; + $this->userFolder->method('newFile')->willReturnCallback(function (string $path) use (&$written): File { + $written[] = $path; + if ($path === 'boom.txt') { + throw new RuntimeException('disk full'); + } + + $file = $this->file(100 + count($written), $path); + $file->method('fopen')->willReturn(fopen('php://memory', 'w')); + return $file; + }); + + $result = $service->uploadFiles('alice', ['allowUpload' => true, 'mimeTypeFilter' => ['text/*']], null, [ + $this->upload(''), + $this->upload('err.txt', 'x', true, UPLOAD_ERR_PARTIAL), + ['name' => 'notmp.txt', 'tmp_name' => '', 'size' => 1, 'error' => UPLOAD_ERR_OK], + $this->upload('crafted.txt', 'x', false), + $this->upload('huge.txt', 'x', true, UPLOAD_ERR_OK, FilesWidgetService::MAX_UPLOAD_BYTES + 1), + $this->upload('image.txt', "\x89PNG\r\n\x1a\n" . str_repeat("\0", 32)), + $this->upload('taken.txt'), + $this->upload('a/b\\c.txt'), + $this->upload('..'), + $this->upload('.env'), + $this->upload('noext'), + $this->upload('boom.txt'), + ]); + + $this->assertSame([ + ['name' => '', 'error' => 'upload_failed'], + ['name' => 'err.txt', 'error' => 'upload_failed'], + ['name' => 'notmp.txt', 'error' => 'upload_failed'], + ['name' => 'crafted.txt', 'error' => 'upload_failed'], + ['name' => 'huge.txt', 'error' => 'file_too_large'], + ['name' => 'image.txt', 'error' => 'file_type_not_allowed'], + ['name' => 'boom.txt', 'error' => 'storage_failed'], + ], $result['errors']); + $this->assertSame(['taken (2).txt', 'a_b_c.txt', 'upload', '.env (1)', 'noext (1)', 'boom.txt'], $written); + $this->assertSame(['taken (2).txt', 'a_b_c.txt', 'upload', '.env (1)', 'noext (1)'], array_column($result['uploaded'], 'name')); + } + + public function testUploadWithoutFilterAcceptsAnyType(): void { + $this->userFolder->method('nodeExists')->willReturn(false); + $file = $this->file(5, 'image.png'); + $file->method('fopen')->willReturn(fopen('php://memory', 'w')); + $this->userFolder->method('newFile')->willReturn($file); + + $result = $this->service->uploadFiles('alice', ['allowUpload' => true], null, [$this->upload('image.png', "\x89PNG\r\n\x1a\n")]); + + $this->assertSame([], $result['errors']); + $this->assertCount(1, $result['uploaded']); + } + + private function node(int $id, string $path, ?Node $parent, bool $deletable = true): File { + $node = $this->createMock(File::class); + $node->method('getId')->willReturn($id); + $node->method('getPath')->willReturn($path); + $node->method('isDeletable')->willReturn($deletable); + if ($parent === null) { + $node->method('getParent')->willThrowException(new NotFoundException('top')); + } else { + $node->method('getParent')->willReturn($parent); + } + + return $node; + } + + public function testDeleteNotAllowedIsRefusedBeforeAnyLookup(): void { + $this->rootFolder->expects($this->never())->method('getUserFolder'); + $this->expectException(NoAccessException::class); + + $this->service->deleteFile('alice', [], 5); + } + + /** + * Delete cases and their outcome. + * + * @return array + */ + public static function deletes(): array { + return [ + 'missing id' => ['missing', FolderNotFoundException::class], + 'lookup error' => ['lookup', FolderNotFoundException::class], + 'outside root' => ['outside', NoAccessException::class], + 'parent error' => ['orphan', NoAccessException::class], + 'too deep' => ['deep', NoAccessException::class], + 'not deletable' => ['locked', NoAccessException::class], + 'delete error' => ['fails', NoAccessException::class], + 'direct child' => ['child', 'ok'], + 'nested' => ['nested', 'ok'], + 'the root itself' => ['root', 'ok'], + ]; + } + + /** + * @dataProvider deletes + */ + public function testDelete(string $case, string $outcome): void { + $root = $this->folder(id: 40, path: '/alice/files/Shared'); + $this->userFolder->method('get')->willReturn($root); + $top = $this->folder(id: 1, path: '/'); + $other = $this->folder(id: 41, path: '/alice/files/Other'); + $other->method('getParent')->willReturn($top); + $sub = $this->folder(id: 42, path: '/alice/files/Shared/sub'); + $sub->method('getParent')->willReturn($root); + $deep = $this->node(500, '/d', null); + for ($i = 0; $i < 33; $i++) { + $deep = $this->node(600 + $i, '/d' . $i, $deep); + } + + $target = match ($case) { + 'outside' => $this->node(5, '/alice/files/Other/a', $other), + 'orphan' => $this->node(5, '/a', null), + 'deep' => $deep, + 'locked' => $this->node(5, '/alice/files/Shared/a', $root, false), + 'nested' => $this->node(5, '/alice/files/Shared/sub/a', $sub), + 'root' => $root, + default => $this->node(5, '/alice/files/Shared/a', $root), + }; + if ($case === 'fails') { + $target->method('delete')->willThrowException(new RuntimeException('x')); + } + + $this->userFolder->method('getById')->willReturnCallback(static function () use ($case, $target): array { + if ($case === 'lookup') { + throw new RuntimeException('x'); + } + + return $case === 'missing' ? [] : [$target]; + }); + + if ($outcome !== 'ok') { + $this->expectException($outcome); + } + + $result = $this->service->deleteFile('alice', ['allowDelete' => true, 'folderPath' => 'Shared'], 5); + + $this->assertSame(['status' => 'success', 'fileId' => 5], $result); + } + }//end class +}