fix(files): distinguish between files and folders for validation messages

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

@@ -190,7 +190,7 @@ export default defineComponent({
 				return
 			}
 
-			let validity = getFilenameValidity(newName)
+			let validity = getFilenameValidity(newName, false, this.source.type === FileType.Folder)
 			// Checking if already exists
 			if (validity === '' && this.checkIfNodeExists(newName)) {
 				validity = t('files', 'Another entry with the same name already exists.')

apps/files/src/components/NewNodeDialog.vue

@@ -89,6 +89,14 @@ const props = defineProps({
 		type: String,
 		default: t('files', 'Folder name'),
 	},
+
+	/**
+	 * Whether the name is for a folder, which affects the validation of the name. Defaults to false.
+	 */
+	isFolder: {
+		type: Boolean,
+		default: false,
+	},
 })
 
 const emit = defineEmits<{
@@ -142,7 +150,7 @@ watchEffect(() => {
 	if (props.otherNames.includes(localDefaultName.value.trim())) {
 		validity.value = t('files', 'This name is already in use.')
 	} else {
-		validity.value = getFilenameValidity(localDefaultName.value.trim())
+		validity.value = getFilenameValidity(localDefaultName.value.trim(), false, props.isFolder)
 	}
 	const input = nameInput.value?.$el.querySelector('input')
 	if (input) {

apps/files/src/newMenu/newFolder.ts

@@ -29,7 +29,7 @@ export const entry: NewMenuEntry = {
 	},
 
 	async handler(context: IFolder, content: INode[]) {
-		const name = await newNodeName(t('files', 'New folder'), content)
+		const name = await newNodeName(t('files', 'New folder'), content, { isFolder: true })
 		if (name === null) {
 			return
 		}

apps/files/src/newMenu/newTemplatesFolder.ts

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import type { Folder, NewMenuEntry, Node } from '@nextcloud/files'
+import type { IFolder, INode, NewMenuEntry } from '@nextcloud/files'
 
 import PlusSvg from '@mdi/svg/svg/plus.svg?raw'
 import { getCurrentUser } from '@nextcloud/auth'
@@ -28,7 +28,7 @@ logger.debug('Initial templates folder', { templatesPath })
  * @param directory Folder where to create the templates folder
  * @param name Name to use or the templates folder
  */
-async function initTemplatesFolder(directory: Folder, name: string) {
+async function initTemplatesFolder(directory: IFolder, name: string) {
 	const templatePath = join(directory.path, name)
 	try {
 		logger.debug('Initializing the templates directory', { templatePath })
@@ -59,7 +59,7 @@ export const entry: NewMenuEntry = {
 	displayName: t('files', 'Create templates folder'),
 	iconSvgInline: PlusSvg,
 	order: 30,
-	enabled(context: Folder): boolean {
+	enabled(context: IFolder): boolean {
 		// Templates disabled or templates folder already initialized
 		if (!templatesEnabled || templatesPath) {
 			return false
@@ -70,8 +70,8 @@ export const entry: NewMenuEntry = {
 		}
 		return (context.permissions & Permission.CREATE) !== 0
 	},
-	async handler(context: Folder, content: Node[]) {
-		const name = await newNodeName(t('files', 'Templates'), content, { name: t('files', 'New template folder') })
+	async handler(context: IFolder, content: INode[]) {
+		const name = await newNodeName(t('files', 'Templates'), content, { name: t('files', 'New template folder'), isFolder: true })
 
 		if (name !== null) {
 			// Create the template folder

apps/files/src/utils/filenameValidity.spec.ts

@@ -0,0 +1,100 @@
+/*!
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import { describe, expect, it, vi } from 'vitest'
+import { getFilenameValidity } from './filenameValidity.ts'
+
+vi.mock('@nextcloud/capabilities', () => ({
+	getCapabilities: () => ({
+		files: {
+			forbidden_filename_characters: ['/', '\\', '>'],
+			forbidden_filenames: ['.htaccess'],
+			forbidden_filename_basenames: ['con'],
+			forbidden_filename_extensions: ['.exe', '.~'],
+		},
+	}),
+}))
+
+describe('getFilenameValidity', () => {
+	it('returns no error for a valid filename', () => {
+		expect(getFilenameValidity('valid-name.txt')).toBe('')
+	})
+
+	describe('empty name', () => {
+		it('reports empty filename', () => {
+			expect(getFilenameValidity('')).toBe('Filename must not be empty.')
+		})
+
+		it('reports empty filename for whitespace only', () => {
+			expect(getFilenameValidity('   ')).toBe('Filename must not be empty.')
+		})
+
+		it('reports empty folder name when isFolder is set', () => {
+			expect(getFilenameValidity('', false, true)).toBe('Folder name must not be empty.')
+		})
+	})
+
+	describe('forbidden character', () => {
+		it('reports forbidden character for a filename', () => {
+			expect(getFilenameValidity('inva/lid')).toBe('"/" is not allowed inside a filename.')
+		})
+
+		it('reports forbidden character for a folder name', () => {
+			expect(getFilenameValidity('inva/lid', false, true)).toBe('"/" is not allowed inside a folder name.')
+		})
+	})
+
+	describe('reserved name', () => {
+		it('reports a reserved filename', () => {
+			expect(getFilenameValidity('.htaccess')).toBe('".htaccess" is a reserved name and not allowed for filenames.')
+		})
+
+		it('reports a reserved folder name', () => {
+			expect(getFilenameValidity('.htaccess', false, true)).toBe('".htaccess" is a reserved name and not allowed for folder names.')
+		})
+
+		it('reports a reserved basename', () => {
+			expect(getFilenameValidity('con.txt')).toBe('"con" is a reserved name and not allowed for filenames.')
+		})
+	})
+
+	describe('forbidden extension', () => {
+		it('reports a disallowed filetype when the extension looks like a real type', () => {
+			expect(getFilenameValidity('virus.exe')).toBe('".exe" is not an allowed filetype.')
+		})
+
+		it('reports the extension as not allowed at the end of a filename when it is not a recognisable filetype', () => {
+			// '.~' does not match /\.[a-z]/i, so the generic "must not end with" message is used.
+			expect(getFilenameValidity('document.~')).toBe('Filenames must not end with ".~".')
+		})
+
+		it('reports the extension as not allowed for a folder name', () => {
+			expect(getFilenameValidity('folder.exe', false, true)).toBe('Folder names must not end with ".exe".')
+		})
+	})
+
+	describe('escape option', () => {
+		it('does not affect the returned string for safe characters', () => {
+			expect(getFilenameValidity('inva/lid', true)).toBe('"/" is not allowed inside a filename.')
+		})
+
+		it('escapes the matched character when requested', () => {
+			expect(getFilenameValidity('inva>lid', true)).toBe('">" is not allowed inside a filename.')
+		})
+
+		it('does not escape the matched character by default', () => {
+			expect(getFilenameValidity('inva>lid')).toBe('">" is not allowed inside a filename.')
+		})
+	})
+
+	it('rethrows errors that are not InvalidFilenameError', async () => {
+		const files = await import('@nextcloud/files')
+		const spy = vi.spyOn(files, 'validateFilename').mockImplementation(() => {
+			throw new Error('unexpected')
+		})
+		expect(() => getFilenameValidity('anything')).toThrow('unexpected')
+		spy.mockRestore()
+	})
+})

apps/files/src/utils/filenameValidity.ts

@@ -11,9 +11,13 @@ import { t } from '@nextcloud/l10n'
  *
  * @param name The filename
  * @param escape Escape the matched string in the error (only set when used in HTML)
+ * @param isFolder Whether the filename is for a folder
  */
-export function getFilenameValidity(name: string, escape = false): string {
+export function getFilenameValidity(name: string, escape = false, isFolder = false): string {
 	if (name.trim() === '') {
+		if (isFolder) {
+			return t('files', 'Folder name must not be empty.')
+		}
 		return t('files', 'Filename must not be empty.')
 	}
 
@@ -27,15 +31,27 @@ export function getFilenameValidity(name: string, escape = false): string {
 
 		switch (error.reason) {
 			case InvalidFilenameErrorReason.Character:
-				return t('files', '"{char}" is not allowed inside a filename.', { char: error.segment }, undefined, { escape })
+				if (isFolder) {
+					return t('files', '"{char}" is not allowed inside a folder name.', { char: error.segment }, { escape })
+				}
+				return t('files', '"{char}" is not allowed inside a filename.', { char: error.segment }, { escape })
 			case InvalidFilenameErrorReason.ReservedName:
-				return t('files', '"{segment}" is a reserved name and not allowed for filenames.', { segment: error.segment }, undefined, { escape: false })
+				if (isFolder) {
+					return t('files', '"{segment}" is a reserved name and not allowed for folder names.', { segment: error.segment }, { escape: false })
+				}
+				return t('files', '"{segment}" is a reserved name and not allowed for filenames.', { segment: error.segment }, { 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 })
+				if (!isFolder && error.segment.match(/\.[a-z]/i)) {
+					return t('files', '"{extension}" is not an allowed filetype.', { extension: error.segment }, { escape: false })
 				}
-				return t('files', 'Filenames must not end with "{extension}".', { extension: error.segment }, undefined, { escape: false })
+				if (isFolder) {
+					return t('files', 'Folder names must not end with "{extension}".', { extension: error.segment }, { escape: false })
+				}
+				return t('files', 'Filenames must not end with "{extension}".', { extension: error.segment }, { escape: false })
 			default:
+				if (isFolder) {
+					return t('files', 'Invalid folder name.')
+				}
 				return t('files', 'Invalid filename.')
 		}
 	}

apps/files/src/utils/newNodeDialog.ts

@@ -8,7 +8,7 @@ import type { INode } from '@nextcloud/files'
 import { spawnDialog } from '@nextcloud/vue/functions/dialog'
 import NewNodeDialog from '../components/NewNodeDialog.vue'
 
-interface ILabels {
+interface NewNodeDialogOptions {
 	/**
 	 * Dialog heading, defaults to "New folder name"
 	 */
@@ -17,22 +17,27 @@ interface ILabels {
 	 * Label for input box, defaults to "New folder"
 	 */
 	label?: string
+
+	/**
+	 * Whether the name is for a folder, defaults to false.
+	 */
+	isFolder?: boolean
 }
 
 /**
  * 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 labels Labels to set on the dialog
+ * @param options Options for the dialog
  * @return string if successful otherwise null if aborted
  */
-export function newNodeName(defaultName: string, folderContent: INode[], labels: ILabels = {}) {
+export function newNodeName(defaultName: string, folderContent: INode[], options: NewNodeDialogOptions = {}) {
 	const contentNames = folderContent.map((node: INode) => node.basename)
 
 	return new Promise<string | null>((resolve) => {
 		spawnDialog(NewNodeDialog, {
-			...labels,
+			...options,
 			defaultName,
 			otherNames: contentNames,
 		}, (folderName) => {