Skip to content

Commit bb82dd4

Browse files
committed
fix(files): verify files are still accessible before downloading
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
1 parent f0c392e commit bb82dd4

File tree

4 files changed

+115
-8
lines changed

4 files changed

+115
-8
lines changed

__mocks__/@nextcloud/axios.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,5 @@ export default {
1414
get: async () => ({ status: 200, data: {} }),
1515
delete: async () => ({ status: 200, data: {} }),
1616
post: async () => ({ status: 200, data: {} }),
17+
head: async () => ({ status: 200, data: {} }),
1718
}

apps/files/src/actions/downloadAction.spec.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@ import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'
77

88
import { action } from './downloadAction'
99

10+
import axios from '@nextcloud/axios'
11+
import * as dialogs from '@nextcloud/dialogs'
12+
import * as eventBus from '@nextcloud/event-bus'
13+
14+
vi.mock('@nextcloud/axios')
15+
vi.mock('@nextcloud/dialogs')
16+
vi.mock('@nextcloud/event-bus')
17+
1018
const view = {
1119
id: 'files',
1220
name: 'Files',
@@ -188,4 +196,46 @@ describe('Download action execute tests', () => {
188196
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')
189197
expect(link.click).toHaveBeenCalledTimes(1)
190198
})
199+
200+
test('Download fails with error', async () => {
201+
const file = new File({
202+
id: 1,
203+
source: 'https://cloud.domain.com/remote.php/dav/files/admin/foobar.txt',
204+
owner: 'admin',
205+
mime: 'text/plain',
206+
permissions: Permission.READ,
207+
})
208+
vi.spyOn(axios, 'head').mockRejectedValue(new Error('File not found'))
209+
210+
const errorSpy = vi.spyOn(dialogs, 'showError')
211+
const exec = await action.exec(file, view, '/')
212+
expect(exec).toBe(null)
213+
expect(errorSpy).toHaveBeenCalledWith('The requested file is not available.')
214+
expect(link.click).not.toHaveBeenCalled()
215+
})
216+
217+
test('Download batch fails with error', async () => {
218+
const file1 = new File({
219+
id: 1,
220+
source: 'https://cloud.domain.com/remote.php/dav/files/admin/foo.txt',
221+
owner: 'admin',
222+
mime: 'text/plain',
223+
permissions: Permission.READ,
224+
})
225+
const file2 = new File({
226+
id: 2,
227+
source: 'https://cloud.domain.com/remote.php/dav/files/admin/bar.txt',
228+
owner: 'admin',
229+
mime: 'text/plain',
230+
permissions: Permission.READ,
231+
})
232+
vi.spyOn(axios, 'head').mockRejectedValue(new Error('File not found'))
233+
vi.spyOn(eventBus, 'emit').mockImplementation(() => {})
234+
235+
const errorSpy = vi.spyOn(dialogs, 'showError')
236+
const exec = await action.execBatch!([file1, file2], view, '/')
237+
expect(exec).toStrictEqual([null, null])
238+
expect(errorSpy).toHaveBeenCalledWith('The requested files are not available.')
239+
expect(link.click).not.toHaveBeenCalled()
240+
})
191241
})

apps/files/src/actions/downloadAction.ts

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,28 @@
44
*/
55
import type { Node, View } from '@nextcloud/files'
66
import { FileAction, FileType, DefaultType } from '@nextcloud/files'
7+
import { showError } from '@nextcloud/dialogs'
78
import { t } from '@nextcloud/l10n'
8-
import { isDownloadable } from '../utils/permissions'
9+
import axios from '@nextcloud/axios'
910

1011
import ArrowDownSvg from '@mdi/svg/svg/arrow-down.svg?raw'
1112

13+
import { isDownloadable } from '../utils/permissions'
14+
import { usePathsStore } from '../store/paths'
15+
import { getPinia } from '../store'
16+
import { useFilesStore } from '../store/files'
17+
import { emit } from '@nextcloud/event-bus'
18+
1219
/**
1320
* Trigger downloading a file.
1421
*
1522
* @param url The url of the asset to download
1623
* @param name Optionally the recommended name of the download (browsers might ignore it)
1724
*/
18-
function triggerDownload(url: string, name?: string) {
25+
async function triggerDownload(url: string, name?: string) {
26+
// try to see if the resource is still available
27+
await axios.head(url)
28+
1929
const hiddenElement = document.createElement('a')
2030
hiddenElement.download = name ?? ''
2131
hiddenElement.href = url
@@ -44,12 +54,21 @@ function longestCommonPath(first: string, second: string): string {
4454
return base
4555
}
4656

47-
const downloadNodes = function(nodes: Node[]) {
57+
/**
58+
* Download the given nodes.
59+
*
60+
* If only one node is given, it will be downloaded directly.
61+
* If multiple nodes are given, they will be zipped and downloaded.
62+
*
63+
* @param nodes The node(s) to download
64+
*/
65+
async function downloadNodes(nodes: Node[]) {
4866
let url: URL
4967

5068
if (nodes.length === 1) {
5169
if (nodes[0].type === FileType.File) {
52-
return triggerDownload(nodes[0].encodedSource, nodes[0].displayname)
70+
await triggerDownload(nodes[0].encodedSource, nodes[0].displayname)
71+
return
5372
} else {
5473
url = new URL(nodes[0].encodedSource)
5574
url.searchParams.append('accept', 'zip')
@@ -72,7 +91,29 @@ const downloadNodes = function(nodes: Node[]) {
7291
url.pathname = `${url.pathname}/`
7392
}
7493

75-
return triggerDownload(url.href)
94+
await triggerDownload(url.href)
95+
}
96+
97+
/**
98+
* Get the current directory node for the given view and path.
99+
* TODO: ideally the folder would directly be passed as exec params
100+
*
101+
* @param view The current view
102+
* @param directory The directory path
103+
* @return The current directory node or null if not found
104+
*/
105+
function getCurrentDirectory(view: View, directory: string): Node | null {
106+
const filesStore = useFilesStore(getPinia())
107+
const pathsStore = usePathsStore(getPinia())
108+
if (!view?.id) {
109+
return null
110+
}
111+
112+
if (directory === '/') {
113+
return filesStore.getRoot(view.id) || null
114+
}
115+
const fileId = pathsStore.getPath(view.id, directory)!
116+
return filesStore.getNode(fileId) || null
76117
}
77118

78119
export const action = new FileAction({
@@ -101,12 +142,24 @@ export const action = new FileAction({
101142
},
102143

103144
async exec(node: Node) {
104-
downloadNodes([node])
145+
try {
146+
await downloadNodes([node])
147+
} catch (e) {
148+
showError(t('files', 'The requested file is not available.'))
149+
emit('files:node:deleted', node)
150+
}
105151
return null
106152
},
107153

108-
async execBatch(nodes: Node[]) {
109-
downloadNodes(nodes)
154+
async execBatch(nodes: Node[], view: View, dir: string) {
155+
try {
156+
await downloadNodes(nodes)
157+
} catch (e) {
158+
showError(t('files', 'The requested files are not available.'))
159+
// Try to reload the current directory to update the view
160+
const directory = getCurrentDirectory(view, dir)!
161+
emit('files:node:updated', directory)
162+
}
110163
return new Array(nodes.length).fill(null)
111164
},
112165

apps/files/src/views/Navigation.cy.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ const createView = (id: string, name: string, parent?: string) => new View({
2828
parent,
2929
})
3030

31+
/**
32+
*
33+
*/
3134
function mockWindow() {
3235
window.OCP ??= {}
3336
window.OCP.Files ??= {}

0 commit comments

Comments
 (0)