Merge pull request #61921 from nextcloud/backport/61506/stable32

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

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

@@ -188,7 +188,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

@@ -84,6 +84,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<{
@@ -137,7 +145,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

@@ -2,7 +2,7 @@
  * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-import type { Entry, Node } from '@nextcloud/files'
+import type { NewMenuEntry, IFolder, INode } from '@nextcloud/files'
 
 import { basename } from 'path'
 import { emit } from '@nextcloud/event-bus'
@@ -22,7 +22,7 @@ type createFolderResponse = {
 	source: string
 }
 
-const createNewFolder = async (root: Folder, name: string): Promise<createFolderResponse> => {
+const createNewFolder = async (root: IFolder, name: string): Promise<createFolderResponse> => {
 	const source = root.source + '/' + name
 	const encodedSource = root.encodedSource + '/' + encodeURIComponent(name)
 
@@ -39,7 +39,7 @@ const createNewFolder = async (root: Folder, name: string): Promise<createFolder
 	}
 }
 
-export const entry = {
+export const entry: NewMenuEntry = {
 	id: 'newFolder',
 	displayName: t('files', 'New folder'),
 	enabled: (context: Folder) => Boolean(context.permissions & Permission.CREATE) && Boolean(context.permissions & Permission.READ),
@@ -48,8 +48,8 @@ export const entry = {
 	iconSvgInline: FolderPlusSvg.replace(/viewBox/gi, 'style="color: var(--color-primary-element)" viewBox'),
 	order: 0,
 
-	async handler(context: Folder, content: Node[]) {
-		const name = await newNodeName(t('files', 'New folder'), content)
+	async handler(context: IFolder, content: INode[]) {
+		const name = await newNodeName(t('files', 'New folder'), content, { isFolder: true })
 		if (name === null) {
 			return
 		}
@@ -88,4 +88,4 @@ export const entry = {
 			showError('Creating new folder failed')
 		}
 	},
-} as Entry
+}

apps/files/src/newMenu/newTemplatesFolder.ts

@@ -2,7 +2,8 @@
  * SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-import type { Entry, Folder, Node } from '@nextcloud/files'
+
+import type { IFolder, INode, NewMenuEntry } from '@nextcloud/files'
 
 import { getCurrentUser } from '@nextcloud/auth'
 import { showError } from '@nextcloud/dialogs'
@@ -27,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
  */
-const initTemplatesFolder = async function(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 })
@@ -58,7 +59,7 @@ export const entry = {
 	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
@@ -69,8 +70,8 @@ export const entry = {
 		}
 		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
@@ -80,4 +81,4 @@ export const entry = {
 			removeNewFileMenuEntry('template-picker')
 		}
 	},
-} as Entry
+} as NewMenuEntry

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

@@ -10,9 +10,13 @@ import { t } from '@nextcloud/l10n'
  * 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)
+ * @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.')
 	}
 
@@ -26,15 +30,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 })
-		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 })
+			if (isFolder) {
+				return t('files', '"{char}" is not allowed inside a folder name.', { char: error.segment }, { escape })
 			}
-			return t('files', 'Filenames must not end with "{extension}".', { extension: error.segment }, undefined, { escape: false })
+			return t('files', '"{char}" is not allowed inside a filename.', { char: error.segment }, { escape })
+		case InvalidFilenameErrorReason.ReservedName:
+			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 (!isFolder && error.segment.match(/\.[a-z]/i)) {
+				return t('files', '"{extension}" is not an allowed filetype.', { extension: error.segment }, { 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

@@ -3,12 +3,12 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import type { Node } from '@nextcloud/files'
+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,21 +17,26 @@ 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: Node[], labels: ILabels = {}) {
-	const contentNames = folderContent.map((node: Node) => node.basename)
+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) => {