aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJohn Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>2025-08-07 11:31:14 +0200
committerJohn Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>2025-08-07 14:54:37 +0200
commitbb82dd4e8b48c30cb667da28e00f21d8c5b489a6 (patch)
tree0ca4ec15872ff943404a7d23f942eb00f072f1ce
parentf0c392e21c930ecba18f5f31b93e8296082bade8 (diff)
downloadnextcloud-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.ts1
-rw-r--r--apps/files/src/actions/downloadAction.spec.ts50
-rw-r--r--apps/files/src/actions/downloadAction.ts69
-rw-r--r--apps/files/src/views/Navigation.cy.ts3
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 ??= {}