Skip to content

Commit

Permalink
Merge pull request #44407 from nextcloud/fix/files-copy-move-escaping
Browse files Browse the repository at this point in the history
fix(files): Do not escape file names in the file picker
  • Loading branch information
Pytal authored Mar 22, 2024
2 parents c35e237 + 2a8d9e0 commit f3f73ba
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 12 deletions.
4 changes: 2 additions & 2 deletions apps/files/src/actions/moveOrCopyAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ const openFilePickerForAction = async (action: MoveCopyAction, dir = '/', nodes:

if (action === MoveCopyAction.COPY || action === MoveCopyAction.MOVE_OR_COPY) {
buttons.push({
label: target ? t('files', 'Copy to {target}', { target }) : t('files', 'Copy'),
label: target ? t('files', 'Copy to {target}', { target }, undefined, { escape: false, sanitize: false }) : t('files', 'Copy'),
type: 'primary',
icon: CopyIconSvg,
async callback(destination: Node[]) {
Expand All @@ -237,7 +237,7 @@ const openFilePickerForAction = async (action: MoveCopyAction, dir = '/', nodes:

if (action === MoveCopyAction.MOVE || action === MoveCopyAction.MOVE_OR_COPY) {
buttons.push({
label: target ? t('files', 'Move to {target}', { target }) : t('files', 'Move'),
label: target ? t('files', 'Move to {target}', { target }, undefined, { escape: false, sanitize: false }) : t('files', 'Move'),
type: action === MoveCopyAction.MOVE ? 'primary' : 'secondary',
icon: FolderMoveSvg,
async callback(destination: Node[]) {
Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/files/FilesUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export const copyFile = (fileName: string, dirName: string) => {
cy.contains('button', 'Copy').should('be.visible').click()
} else {
// select folder
cy.get(`[data-filename="${dirName}"]`).should('be.visible').click()
cy.get(`[data-filename="${CSS.escape(dirName)}"]`).should('be.visible').click()
// click copy
cy.contains('button', `Copy to ${dirName}`).should('be.visible').click()
}
Expand Down
38 changes: 38 additions & 0 deletions cypress/e2e/files/files_copy-move.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('Files: Move or copy files', { testIsolation: true }, () => {
cy.deleteUser(currentUser)
})


it('Can copy a file to new folder', () => {
// Prepare initial state
cy.uploadContent(currentUser, new Blob(), 'text/plain', '/original.txt')
Expand Down Expand Up @@ -136,4 +137,41 @@ describe('Files: Move or copy files', { testIsolation: true }, () => {
getRowForFile('original.txt').should('be.visible')
getRowForFile('original (copy 2).txt').should('be.visible')
})

/** Test for https://github.com/nextcloud/server/issues/43329 */
context.only('escaping file and folder names', () => {
it('Can handle files with special characters', () => {
cy.uploadContent(currentUser, new Blob(), 'text/plain', '/original.txt')
.mkdir(currentUser, '/can\'t say')
cy.login(currentUser)
cy.visit('/apps/files')

copyFile('original.txt', 'can\'t say')

navigateToFolder('can\'t say')

cy.url().should('contain', 'dir=/can%27t%20say')
getRowForFile('original.txt').should('be.visible')
getRowForFile('can\'t say').should('not.exist')
})

/**
* If escape is set to false (required for test above) then "<a>foo" would result in "<a>foo</a>" if sanitizing is not disabled
* We should disable it as vue already escapes the text when using v-text
*/
it('does not incorrectly sanitize file names', () => {
cy.uploadContent(currentUser, new Blob(), 'text/plain', '/original.txt')
.mkdir(currentUser, '/<a href="#">foo')
cy.login(currentUser)
cy.visit('/apps/files')

copyFile('original.txt', '<a href="#">foo')

navigateToFolder('<a href="#">foo')

cy.url().should('contain', 'dir=/%3Ca%20href%3D%22%23%22%3Efoo')
getRowForFile('original.txt').should('be.visible')
getRowForFile('<a href="#">foo').should('not.exist')
})
})
})
4 changes: 2 additions & 2 deletions dist/core-common.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/core-common.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/files-init.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files-init.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/files-main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files-main.js.map

Large diffs are not rendered by default.

0 comments on commit f3f73ba

Please sign in to comment.