fix(files): Correctly validate new node name

* Resolves https://github.com/nextcloud/server/issues/45409

This includes two fixes:
1. The name in the "new node" dialog is correctly selected (e.g. `file.txt` only `file` is selected by default), to allow quick naming
2. `@nextcloud/files` functions for filename validation are used, this allows to use new Nextcloud 30 capabilities (e.g. reserved names)

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>

apps/files/src/components/FileEntry/FileEntryName.vue

@@ -27,7 +27,7 @@
 		class="files-list__row-name-link"
 		data-cy-files-list-row-name-link
 		v-bind="linkTo.params">
-		<!-- File name -->
+		<!-- Filename -->
 		<span class="files-list__row-name-text">
 			<!-- Keep the filename stuck to the extension to avoid whitespace rendering issues-->
 			<span class="files-list__row-name-" v-text="basename" />
@@ -123,7 +123,7 @@ export default defineComponent({
 
 		renameLabel() {
 			const matchLabel: Record<FileType, string> = {
-				[FileType.File]: t('files', 'File name'),
+				[FileType.File]: t('files', 'Filename'),
 				[FileType.Folder]: t('files', 'Folder name'),
 			}
 			return matchLabel[this.source.type]
@@ -173,7 +173,7 @@ export default defineComponent({
 
 	watch: {
 		/**
-		 * If renaming starts, select the file name
+		 * If renaming starts, select the filename
 		 * in the input, without the extension.
 		 * @param renaming
 		 */

apps/files/src/components/NewNodeDialog.vue

@@ -8,205 +8,150 @@
 		:open="open"
 		close-on-click-outside
 		out-transition
-		@update:open="onClose">
+		@update:open="emit('close', null)">
 		<template #actions>
 			<NcButton data-cy-files-new-node-dialog-submit
 				type="primary"
-				:disabled="!isUniqueName"
-				@click="onCreate">
+				:disabled="validity !== ''"
+				@click="submit">
 				{{ t('files', 'Create') }}
 			</NcButton>
 		</template>
-		<form @submit.prevent="onCreate">
-			<NcTextField ref="input"
+		<form ref="formElement"
+			class="new-node-dialog__form"
+			@submit.prevent="emit('close', localDefaultName)">
+			<NcTextField ref="nameInput"
 				data-cy-files-new-node-dialog-input
-				class="dialog__input"
-				:error="!isUniqueName"
-				:helper-text="errorMessage"
+				:error="validity !== ''"
+				:helper-text="validity"
 				:label="label"
-				:value.sync="localDefaultName"
-				@keyup="checkInputValidity" />
+				:value.sync="localDefaultName" />
 		</form>
 	</NcDialog>
 </template>
 
-<script lang="ts">
-import type { PropType } from 'vue'
-
-import { defineComponent } from 'vue'
-import { translate as t } from '@nextcloud/l10n'
+<script setup lang="ts">
+import type { ComponentPublicInstance, PropType } from 'vue'
 import { getUniqueName } from '@nextcloud/files'
-import { loadState } from '@nextcloud/initial-state'
+import { t } from '@nextcloud/l10n'
+import { extname } from 'path'
+import { nextTick, onMounted, ref, watch, watchEffect } from 'vue'
+import { getFilenameValidity } from '../utils/filenameValidity.ts'
 
 import NcButton from '@nextcloud/vue/dist/Components/NcButton.js'
 import NcDialog from '@nextcloud/vue/dist/Components/NcDialog.js'
 import NcTextField from '@nextcloud/vue/dist/Components/NcTextField.js'
-import logger from '../logger.js'
 
-interface ICanFocus {
-	focus: () => void
+const props = defineProps({
+	/**
+	 * The name to be used by default
+	 */
+	defaultName: {
+		type: String,
+		default: t('files', 'New folder'),
+	},
+	/**
+	 * Other files that are in the current directory
+	 */
+	otherNames: {
+		type: Array as PropType<string[]>,
+		default: () => [],
+	},
+	/**
+	 * Open state of the dialog
+	 */
+	open: {
+		type: Boolean,
+		default: true,
+	},
+	/**
+	 * Dialog name
+	 */
+	name: {
+		type: String,
+		default: t('files', 'Create new folder'),
+	},
+	/**
+	 * Input label
+	 */
+	label: {
+		type: String,
+		default: t('files', 'Folder name'),
+	},
+})
+
+const emit = defineEmits<{
+	(event: 'close', name: string | null): void
+}>()
+
+const localDefaultName = ref<string>(props.defaultName)
+const nameInput = ref<ComponentPublicInstance>()
+const formElement = ref<HTMLFormElement>()
+const validity = ref('')
+
+/**
+ * Focus the filename input field
+ */
+function focusInput() {
+	nextTick(() => {
+		// get the input element
+		const input = nameInput.value?.$el.querySelector('input')
+		if (!props.open || !input) {
+			return
+		}
+
+		// length of the basename
+		const length = localDefaultName.value.length - extname(localDefaultName.value).length
+		// focus the input
+		input.focus()
+		// and set the selection to the basename (name without extension)
+		input.setSelectionRange(0, length)
+	})
 }
 
-const forbiddenCharacters = loadState<string[]>('files', 'forbiddenCharacters', [])
+/**
+ * Trigger submit on the form
+ */
+function submit() {
+	formElement.value?.requestSubmit()
+}
 
-export default defineComponent({
-	name: 'NewNodeDialog',
-	components: {
-		NcButton,
-		NcDialog,
-		NcTextField,
-	},
-	props: {
-		/**
-		 * The name to be used by default
-		 */
-		defaultName: {
-			type: String,
-			default: t('files', 'New folder'),
-		},
-		/**
-		 * Other files that are in the current directory
-		 */
-		otherNames: {
-			type: Array as PropType<string[]>,
-			default: () => [],
-		},
-		/**
-		 * Open state of the dialog
-		 */
-		open: {
-			type: Boolean,
-			default: true,
-		},
-		/**
-		 * Dialog name
-		 */
-		name: {
-			type: String,
-			default: t('files', 'Create new folder'),
-		},
-		/**
-		 * Input label
-		 */
-		label: {
-			type: String,
-			default: t('files', 'Folder name'),
-		},
-	},
-	emits: {
-		close: (name: string|null) => name === null || name,
-	},
-	data() {
-		return {
-			localDefaultName: this.defaultName || t('files', 'New folder'),
-		}
-	},
-	computed: {
-		errorMessage() {
-			if (this.isUniqueName) {
-				return ''
-			} else {
-				return t('files', 'A file or folder with that name already exists.')
-			}
-		},
-		uniqueName() {
-			return getUniqueName(this.localDefaultName, this.otherNames)
-		},
-		isUniqueName() {
-			return this.localDefaultName === this.uniqueName
-		},
-	},
-	watch: {
-		defaultName() {
-			this.localDefaultName = this.defaultName || t('files', 'New folder')
-		},
+// Reset local name on props change
+watch(() => props.defaultName, () => {
+	localDefaultName.value = getUniqueName(props.defaultName, props.otherNames)
+})
 
-		/**
-		 * Ensure the input is focussed even if the dialog is already mounted but not open
-		 */
-		open() {
-			this.$nextTick(() => this.focusInput())
-		},
-	},
-	mounted() {
-		// on mounted lets use the unique name
-		this.localDefaultName = this.uniqueName
-		this.$nextTick(() => this.focusInput())
-	},
-	methods: {
-		t,
+// Validate the local name
+watchEffect(() => {
+	if (props.otherNames.includes(localDefaultName.value)) {
+		validity.value = t('files', 'This name is already in use.')
+	} else {
+		validity.value = getFilenameValidity(localDefaultName.value)
+	}
+	const input = nameInput.value?.$el.querySelector('input')
+	if (input) {
+		input.setCustomValidity(validity.value)
+		input.reportValidity()
+	}
+})
 
-		/**
-		 * Focus the filename input field
-		 */
-		focusInput() {
-			if (this.open) {
-				this.$nextTick(() => (this.$refs.input as unknown as ICanFocus)?.focus?.())
-			}
-		},
+// Ensure the input is focussed even if the dialog is already mounted but not open
+watch(() => props.open, () => {
+	nextTick(() => {
+		focusInput()
+	})
+})
 
-		onCreate() {
-			this.$emit('close', this.localDefaultName)
-		},
-		onClose(state: boolean) {
-			if (!state) {
-				this.$emit('close', null)
-			}
-		},
-
-		/**
-		 * Check if the file name is valid and update the
-		 * input validity using browser's native validation.
-		 * @param event the keyup event
-		 */
-		checkInputValidity(event: KeyboardEvent) {
-			const input = event.target as HTMLInputElement
-			const newName = this.localDefaultName.trim?.() || ''
-			logger.debug('Checking input validity', { newName })
-			try {
-				this.isFileNameValid(newName)
-				input.setCustomValidity('')
-				input.title = ''
-			} catch (e) {
-				if (e instanceof Error) {
-					input.setCustomValidity(e.message)
-					input.title = e.message
-				} else {
-					input.setCustomValidity(t('files', 'Invalid file name'))
-				}
-			} finally {
-				input.reportValidity()
-			}
-		},
-
-		isFileNameValid(name: string) {
-			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) {
-				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(window.OC.config.blacklist_files_regex)) {
-				throw new Error(t('files', '"{name}" is not an allowed filetype.', { name }))
-			}
-
-			return true
-		},
-	},
+onMounted(() => {
+	// on mounted lets use the unique name
+	localDefaultName.value = getUniqueName(localDefaultName.value, props.otherNames)
+	nextTick(() => focusInput())
 })
 </script>
 
-<style lang="scss" scoped>
-.dialog__input {
-	:deep(input:invalid) {
-		// Show red border on invalid input
-		border-color: var(--color-error);
-		color: red;
-	}
+<style scoped>
+.new-node-dialog__form {
+	/* Ensure the dialog does not jump when there is a validity error */
+	min-height: calc(3 * var(--default-clickable-area));
 }
 </style>

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<HTMLElement>) => expect((el.get(0) as HTMLInputElement).validationMessage).to.equal(validity)
+	}
+	return (el: JQuery<HTMLElement>) => 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(/reserved name/i))
+	})
+})