Skip to content

Commit ee144b5

Browse files
susnuxbackportbot[bot]
authored andcommitted
fix(files): distinguish between files and folders for validation messages
Unit tests are AI assisted. Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent ff0d125 commit ee144b5

7 files changed

Lines changed: 147 additions & 18 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ export default defineComponent({
190190
return
191191
}
192192
193-
let validity = getFilenameValidity(newName)
193+
let validity = getFilenameValidity(newName, false, this.source.type === FileType.Folder)
194194
// Checking if already exists
195195
if (validity === '' && this.checkIfNodeExists(newName)) {
196196
validity = t('files', 'Another entry with the same name already exists.')

apps/files/src/components/NewNodeDialog.vue

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,14 @@ const props = defineProps({
8989
type: String,
9090
default: t('files', 'Folder name'),
9191
},
92+
93+
/**
94+
* Whether the name is for a folder, which affects the validation of the name. Defaults to false.
95+
*/
96+
isFolder: {
97+
type: Boolean,
98+
default: false,
99+
},
92100
})
93101
94102
const emit = defineEmits<{
@@ -142,7 +150,7 @@ watchEffect(() => {
142150
if (props.otherNames.includes(localDefaultName.value.trim())) {
143151
validity.value = t('files', 'This name is already in use.')
144152
} else {
145-
validity.value = getFilenameValidity(localDefaultName.value.trim())
153+
validity.value = getFilenameValidity(localDefaultName.value.trim(), false, props.isFolder)
146154
}
147155
const input = nameInput.value?.$el.querySelector('input')
148156
if (input) {

apps/files/src/newMenu/newFolder.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export const entry: NewMenuEntry = {
2929
},
3030

3131
async handler(context: IFolder, content: INode[]) {
32-
const name = await newNodeName(t('files', 'New folder'), content)
32+
const name = await newNodeName(t('files', 'New folder'), content, { isFolder: true })
3333
if (name === null) {
3434
return
3535
}

apps/files/src/newMenu/newTemplatesFolder.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* SPDX-License-Identifier: AGPL-3.0-or-later
44
*/
55

6-
import type { Folder, NewMenuEntry, Node } from '@nextcloud/files'
6+
import type { IFolder, INode, NewMenuEntry } from '@nextcloud/files'
77

88
import PlusSvg from '@mdi/svg/svg/plus.svg?raw'
99
import { getCurrentUser } from '@nextcloud/auth'
@@ -28,7 +28,7 @@ logger.debug('Initial templates folder', { templatesPath })
2828
* @param directory Folder where to create the templates folder
2929
* @param name Name to use or the templates folder
3030
*/
31-
async function initTemplatesFolder(directory: Folder, name: string) {
31+
async function initTemplatesFolder(directory: IFolder, name: string) {
3232
const templatePath = join(directory.path, name)
3333
try {
3434
logger.debug('Initializing the templates directory', { templatePath })
@@ -59,7 +59,7 @@ export const entry: NewMenuEntry = {
5959
displayName: t('files', 'Create templates folder'),
6060
iconSvgInline: PlusSvg,
6161
order: 30,
62-
enabled(context: Folder): boolean {
62+
enabled(context: IFolder): boolean {
6363
// Templates disabled or templates folder already initialized
6464
if (!templatesEnabled || templatesPath) {
6565
return false
@@ -70,8 +70,8 @@ export const entry: NewMenuEntry = {
7070
}
7171
return (context.permissions & Permission.CREATE) !== 0
7272
},
73-
async handler(context: Folder, content: Node[]) {
74-
const name = await newNodeName(t('files', 'Templates'), content, { name: t('files', 'New template folder') })
73+
async handler(context: IFolder, content: INode[]) {
74+
const name = await newNodeName(t('files', 'Templates'), content, { name: t('files', 'New template folder'), isFolder: true })
7575

7676
if (name !== null) {
7777
// Create the template folder
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*!
2+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import { describe, expect, it, vi } from 'vitest'
7+
import { getFilenameValidity } from './filenameValidity.ts'
8+
9+
vi.mock('@nextcloud/capabilities', () => ({
10+
getCapabilities: () => ({
11+
files: {
12+
forbidden_filename_characters: ['/', '\\', '>'],
13+
forbidden_filenames: ['.htaccess'],
14+
forbidden_filename_basenames: ['con'],
15+
forbidden_filename_extensions: ['.exe', '.~'],
16+
},
17+
}),
18+
}))
19+
20+
describe('getFilenameValidity', () => {
21+
it('returns no error for a valid filename', () => {
22+
expect(getFilenameValidity('valid-name.txt')).toBe('')
23+
})
24+
25+
describe('empty name', () => {
26+
it('reports empty filename', () => {
27+
expect(getFilenameValidity('')).toBe('Filename must not be empty.')
28+
})
29+
30+
it('reports empty filename for whitespace only', () => {
31+
expect(getFilenameValidity(' ')).toBe('Filename must not be empty.')
32+
})
33+
34+
it('reports empty folder name when isFolder is set', () => {
35+
expect(getFilenameValidity('', false, true)).toBe('Folder name must not be empty.')
36+
})
37+
})
38+
39+
describe('forbidden character', () => {
40+
it('reports forbidden character for a filename', () => {
41+
expect(getFilenameValidity('inva/lid')).toBe('"/" is not allowed inside a filename.')
42+
})
43+
44+
it('reports forbidden character for a folder name', () => {
45+
expect(getFilenameValidity('inva/lid', false, true)).toBe('"/" is not allowed inside a folder name.')
46+
})
47+
})
48+
49+
describe('reserved name', () => {
50+
it('reports a reserved filename', () => {
51+
expect(getFilenameValidity('.htaccess')).toBe('".htaccess" is a reserved name and not allowed for filenames.')
52+
})
53+
54+
it('reports a reserved folder name', () => {
55+
expect(getFilenameValidity('.htaccess', false, true)).toBe('".htaccess" is a reserved name and not allowed for folder names.')
56+
})
57+
58+
it('reports a reserved basename', () => {
59+
expect(getFilenameValidity('con.txt')).toBe('"con" is a reserved name and not allowed for filenames.')
60+
})
61+
})
62+
63+
describe('forbidden extension', () => {
64+
it('reports a disallowed filetype when the extension looks like a real type', () => {
65+
expect(getFilenameValidity('virus.exe')).toBe('".exe" is not an allowed filetype.')
66+
})
67+
68+
it('reports the extension as not allowed at the end of a filename when it is not a recognisable filetype', () => {
69+
// '.~' does not match /\.[a-z]/i, so the generic "must not end with" message is used.
70+
expect(getFilenameValidity('document.~')).toBe('Filenames must not end with ".~".')
71+
})
72+
73+
it('reports the extension as not allowed for a folder name', () => {
74+
expect(getFilenameValidity('folder.exe', false, true)).toBe('Folder names must not end with ".exe".')
75+
})
76+
})
77+
78+
describe('escape option', () => {
79+
it('does not affect the returned string for safe characters', () => {
80+
expect(getFilenameValidity('inva/lid', true)).toBe('"/" is not allowed inside a filename.')
81+
})
82+
83+
it('escapes the matched character when requested', () => {
84+
expect(getFilenameValidity('inva>lid', true)).toBe('"&gt;" is not allowed inside a filename.')
85+
})
86+
87+
it('does not escape the matched character by default', () => {
88+
expect(getFilenameValidity('inva>lid')).toBe('">" is not allowed inside a filename.')
89+
})
90+
})
91+
92+
it('rethrows errors that are not InvalidFilenameError', async () => {
93+
const files = await import('@nextcloud/files')
94+
const spy = vi.spyOn(files, 'validateFilename').mockImplementation(() => {
95+
throw new Error('unexpected')
96+
})
97+
expect(() => getFilenameValidity('anything')).toThrow('unexpected')
98+
spy.mockRestore()
99+
})
100+
})

apps/files/src/utils/filenameValidity.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,13 @@ import { t } from '@nextcloud/l10n'
1111
*
1212
* @param name The filename
1313
* @param escape Escape the matched string in the error (only set when used in HTML)
14+
* @param isFolder Whether the filename is for a folder
1415
*/
15-
export function getFilenameValidity(name: string, escape = false): string {
16+
export function getFilenameValidity(name: string, escape = false, isFolder = false): string {
1617
if (name.trim() === '') {
18+
if (isFolder) {
19+
return t('files', 'Folder name must not be empty.')
20+
}
1721
return t('files', 'Filename must not be empty.')
1822
}
1923

@@ -27,15 +31,27 @@ export function getFilenameValidity(name: string, escape = false): string {
2731

2832
switch (error.reason) {
2933
case InvalidFilenameErrorReason.Character:
30-
return t('files', '"{char}" is not allowed inside a filename.', { char: error.segment }, undefined, { escape })
34+
if (isFolder) {
35+
return t('files', '"{char}" is not allowed inside a folder name.', { char: error.segment }, { escape })
36+
}
37+
return t('files', '"{char}" is not allowed inside a filename.', { char: error.segment }, { escape })
3138
case InvalidFilenameErrorReason.ReservedName:
32-
return t('files', '"{segment}" is a reserved name and not allowed for filenames.', { segment: error.segment }, undefined, { escape: false })
39+
if (isFolder) {
40+
return t('files', '"{segment}" is a reserved name and not allowed for folder names.', { segment: error.segment }, { escape: false })
41+
}
42+
return t('files', '"{segment}" is a reserved name and not allowed for filenames.', { segment: error.segment }, { escape: false })
3343
case InvalidFilenameErrorReason.Extension:
34-
if (error.segment.match(/\.[a-z]/i)) {
35-
return t('files', '"{extension}" is not an allowed filetype.', { extension: error.segment }, undefined, { escape: false })
44+
if (!isFolder && error.segment.match(/\.[a-z]/i)) {
45+
return t('files', '"{extension}" is not an allowed filetype.', { extension: error.segment }, { escape: false })
3646
}
37-
return t('files', 'Filenames must not end with "{extension}".', { extension: error.segment }, undefined, { escape: false })
47+
if (isFolder) {
48+
return t('files', 'Folder names must not end with "{extension}".', { extension: error.segment }, { escape: false })
49+
}
50+
return t('files', 'Filenames must not end with "{extension}".', { extension: error.segment }, { escape: false })
3851
default:
52+
if (isFolder) {
53+
return t('files', 'Invalid folder name.')
54+
}
3955
return t('files', 'Invalid filename.')
4056
}
4157
}

apps/files/src/utils/newNodeDialog.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import type { INode } from '@nextcloud/files'
88
import { spawnDialog } from '@nextcloud/vue/functions/dialog'
99
import NewNodeDialog from '../components/NewNodeDialog.vue'
1010

11-
interface ILabels {
11+
interface NewNodeDialogOptions {
1212
/**
1313
* Dialog heading, defaults to "New folder name"
1414
*/
@@ -17,22 +17,27 @@ interface ILabels {
1717
* Label for input box, defaults to "New folder"
1818
*/
1919
label?: string
20+
21+
/**
22+
* Whether the name is for a folder, defaults to false.
23+
*/
24+
isFolder?: boolean
2025
}
2126

2227
/**
2328
* Ask user for file or folder name
2429
*
2530
* @param defaultName Default name to use
2631
* @param folderContent Nodes with in the current folder to check for unique name
27-
* @param labels Labels to set on the dialog
32+
* @param options Options for the dialog
2833
* @return string if successful otherwise null if aborted
2934
*/
30-
export function newNodeName(defaultName: string, folderContent: INode[], labels: ILabels = {}) {
35+
export function newNodeName(defaultName: string, folderContent: INode[], options: NewNodeDialogOptions = {}) {
3136
const contentNames = folderContent.map((node: INode) => node.basename)
3237

3338
return new Promise<string | null>((resolve) => {
3439
spawnDialog(NewNodeDialog, {
35-
...labels,
40+
...options,
3641
defaultName,
3742
otherNames: contentNames,
3843
}, (folderName) => {

0 commit comments

Comments
 (0)