diff --git a/apps/files/src/components/FileEntry/FileEntryName.vue b/apps/files/src/components/FileEntry/FileEntryName.vue index c73972f3ead..307a9c78330 100644 --- a/apps/files/src/components/FileEntry/FileEntryName.vue +++ b/apps/files/src/components/FileEntry/FileEntryName.vue @@ -44,7 +44,7 @@ class="files-list__row-name-link" data-cy-files-list-row-name-link v-bind="linkTo.params"> - + @@ -60,7 +60,7 @@ import type { PropType } from 'vue' import axios from '@nextcloud/axios' import { showError, showSuccess } from '@nextcloud/dialogs' import { emit } from '@nextcloud/event-bus' -import { FileType, NodeStatus } from '@nextcloud/files' +import { FileType, InvalidFilenameError, InvalidFilenameErrorReason, NodeStatus, validateFilename } from '@nextcloud/files' import { loadState } from '@nextcloud/initial-state' import { translate as t } from '@nextcloud/l10n' import { isAxiosError} from 'axios' @@ -149,7 +149,7 @@ export default Vue.extend({ renameLabel() { const matchLabel: Record = { - [FileType.File]: t('files', 'File name'), + [FileType.File]: t('files', 'Filename'), [FileType.Folder]: t('files', 'Folder name'), } return matchLabel[this.source.type] @@ -187,7 +187,7 @@ export default Vue.extend({ watch: { /** - * If renaming starts, select the file name + * If renaming starts, select the filename * in the input, without the extension. * @param renaming */ @@ -222,27 +222,35 @@ export default Vue.extend({ input.reportValidity() } }, - isFileNameValid(name) { - const trimmedName = name.trim() - const char = trimmedName.indexOf('/') !== -1 - ? '/' - : forbiddenCharacters.find((char) => trimmedName.includes(char)) - - if (trimmedName === '.' || trimmedName === '..') { - throw new Error(t('files', '"{name}" is an invalid file name.', { name })) - } else if (trimmedName.length === 0) { + isFileNameValid(name: string) { + if (name.trim() === '') { throw new Error(t('files', 'File name cannot be empty.')) - } else if (char) { - throw new Error(t('files', '"{char}" is not allowed inside a file name.', { char })) - } else if (trimmedName.match(OC.config.blacklist_files_regex)) { - throw new Error(t('files', '"{name}" is not an allowed filetype.', { name })) } else if (this.checkIfNodeExists(name)) { throw new Error(t('files', '{newName} already exists.', { newName: name })) } - return true + try { + validateFilename(name) + } catch (error) { + if (!(error instanceof InvalidFilenameError)) { + logger.error(error as Error) + return + } + switch (error.reason) { + case InvalidFilenameErrorReason.Character: + throw new Error(t('files', '"{segment}" is not allowed inside a filename.', { segment: error.segment })) + case InvalidFilenameErrorReason.ReservedName: + throw new Error(t('files', '"{segment}" is a forbidden file or folder name.', { segment: error.segment })) + case InvalidFilenameErrorReason.Extension: + if (error.segment.startsWith('.')) { + throw new Error(t('files', '"{segment}" is not an allowed filetype.', { segment: error.segment })) + } else { + throw new Error(t('files', 'Filenames must not end with "{segment}".', { segment: error.segment })) + } + } + } }, - checkIfNodeExists(name) { + checkIfNodeExists(name: string) { return this.nodes.find(node => node.basename === name && node !== this.source) }, diff --git a/apps/files/src/components/NewNodeDialog.vue b/apps/files/src/components/NewNodeDialog.vue index 0f6a739d21c..b4647724de3 100644 --- a/apps/files/src/components/NewNodeDialog.vue +++ b/apps/files/src/components/NewNodeDialog.vue @@ -1,226 +1,157 @@ + - SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + - SPDX-License-Identifier: AGPL-3.0-or-later +--> - - diff --git a/apps/files/src/utils/filenameValidity.ts b/apps/files/src/utils/filenameValidity.ts new file mode 100644 index 00000000000..2666d530052 --- /dev/null +++ b/apps/files/src/utils/filenameValidity.ts @@ -0,0 +1,41 @@ +/*! + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import { InvalidFilenameError, InvalidFilenameErrorReason, validateFilename } from '@nextcloud/files' +import { t } from '@nextcloud/l10n' + +/** + * Get the validity of a filename (empty if valid). + * This can be used for `setCustomValidity` on input elements + * @param name The filename + * @param escape Escape the matched string in the error (only set when used in HTML) + */ +export function getFilenameValidity(name: string, escape = false): string { + if (name.trim() === '') { + return t('files', 'Filename must not be empty.') + } + + try { + validateFilename(name) + return '' + } catch (error) { + if (!(error instanceof InvalidFilenameError)) { + throw error + } + + switch (error.reason) { + case InvalidFilenameErrorReason.Character: + return t('files', '"{char}" is not allowed inside a filename.', { char: error.segment }, undefined, { escape }) + case InvalidFilenameErrorReason.ReservedName: + return t('files', '"{segment}" is a reserved name and not allowed for filenames.', { segment: error.segment }, undefined, { escape: false }) + case InvalidFilenameErrorReason.Extension: + if (error.segment.match(/\.[a-z]/i)) { + return t('files', '"{extension}" is not an allowed filetype.', { extension: error.segment }, undefined, { escape: false }) + } + return t('files', 'Filenames must not end with "{extension}".', { extension: error.segment }, undefined, { escape: false }) + default: + return t('files', 'Invalid filename.') + } + } +} diff --git a/cypress/e2e/files/FilesUtils.ts b/cypress/e2e/files/FilesUtils.ts index 3ff459d4c43..98dae12ce97 100644 --- a/cypress/e2e/files/FilesUtils.ts +++ b/cypress/e2e/files/FilesUtils.ts @@ -28,9 +28,12 @@ export const getActionButtonForFile = (filename: string) => getActionsForFile(fi export const triggerActionForFile = (filename: string, actionId: string) => { getActionButtonForFile(filename).click({ force: true }) - cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`).should('exist') - cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`).scrollIntoView() - cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`).click({ force: true }) + cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`) + .should('exist') + .scrollIntoView() + cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`) + .should('be.visible') + .click({ force: true }) } export const moveFile = (fileName: string, dirPath: string) => { diff --git a/cypress/e2e/files/files-renaming.cy.ts b/cypress/e2e/files/files-renaming.cy.ts new file mode 100644 index 00000000000..e6dfc227f2a --- /dev/null +++ b/cypress/e2e/files/files-renaming.cy.ts @@ -0,0 +1,77 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { User } from '@nextcloud/cypress' +import { getRowForFile, triggerActionForFile } from './FilesUtils' + +const haveValidity = (validity: string | RegExp) => { + if (typeof validity === 'string') { + return (el: JQuery) => expect((el.get(0) as HTMLInputElement).validationMessage).to.equal(validity) + } + return (el: JQuery) => expect((el.get(0) as HTMLInputElement).validationMessage).to.match(validity) +} + +describe('files: Rename nodes', { testIsolation: true }, () => { + let user: User + + beforeEach(() => cy.createRandomUser().then(($user) => { + user = $user + + cy.uploadContent(user, new Blob([]), 'text/plain', '/file.txt') + cy.login(user) + cy.visit('/apps/files') + })) + + it('can rename a file', () => { + // All are visible by default + getRowForFile('file.txt').should('be.visible') + + triggerActionForFile('file.txt', 'rename') + + getRowForFile('file.txt') + .findByRole('textbox', { name: 'Filename' }) + .should('be.visible') + .type('{selectAll}other.txt') + .should(haveValidity('')) + .type('{enter}') + + // See it is renamed + getRowForFile('other.txt').should('be.visible') + }) + + /** + * If this test gets flaky than we have a problem: + * It means that the selection is not reliable set to the basename + */ + it('only selects basename of file', () => { + // All are visible by default + getRowForFile('file.txt').should('be.visible') + + triggerActionForFile('file.txt', 'rename') + + getRowForFile('file.txt') + .findByRole('textbox', { name: 'Filename' }) + .should('be.visible') + .should((el) => { + const input = el.get(0) as HTMLInputElement + expect(input.selectionStart).to.equal(0) + expect(input.selectionEnd).to.equal('file'.length) + }) + }) + + it('show validation error on file rename', () => { + // All are visible by default + getRowForFile('file.txt').should('be.visible') + + triggerActionForFile('file.txt', 'rename') + + getRowForFile('file.txt') + .findByRole('textbox', { name: 'Filename' }) + .should('be.visible') + .type('{selectAll}.htaccess') + // See validity + .should(haveValidity(/forbidden file or folder name/i)) + }) +}) diff --git a/package-lock.json b/package-lock.json index e72dd42d53f..6e01a8f1c0b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,7 +20,7 @@ "@nextcloud/capabilities": "^1.2.0", "@nextcloud/dialogs": "^5.3.7", "@nextcloud/event-bus": "^3.3.1", - "@nextcloud/files": "^3.6.0", + "@nextcloud/files": "^3.8.0", "@nextcloud/initial-state": "^2.2.0", "@nextcloud/l10n": "^3.1.0", "@nextcloud/logger": "^2.5.0", diff --git a/package.json b/package.json index 945d6f7ea08..5c844af5359 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "@nextcloud/capabilities": "^1.2.0", "@nextcloud/dialogs": "^5.3.7", "@nextcloud/event-bus": "^3.3.1", - "@nextcloud/files": "^3.6.0", + "@nextcloud/files": "^3.8.0", "@nextcloud/initial-state": "^2.2.0", "@nextcloud/l10n": "^3.1.0", "@nextcloud/logger": "^2.5.0",