Skip to content

Commit

Permalink
Merge pull request #44416 from nextcloud/backport/44407/stable28
Browse files Browse the repository at this point in the history
[stable28] fix(files): Do not escape file names in the file picker
  • Loading branch information
susnux authored Apr 17, 2024
2 parents 63c9700 + 721521f commit 8c87769
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 26 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
82 changes: 82 additions & 0 deletions cypress/e2e/files/FilesUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,85 @@ export const triggerActionForFile = (filename: string, actionId: string) => {
getActionButtonForFile(filename).click()
cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"] > button`).should('exist').click()
}

export const moveFile = (fileName: string, dirPath: string) => {
getRowForFile(fileName).should('be.visible')
triggerActionForFile(fileName, 'move-copy')

cy.get('.file-picker').within(() => {
// intercept the copy so we can wait for it
cy.intercept('MOVE', /\/remote.php\/dav\/files\//).as('moveFile')

if (dirPath === '/') {
// select home folder
cy.get('button[title="Home"]').should('be.visible').click()
// click move
cy.contains('button', 'Move').should('be.visible').click()
} else if (dirPath === '.') {
// click move
cy.contains('button', 'Copy').should('be.visible').click()
} else {
const directories = dirPath.split('/')
directories.forEach((directory) => {
// select the folder
cy.get(`[data-filename="${directory}"]`).should('be.visible').click()
})

// click move
cy.contains('button', `Move to ${directories.at(-1)}`).should('be.visible').click()
}

cy.wait('@moveFile')
})
}

export const copyFile = (fileName: string, dirPath: string) => {
getRowForFile(fileName).should('be.visible')
triggerActionForFile(fileName, 'move-copy')

cy.get('.file-picker').within(() => {
// intercept the copy so we can wait for it
cy.intercept('COPY', /\/remote.php\/dav\/files\//).as('copyFile')

if (dirPath === '/') {
// select home folder
cy.get('button[title="Home"]').should('be.visible').click()
// click copy
cy.contains('button', 'Copy').should('be.visible').click()
} else if (dirPath === '.') {
// click copy
cy.contains('button', 'Copy').should('be.visible').click()
} else {
const directories = dirPath.split('/')
directories.forEach((directory) => {
// select the folder
cy.get(`[data-filename="${CSS.escape(directory)}"]`).should('be.visible').click()
})

// click copy
cy.contains('button', `Copy to ${directories.at(-1)}`).should('be.visible').click()
}

cy.wait('@copyFile')
})
}

export const renameFile = (fileName: string, newFileName: string) => {
getRowForFile(fileName)
triggerActionForFile(fileName, 'rename')

// intercept the move so we can wait for it
cy.intercept('MOVE', /\/remote.php\/dav\/files\//).as('moveFile')

getRowForFile(fileName).find('[data-cy-files-list-row-name] input').clear()
getRowForFile(fileName).find('[data-cy-files-list-row-name] input').type(`${newFileName}{enter}`)

cy.wait('@moveFile')
}

export const navigateToFolder = (dirPath: string) => {
const directories = dirPath.split('/')
directories.forEach((directory) => {
getRowForFile(directory).should('be.visible').find('[data-cy-files-list-row-name-link]').click()
})
}
57 changes: 39 additions & 18 deletions cypress/e2e/files/files_copy-move.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
*
*/

import { getRowForFile, triggerActionForFile } from './FilesUtils.ts'
import { copyFile, getRowForFile, navigateToFolder, triggerActionForFile } from './FilesUtils.ts'

describe('Files: Move or copy files', { testIsolation: true }, () => {
let currentUser
Expand Down Expand Up @@ -162,16 +162,8 @@ describe('Files: Move or copy files', { testIsolation: true }, () => {
cy.login(currentUser)
cy.visit('/apps/files')

// intercept the copy so we can wait for it
cy.intercept('COPY', /\/remote.php\/dav\/files\//).as('copyFile')

getRowForFile('original.txt').should('be.visible')
triggerActionForFile('original.txt', 'move-copy')

// click copy
cy.get('.file-picker').contains('button', 'Copy').should('be.visible').click()
copyFile('original.txt', '.')

cy.wait('@copyFile')
getRowForFile('original.txt').should('be.visible')
getRowForFile('original (copy).txt').should('be.visible')
})
Expand All @@ -182,17 +174,46 @@ describe('Files: Move or copy files', { testIsolation: true }, () => {
cy.login(currentUser)
cy.visit('/apps/files')

// intercept the copy so we can wait for it
cy.intercept('COPY', /\/remote.php\/dav\/files\//).as('copyFile')
copyFile('original.txt', '.')

getRowForFile('original.txt').should('be.visible')
triggerActionForFile('original.txt', 'move-copy')
getRowForFile('original (copy 2).txt').should('be.visible')
})

// click copy
cy.get('.file-picker').contains('button', 'Copy').should('be.visible').click()
/** Test for https://github.com/nextcloud/server/issues/43329 */
context('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')

cy.wait('@copyFile')
getRowForFile('original.txt').should('be.visible')
getRowForFile('original (copy 2).txt').should('be.visible')
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/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 8c87769

Please sign in to comment.