]> source.dussan.org Git - nextcloud-server.git/commitdiff
fix(files): Correctly validate new node name
authorFerdinand Thiessen <opensource@fthiessen.de>
Thu, 25 Jul 2024 20:51:12 +0000 (22:51 +0200)
committerFerdinand Thiessen <opensource@fthiessen.de>
Fri, 2 Aug 2024 20:08:16 +0000 (22:08 +0200)
* 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>
apps/files/src/components/FileEntry/FileEntryName.vue
apps/files/src/components/NewNodeDialog.vue
cypress/e2e/files/files-renaming.cy.ts [new file with mode: 0644]

index d157730a3dfce152e88e42ca4f974c168009bccc..9aa3bcfeb8771a57e0b279042a4bd9fc36769808 100644 (file)
@@ -44,7 +44,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" />
@@ -140,7 +140,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]
@@ -190,7 +190,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
                 */
index 0f6a739d21c1b950d763ff3dda7d1e974c3d0274..b4647724de3038d693ced63965e0c558181d0bf0 100644 (file)
 <!--
-  - @copyright Copyright (c) 2024 Ferdinand Thiessen <opensource@fthiessen.de>
-  -
-  - @author Ferdinand Thiessen <opensource@fthiessen.de>
-  -
-  - @license AGPL-3.0-or-later
-  -
-  - This program is free software: you can redistribute it and/or modify
-  - it under the terms of the GNU Affero General Public License as
-  - published by the Free Software Foundation, either version 3 of the
-  - License, or (at your option) any later version.
-  -
-  - This program is distributed in the hope that it will be useful,
-  - but WITHOUT ANY WARRANTY; without even the implied warranty of
-  - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-  - GNU Affero General Public License for more details.
-  -
-  - You should have received a copy of the GNU Affero General Public License
-  - along with this program. If not, see <http://www.gnu.org/licenses/>.
-  -
-  -->
+  - SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+  - SPDX-License-Identifier: AGPL-3.0-or-later
+-->
 <template>
-       <NcDialog :name="name"
+       <NcDialog data-cy-files-new-node-dialog
+               :name="name"
                :open="open"
                close-on-click-outside
                out-transition
-               @update:open="onClose">
+               @update:open="emit('close', null)">
                <template #actions>
-                       <NcButton type="primary"
-                               :disabled="!isUniqueName"
-                               @click="onCreate">
+                       <NcButton data-cy-files-new-node-dialog-submit
+                               type="primary"
+                               :disabled="validity !== ''"
+                               @click="submit">
                                {{ t('files', 'Create') }}
                        </NcButton>
                </template>
-               <form @submit.prevent="onCreate">
-                       <NcTextField ref="input"
-                               class="dialog__input"
-                               :error="!isUniqueName"
-                               :helper-text="errorMessage"
+               <form ref="formElement"
+                       class="new-node-dialog__form"
+                       @submit.prevent="emit('close', localDefaultName)">
+                       <NcTextField ref="nameInput"
+                               data-cy-files-new-node-dialog-input
+                               :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 forbiddenCharacters = loadState<string>('files', 'forbiddenCharacters', '').split('')
-
-export default defineComponent({
-       name: 'NewNodeDialog',
-       components: {
-               NcButton,
-               NcDialog,
-               NcTextField,
+const props = defineProps({
+       /**
+        * The name to be used by default
+        */
+       defaultName: {
+               type: String,
+               default: t('files', 'New folder'),
        },
-       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'),
-               },
+       /**
+        * Other files that are in the current directory
+        */
+       otherNames: {
+               type: Array as PropType<string[]>,
+               default: () => [],
        },
-       emits: {
-               close: (name: string|null) => name === null || name,
+       /**
+        * Open state of the dialog
+        */
+       open: {
+               type: Boolean,
+               default: true,
        },
-       data() {
-               return {
-                       localDefaultName: this.defaultName || t('files', 'New folder'),
-               }
+       /**
+        * Dialog name
+        */
+       name: {
+               type: String,
+               default: t('files', 'Create 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
-               },
+       /**
+        * Input label
+        */
+       label: {
+               type: String,
+               default: t('files', 'Folder name'),
        },
-       watch: {
-               defaultName() {
-                       this.localDefaultName = this.defaultName || t('files', 'New folder')
-               },
+})
 
-               /**
-                * 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,
+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
+               }
 
-               /**
-                * Focus the filename input field
-                */
-               focusInput() {
-                       if (this.open) {
-                               this.$nextTick(() => (this.$refs.input as unknown as ICanFocus)?.focus?.())
-                       }
-               },
+               // 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)
+       })
+}
 
-               onCreate() {
-                       this.$emit('close', this.localDefaultName)
-               },
-               onClose(state: boolean) {
-                       if (!state) {
-                               this.$emit('close', null)
-                       }
-               },
+/**
+ * Trigger submit on the form
+ */
+function submit() {
+       formElement.value?.requestSubmit()
+}
 
-               /**
-                * 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()
-                       }
-               },
+// Reset local name on props change
+watch(() => props.defaultName, () => {
+       localDefaultName.value = getUniqueName(props.defaultName, props.otherNames)
+})
 
-               isFileNameValid(name: string) {
-                       const trimmedName = name.trim()
-                       const char = trimmedName.indexOf('/') !== -1
-                               ? '/'
-                               : forbiddenCharacters.find((char) => trimmedName.includes(char))
+// 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()
+       }
+})
 
-                       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 }))
-                       }
+// Ensure the input is focussed even if the dialog is already mounted but not open
+watch(() => props.open, () => {
+       nextTick(() => {
+               focusInput()
+       })
+})
 
-                       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>
diff --git a/cypress/e2e/files/files-renaming.cy.ts b/cypress/e2e/files/files-renaming.cy.ts
new file mode 100644 (file)
index 0000000..9404c95
--- /dev/null
@@ -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))
+       })
+})