diff options
author | John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com> | 2025-08-07 11:31:14 +0200 |
---|---|---|
committer | John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com> | 2025-08-07 14:54:37 +0200 |
commit | bb82dd4e8b48c30cb667da28e00f21d8c5b489a6 (patch) | |
tree | 0ca4ec15872ff943404a7d23f942eb00f072f1ce | |
parent | f0c392e21c930ecba18f5f31b93e8296082bade8 (diff) | |
download | nextcloud-server-fix/download-action.tar.gz nextcloud-server-fix/download-action.zip |
fix(files): verify files are still accessible before downloadingfix/download-action
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
-rw-r--r-- | __mocks__/@nextcloud/axios.ts | 1 | ||||
-rw-r--r-- | apps/files/src/actions/downloadAction.spec.ts | 50 | ||||
-rw-r--r-- | apps/files/src/actions/downloadAction.ts | 69 | ||||
-rw-r--r-- | apps/files/src/views/Navigation.cy.ts | 3 |
4 files changed, 115 insertions, 8 deletions
diff --git a/__mocks__/@nextcloud/axios.ts b/__mocks__/@nextcloud/axios.ts index 5133574d9ed..61450985118 100644 --- a/__mocks__/@nextcloud/axios.ts +++ b/__mocks__/@nextcloud/axios.ts @@ -14,4 +14,5 @@ export default { get: async () => ({ status: 200, data: {} }), delete: async () => ({ status: 200, data: {} }), post: async () => ({ status: 200, data: {} }), + head: async () => ({ status: 200, data: {} }), } diff --git a/apps/files/src/actions/downloadAction.spec.ts b/apps/files/src/actions/downloadAction.spec.ts index 8d5612d982b..cd853d4a7ae 100644 --- a/apps/files/src/actions/downloadAction.spec.ts +++ b/apps/files/src/actions/downloadAction.spec.ts @@ -7,6 +7,14 @@ import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest' import { action } from './downloadAction' +import axios from '@nextcloud/axios' +import * as dialogs from '@nextcloud/dialogs' +import * as eventBus from '@nextcloud/event-bus' + +vi.mock('@nextcloud/axios') +vi.mock('@nextcloud/dialogs') +vi.mock('@nextcloud/event-bus') + const view = { id: 'files', name: 'Files', @@ -188,4 +196,46 @@ describe('Download action execute tests', () => { expect(link.href).toMatch('https://cloud.domain.com/remote.php/dav/files/admin/Dir/?accept=zip&files=%5B%22foo.txt%22%2C%22bar.txt%22%5D') expect(link.click).toHaveBeenCalledTimes(1) }) + + test('Download fails with error', async () => { + const file = new File({ + id: 1, + source: 'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt', + owner: 'admin', + mime: 'text/plain', + permissions: Permission.READ, + }) + vi.spyOn(axios, 'head').mockRejectedValue(new Error('File not found')) + + const errorSpy = vi.spyOn(dialogs, 'showError') + const exec = await action.exec(file, view, '/') + expect(exec).toBe(null) + expect(errorSpy).toHaveBeenCalledWith('The requested file is not available.') + expect(link.click).not.toHaveBeenCalled() + }) + + test('Download batch fails with error', async () => { + const file1 = new File({ + id: 1, + source: 'https://cloud.domain.com/remote.php/dav/files/admin/foo.txt', + owner: 'admin', + mime: 'text/plain', + permissions: Permission.READ, + }) + const file2 = new File({ + id: 2, + source: 'https://cloud.domain.com/remote.php/dav/files/admin/bar.txt', + owner: 'admin', + mime: 'text/plain', + permissions: Permission.READ, + }) + vi.spyOn(axios, 'head').mockRejectedValue(new Error('File not found')) + vi.spyOn(eventBus, 'emit').mockImplementation(() => {}) + + const errorSpy = vi.spyOn(dialogs, 'showError') + const exec = await action.execBatch!([file1, file2], view, '/') + expect(exec).toStrictEqual([null, null]) + expect(errorSpy).toHaveBeenCalledWith('The requested files are not available.') + expect(link.click).not.toHaveBeenCalled() + }) }) diff --git a/apps/files/src/actions/downloadAction.ts b/apps/files/src/actions/downloadAction.ts index 8abd87972ee..f705e5f4430 100644 --- a/apps/files/src/actions/downloadAction.ts +++ b/apps/files/src/actions/downloadAction.ts @@ -4,18 +4,28 @@ */ import type { Node, View } from '@nextcloud/files' import { FileAction, FileType, DefaultType } from '@nextcloud/files' +import { showError } from '@nextcloud/dialogs' import { t } from '@nextcloud/l10n' -import { isDownloadable } from '../utils/permissions' +import axios from '@nextcloud/axios' import ArrowDownSvg from '@mdi/svg/svg/arrow-down.svg?raw' +import { isDownloadable } from '../utils/permissions' +import { usePathsStore } from '../store/paths' +import { getPinia } from '../store' +import { useFilesStore } from '../store/files' +import { emit } from '@nextcloud/event-bus' + /** * Trigger downloading a file. * * @param url The url of the asset to download * @param name Optionally the recommended name of the download (browsers might ignore it) */ -function triggerDownload(url: string, name?: string) { +async function triggerDownload(url: string, name?: string) { + // try to see if the resource is still available + await axios.head(url) + const hiddenElement = document.createElement('a') hiddenElement.download = name ?? '' hiddenElement.href = url @@ -44,12 +54,21 @@ function longestCommonPath(first: string, second: string): string { return base } -const downloadNodes = function(nodes: Node[]) { +/** + * Download the given nodes. + * + * If only one node is given, it will be downloaded directly. + * If multiple nodes are given, they will be zipped and downloaded. + * + * @param nodes The node(s) to download + */ +async function downloadNodes(nodes: Node[]) { let url: URL if (nodes.length === 1) { if (nodes[0].type === FileType.File) { - return triggerDownload(nodes[0].encodedSource, nodes[0].displayname) + await triggerDownload(nodes[0].encodedSource, nodes[0].displayname) + return } else { url = new URL(nodes[0].encodedSource) url.searchParams.append('accept', 'zip') @@ -72,7 +91,29 @@ const downloadNodes = function(nodes: Node[]) { url.pathname = `${url.pathname}/` } - return triggerDownload(url.href) + await triggerDownload(url.href) +} + +/** + * Get the current directory node for the given view and path. + * TODO: ideally the folder would directly be passed as exec params + * + * @param view The current view + * @param directory The directory path + * @return The current directory node or null if not found + */ +function getCurrentDirectory(view: View, directory: string): Node | null { + const filesStore = useFilesStore(getPinia()) + const pathsStore = usePathsStore(getPinia()) + if (!view?.id) { + return null + } + + if (directory === '/') { + return filesStore.getRoot(view.id) || null + } + const fileId = pathsStore.getPath(view.id, directory)! + return filesStore.getNode(fileId) || null } export const action = new FileAction({ @@ -101,12 +142,24 @@ export const action = new FileAction({ }, async exec(node: Node) { - downloadNodes([node]) + try { + await downloadNodes([node]) + } catch (e) { + showError(t('files', 'The requested file is not available.')) + emit('files:node:deleted', node) + } return null }, - async execBatch(nodes: Node[]) { - downloadNodes(nodes) + async execBatch(nodes: Node[], view: View, dir: string) { + try { + await downloadNodes(nodes) + } catch (e) { + showError(t('files', 'The requested files are not available.')) + // Try to reload the current directory to update the view + const directory = getCurrentDirectory(view, dir)! + emit('files:node:updated', directory) + } return new Array(nodes.length).fill(null) }, diff --git a/apps/files/src/views/Navigation.cy.ts b/apps/files/src/views/Navigation.cy.ts index 7357943ee28..8dd0573201d 100644 --- a/apps/files/src/views/Navigation.cy.ts +++ b/apps/files/src/views/Navigation.cy.ts @@ -28,6 +28,9 @@ const createView = (id: string, name: string, parent?: string) => new View({ parent, }) +/** + * + */ function mockWindow() { window.OCP ??= {} window.OCP.Files ??= {} |