diff --git a/apps/files/src/components/NewNodeDialog.vue b/apps/files/src/components/NewNodeDialog.vue index 21a15a0a74ce6..2a37ffffec632 100644 --- a/apps/files/src/components/NewNodeDialog.vue +++ b/apps/files/src/components/NewNodeDialog.vue @@ -16,7 +16,7 @@ variant="primary" :disabled="validity !== ''" @click="submit"> - {{ t('files', 'Create') }} + {{ submitLabel }}
({ + getUploader: vi.fn(), + hasConflict: vi.fn(), +})) +vi.mock('../utils/newNodeDialog.ts') +vi.mock('./DropServiceUtils.ts', async (importOriginal) => ({ + ...await importOriginal(), + createDirectoryIfNotExists: vi.fn(), +})) +vi.mock('@nextcloud/capabilities', () => ({ + getCapabilities: () => ({ + files: { + forbidden_filename_characters: ['/', '\\'], + forbidden_filenames: ['.htaccess'], + forbidden_filename_basenames: [], + forbidden_filename_extensions: ['.part'], + }, + }), +})) + +describe('onDropExternalFiles', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(getUploader).mockReturnValue({ + pause: vi.fn(), + start: vi.fn(), + } as never) + vi.mocked(hasConflict).mockReturnValue(false) + }) + + it('asks to rename an invalid entry before starting the upload', async () => { + const root = new Directory('root', [new Directory('test\\')]) as RootDirectory + vi.mocked(newNodeName).mockResolvedValue('test') + + await onDropExternalFiles(root, {} as IFolder, []) + + expect(newNodeName).toHaveBeenCalledWith('test\\', [], expect.objectContaining({ isFolder: true })) + expect(root.contents[0].name).toBe('test') + }) + + it('aborts the upload if the rename is cancelled', async () => { + const root = new Directory('root', [new Directory('test\\')]) as RootDirectory + vi.mocked(newNodeName).mockResolvedValue(null) + + const uploads = await onDropExternalFiles(root, {} as IFolder, []) + + expect(uploads).toEqual([]) + expect(showInfo).toHaveBeenCalledWith('Upload cancelled, drop the files again to retry') + expect(getUploader).not.toHaveBeenCalled() + expect(hasConflict).not.toHaveBeenCalled() + }) + + it('does not report success after a directory creation failure', async () => { + const root = new Directory('root', [new Directory('folder')]) as RootDirectory + vi.mocked(createDirectoryIfNotExists).mockRejectedValue(new Error('Failed to create directory')) + + const uploads = await onDropExternalFiles(root, {} as IFolder, []) + + expect(uploads).toEqual([]) + expect(showError).toHaveBeenCalledWith('Unable to create the directory folder') + expect(showSuccess).not.toHaveBeenCalled() + }) +}) diff --git a/apps/files/src/services/DropService.ts b/apps/files/src/services/DropService.ts index c408f183ac37d..e5978c53cad94 100644 --- a/apps/files/src/services/DropService.ts +++ b/apps/files/src/services/DropService.ts @@ -14,7 +14,7 @@ import { getUploader, hasConflict } from '@nextcloud/upload' import { handleCopyMoveNodesTo, HintException } from '../actions/moveOrCopyAction.ts' import { MoveCopyAction } from '../actions/moveOrCopyActionUtils.ts' import { logger } from '../utils/logger.ts' -import { createDirectoryIfNotExists, Directory, resolveConflict, traverseTree } from './DropServiceUtils.ts' +import { createDirectoryIfNotExists, Directory, renameInvalidDroppedEntries, resolveConflict, traverseTree } from './DropServiceUtils.ts' /** * This function converts a list of DataTransferItems to a file tree. @@ -94,6 +94,12 @@ export async function dataTransferToFileTree(items: DataTransferItem[]): Promise * @param contents - The contents of the destination folder */ export async function onDropExternalFiles(root: RootDirectory, destination: IFolder, contents: INode[]): Promise { + // Ask the user to fix any invalid name before uploading anything + if (!await renameInvalidDroppedEntries(root)) { + showInfo(t('files', 'Upload cancelled, drop the files again to retry')) + return [] + } + const uploader = getUploader() // Check for conflicts on root elements @@ -112,6 +118,7 @@ export async function onDropExternalFiles(root: RootDirectory, destination: IFol // Let's process the files logger.debug(`Uploading files to ${destination.path}`, { root, contents: root.contents }) const queue = [] as Promise[] + let hasDirectoryErrors = false const uploadDirectoryContents = async (directory: Directory, path: string) => { for (const file of directory.contents) { @@ -127,6 +134,7 @@ export async function onDropExternalFiles(root: RootDirectory, destination: IFol await createDirectoryIfNotExists(relativePath, destination) await uploadDirectoryContents(file, relativePath) } catch (error) { + hasDirectoryErrors = true showError(t('files', 'Unable to create the directory {directory}', { directory: file.name })) logger.error('Unable to create the directory', { error, relativePath, directory: file }) } @@ -158,6 +166,8 @@ export async function onDropExternalFiles(root: RootDirectory, destination: IFol if (errors.length > 0) { logger.error('Error while uploading files', { errors }) showError(t('files', 'Some files could not be uploaded')) + } + if (errors.length > 0 || hasDirectoryErrors) { return [] } diff --git a/apps/files/src/services/DropServiceUtils.spec.ts b/apps/files/src/services/DropServiceUtils.spec.ts index 2d9d602e62e46..e8d2e2ad33d69 100644 --- a/apps/files/src/services/DropServiceUtils.spec.ts +++ b/apps/files/src/services/DropServiceUtils.spec.ts @@ -4,13 +4,25 @@ */ import { join } from 'node:path' -import { beforeAll, describe, expect, it, vi } from 'vitest' +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import { DataTransferItem as DataTransferItemMock, FileSystemDirectoryEntry, fileSystemEntryToDataTransferItem, FileSystemFileEntry } from '../../../../__tests__/FileSystemAPIUtils.ts' import { logger } from '../utils/logger.ts' +import { newNodeName } from '../utils/newNodeDialog.ts' import { dataTransferToFileTree } from './DropService.ts' -import { Directory, traverseTree } from './DropServiceUtils.ts' +import { Directory, renameInvalidDroppedEntries, traverseTree } from './DropServiceUtils.ts' vi.mock('@nextcloud/dialogs') +vi.mock('../utils/newNodeDialog.ts') +vi.mock('@nextcloud/capabilities', () => ({ + getCapabilities: () => ({ + files: { + forbidden_filename_characters: ['/', '\\'], + forbidden_filenames: ['.htaccess'], + forbidden_filename_basenames: [], + forbidden_filename_extensions: ['.part', ' '], + }, + }), +})) const dataTree = { 'file0.txt': ['Hello, world!', 1234567890], @@ -87,6 +99,72 @@ describe('Filesystem API traverseTree', () => { }) }) +describe('renameInvalidDroppedEntries', () => { + beforeEach(() => vi.mocked(newNodeName).mockReset()) + + it('does not prompt for a valid tree', async () => { + const tree = new Directory('root', [ + new Directory('folder', [new File([], 'file.txt')]), + ]) + + expect(await renameInvalidDroppedEntries(tree)).toBe(true) + expect(newNodeName).not.toHaveBeenCalled() + }) + + it('renames an invalid top-level folder and keeps its contents', async () => { + const tree = new Directory('root', [new Directory('folder\\', [new File([], 'file.txt')])]) + vi.mocked(newNodeName).mockResolvedValue('folder') + + expect(await renameInvalidDroppedEntries(tree)).toBe(true) + expect(newNodeName).toHaveBeenCalledWith('folder\\', [], expect.objectContaining({ isFolder: true })) + expect(tree.contents[0].name).toBe('folder') + expect((tree.contents[0] as Directory).contents[0].name).toBe('file.txt') + }) + + it('renames an invalid nested file and keeps its content', async () => { + const tree = new Directory('root', [ + new Directory('folder', [new File(['content'], 'file\\.txt', { type: 'text/plain' })]), + ]) + vi.mocked(newNodeName).mockResolvedValue('file.txt') + + expect(await renameInvalidDroppedEntries(tree)).toBe(true) + expect(newNodeName).toHaveBeenCalledWith('file\\.txt', [], expect.objectContaining({ isFolder: false })) + + const file = (tree.contents[0] as Directory).contents[0] + expect(file.name).toBe('file.txt') + expect(file.type).toBe('text/plain') + expect(await file.text()).toBe('content') + }) + + it('keeps the new name unique within the dropped folder', async () => { + const tree = new Directory('root', [ + new File([], 'file.txt'), + new File([], 'file\\.txt'), + ]) + vi.mocked(newNodeName).mockResolvedValue('file.txt') + + expect(await renameInvalidDroppedEntries(tree)).toBe(true) + expect(newNodeName).toHaveBeenCalledWith('file\\.txt', ['file.txt'], expect.anything()) + expect(tree.contents[1].name).toBe('file (1).txt') + }) + + it('trims the name returned by the dialog', async () => { + const tree = new Directory('root', [new Directory('folder\\')]) + vi.mocked(newNodeName).mockResolvedValue(' folder ') + + expect(await renameInvalidDroppedEntries(tree)).toBe(true) + expect(tree.contents[0].name).toBe('folder') + }) + + it('aborts when the user cancels the rename', async () => { + const tree = new Directory('root', [new Directory('folder\\')]) + vi.mocked(newNodeName).mockResolvedValue(null) + + expect(await renameInvalidDroppedEntries(tree)).toBe(false) + expect(tree.contents[0].name).toBe('folder\\') + }) +}) + describe('DropService dataTransferToFileTree', () => { beforeAll(() => { // @ts-expect-error jsdom doesn't have DataTransferItem diff --git a/apps/files/src/services/DropServiceUtils.ts b/apps/files/src/services/DropServiceUtils.ts index 448923755f3b6..c64380b0d9774 100644 --- a/apps/files/src/services/DropServiceUtils.ts +++ b/apps/files/src/services/DropServiceUtils.ts @@ -8,11 +8,14 @@ import type { FileStat, ResponseDataDetailed } from 'webdav' import { showInfo, showWarning } from '@nextcloud/dialogs' import { emit } from '@nextcloud/event-bus' +import { getUniqueName } from '@nextcloud/files' import { defaultRemoteURL, defaultRootPath, getClient, getDefaultPropfind, resultToNode } from '@nextcloud/files/dav' import { t } from '@nextcloud/l10n' import { join } from '@nextcloud/paths' import { openConflictPicker } from '@nextcloud/upload' +import { getFilenameValidity } from '../utils/filenameValidity.ts' import { logger } from '../utils/logger.ts' +import { newNodeName } from '../utils/newNodeDialog.ts' /** * This represents a Directory in the file tree @@ -84,6 +87,49 @@ export type RootDirectory = Directory & { name: 'root' } +/** + * Ask the user to rename every invalid file or folder of a dropped file tree. + * Entries are replaced instead of renamed, as `File.name` is read only. + * + * @param directory Directory to validate + * @param path Path of the directory relative to the upload destination + * @return false if the user aborted the rename + */ +export async function renameInvalidDroppedEntries(directory: Directory, path = ''): Promise { + for (const [index, entry] of directory.contents.entries()) { + const isFolder = entry instanceof Directory + let node = entry + + if (getFilenameValidity(node.name, false, isFolder) !== '') { + const otherNames = directory.contents.filter((other) => other !== node).map((other) => other.name) + const name = await newNodeName(node.name, otherNames, { + name: t('files', 'Invalid name for "{path}"', { path: join(path, node.name) }, { escape: false }), + label: isFolder ? t('files', 'Folder name') : t('files', 'Filename'), + submitLabel: t('files', 'Rename'), + isFolder, + }) + + if (name === null) { + logger.debug('Upload cancelled while renaming an invalid entry', { path, name: node.name }) + return false + } + + // Keep the name unique within the dropped folder + const uniqueName = getUniqueName(name.trim(), otherNames) + node = isFolder + ? new Directory(uniqueName, (node as Directory).contents) + : new File([node], uniqueName, { type: node.type, lastModified: node.lastModified }) + directory.contents.splice(index, 1, node) + } + + if (node instanceof Directory && !await renameInvalidDroppedEntries(node, join(path, node.name))) { + return false + } + } + + return true +} + /** * Traverse a file tree using the Filesystem API * diff --git a/apps/files/src/utils/newNodeDialog.ts b/apps/files/src/utils/newNodeDialog.ts index 30e26a0f9b649..20a571aea5fec 100644 --- a/apps/files/src/utils/newNodeDialog.ts +++ b/apps/files/src/utils/newNodeDialog.ts @@ -18,6 +18,11 @@ interface NewNodeDialogOptions { */ label?: string + /** + * Label for the submit button, defaults to "Create" + */ + submitLabel?: string + /** * Whether the name is for a folder, defaults to false. */ @@ -28,12 +33,12 @@ interface NewNodeDialogOptions { * Ask user for file or folder name * * @param defaultName Default name to use - * @param folderContent Nodes with in the current folder to check for unique name + * @param folderContent Nodes or names within the current folder to check for unique name * @param options Options for the dialog * @return string if successful otherwise null if aborted */ -export function newNodeName(defaultName: string, folderContent: INode[], options: NewNodeDialogOptions = {}) { - const contentNames = folderContent.map((node: INode) => node.basename) +export function newNodeName(defaultName: string, folderContent: (INode | string)[], options: NewNodeDialogOptions = {}) { + const contentNames = folderContent.map((node) => typeof node === 'string' ? node : node.basename) return new Promise((resolve) => { spawnDialog(NewNodeDialog, {