Skip to content

Commit 3c57d9c

Browse files
AndyScherzingerAndyScherzinger
authored
Merge pull request #50815 from nextcloud/backport/29-openfile
[stable29] fix(files): Do not download files with `openfile` query flag
2 parents 50d5aed + 0ad0054 commit 3c57d9c

9 files changed

Lines changed: 323 additions & 41 deletions

File tree

‎apps/files/src/components/FilesListVirtual.vue‎

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,10 @@
7070
</template>
7171

7272
<script lang="ts">
73+
import type { UserConfig } from '../types.ts'
7374
import type { Node as NcNode } from '@nextcloud/files'
7475
import type { PropType } from 'vue'
75-
import type { UserConfig } from '../types'
76+
import type { Location } from 'vue-router'
7677
7778
import { getFileListHeaders, Folder, View, getFileActions, FileType } from '@nextcloud/files'
7879
import { showError } from '@nextcloud/dialogs'
@@ -296,30 +297,45 @@ export default defineComponent({
296297
* Handle opening a file (e.g. by ?openfile=true)
297298
* @param fileId File to open
298299
*/
299-
handleOpenFile(fileId: number|null) {
300-
if (fileId === null) {
301-
return
302-
}
303-
300+
async handleOpenFile(fileId: number) {
304301
const node = this.nodes.find(n => n.fileid === fileId) as NcNode
305-
if (node === undefined || node.type === FileType.Folder) {
302+
if (node === undefined) {
306303
return
307304
}
308305
309-
logger.debug('Opening file ' + node.path, { node })
310-
this.openFileId = fileId
311-
const defaultAction = getFileActions()
312-
// Get only default actions (visible and hidden)
313-
.filter(action => !!action?.default)
314-
// Find actions that are either always enabled or enabled for the current node
315-
.filter((action) => !action.enabled || action.enabled([node], this.currentView))
316-
// Sort enabled default actions by order
317-
.sort((a, b) => (a.order || 0) - (b.order || 0))
318-
// Get the first one
319-
.at(0)
320-
// Some file types do not have a default action (e.g. they can only be downloaded)
321-
// So if there is an enabled default action, so execute it
322-
defaultAction?.exec(node, this.currentView, this.currentFolder.path)
306+
if (node.type === FileType.File) {
307+
const defaultAction = getFileActions()
308+
// Get only default actions (visible and hidden)
309+
.filter((action) => !!action?.default)
310+
// Find actions that are either always enabled or enabled for the current node
311+
.filter((action) => !action.enabled || action.enabled([node], this.currentView))
312+
.filter((action) => action.id !== 'download')
313+
// Sort enabled default actions by order
314+
.sort((a, b) => (a.order || 0) - (b.order || 0))
315+
// Get the first one
316+
.at(0)
317+
318+
// Some file types do not have a default action (e.g. they can only be downloaded)
319+
// So if there is an enabled default action, so execute it
320+
if (defaultAction) {
321+
logger.debug('Opening file ' + node.path, { node })
322+
return await defaultAction.exec(node, this.currentView, this.currentFolder.path)
323+
}
324+
}
325+
// The file is either a folder or has no default action other than downloading
326+
// in this case we need to open the details instead and remove the route from the history
327+
const query = this.$route.query
328+
delete query.openfile
329+
query.opendetails = ''
330+
331+
logger.debug('Ignore `openfile` query and replacing with `opendetails` for ' + node.path, { node })
332+
await this.$router.replace({
333+
...(this.$route as Location),
334+
query,
335+
})
336+
// Remove if we backport https://github.com/nextcloud/server/pull/49432 to Nextcloud 30
337+
// otherwise this will still set the correct URL and result in the same view with this
338+
this.openSidebarForFile(this.fileId)
323339
},
324340
325341
onDragOver(event: DragEvent) {

‎apps/files/src/router/router.ts‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,41 @@
2020
*
2121
*/
2222
import type { RawLocation, Route } from 'vue-router'
23-
import type { ErrorHandler } from 'vue-router/types/router.d.ts'
24-
2523
import { generateUrl } from '@nextcloud/router'
2624
import queryString from 'query-string'
27-
import Router from 'vue-router'
25+
import Router, { isNavigationFailure, NavigationFailureType } from 'vue-router'
2826
import Vue from 'vue'
27+
import logger from '../logger'
2928

3029
Vue.use(Router)
3130

3231
// Prevent router from throwing errors when we're already on the page we're trying to go to
33-
const originalPush = Router.prototype.push as (to, onComplete?, onAbort?) => Promise<Route>
34-
Router.prototype.push = function push(to: RawLocation, onComplete?: ((route: Route) => void) | undefined, onAbort?: ErrorHandler | undefined): Promise<Route> {
35-
if (onComplete || onAbort) return originalPush.call(this, to, onComplete, onAbort)
36-
return originalPush.call(this, to).catch(err => err)
32+
const originalPush = Router.prototype.push
33+
Router.prototype.push = (function(this: Router, ...args: Parameters<typeof originalPush>) {
34+
if (args.length > 1) {
35+
return originalPush.call(this, ...args)
36+
}
37+
return originalPush.call<Router, [RawLocation], Promise<Route>>(this, args[0]).catch(ignoreDuplicateNavigation)
38+
}) as typeof originalPush
39+
40+
const originalReplace = Router.prototype.replace
41+
Router.prototype.replace = (function(this: Router, ...args: Parameters<typeof originalReplace>) {
42+
if (args.length > 1) {
43+
return originalReplace.call(this, ...args)
44+
}
45+
return originalReplace.call<Router, [RawLocation], Promise<Route>>(this, args[0]).catch(ignoreDuplicateNavigation)
46+
}) as typeof originalReplace
47+
48+
/**
49+
* Ignore duplicated-navigation error but forward real exceptions
50+
* @param error The thrown error
51+
*/
52+
function ignoreDuplicateNavigation(error: unknown): void {
53+
if (isNavigationFailure(error, NavigationFailureType.duplicated)) {
54+
logger.debug('Ignoring duplicated navigation from vue-router', { error })
55+
} else {
56+
throw error
57+
}
3758
}
3859

3960
const router = new Router({

‎cypress/e2e/files/router-query.cy.ts‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
/*!
2+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import type { User } from '@nextcloud/cypress'
7+
import { join } from 'path'
8+
import { getRowForFileId } from './FilesUtils.ts'
9+
10+
/**
11+
* Check that the sidebar is opened for a specific file
12+
* @param name The name of the file
13+
*/
14+
function sidebarIsOpen(name: string): void {
15+
cy.get('[data-cy-sidebar]')
16+
.should('be.visible')
17+
.findByRole('heading', { name })
18+
.should('be.visible')
19+
}
20+
21+
/**
22+
* Skip a test without viewer installed
23+
*/
24+
function skipIfViewerDisabled(this: Mocha.Context): void {
25+
cy.runOccCommand('app:list --enabled --output json')
26+
.then((exec) => exec.stdout)
27+
.then((output) => JSON.parse(output))
28+
.then((obj) => 'viewer' in obj.enabled)
29+
.then((enabled) => {
30+
if (!enabled) {
31+
this.skip()
32+
}
33+
})
34+
}
35+
36+
/**
37+
* Check a file was not downloaded
38+
* @param filename The expected filename
39+
*/
40+
function fileNotDownloaded(filename: string): void {
41+
const downloadsFolder = Cypress.config('downloadsFolder')
42+
cy.readFile(join(downloadsFolder, filename)).should('not.exist')
43+
}
44+
45+
describe('Check router query flags:', function() {
46+
let user: User
47+
let imageId: number
48+
let archiveId: number
49+
let folderId: number
50+
51+
before(() => {
52+
cy.createRandomUser().then(($user) => {
53+
user = $user
54+
cy.uploadFile(user, 'image.jpg')
55+
.then((response) => { imageId = Number.parseInt(response.headers['oc-fileid']) })
56+
cy.mkdir(user, '/folder')
57+
.then((response) => { folderId = Number.parseInt(response.headers['oc-fileid']) })
58+
cy.uploadContent(user, new Blob([]), 'application/zstd', '/archive.zst')
59+
.then((response) => { archiveId = Number.parseInt(response.headers['oc-fileid']) })
60+
cy.login(user)
61+
})
62+
})
63+
64+
describe('"opendetails"', () => {
65+
it('open details for known file type', () => {
66+
cy.visit(`/apps/files/files/${imageId}?opendetails`)
67+
68+
// see sidebar
69+
sidebarIsOpen('image.jpg')
70+
71+
// but no viewer
72+
cy.findByRole('dialog', { name: 'image.jpg' })
73+
.should('not.exist')
74+
75+
// and no download
76+
fileNotDownloaded('image.jpg')
77+
})
78+
79+
it('open details for unknown file type', () => {
80+
cy.visit(`/apps/files/files/${archiveId}?opendetails`)
81+
82+
// see sidebar
83+
sidebarIsOpen('archive.zst')
84+
85+
// but no viewer
86+
cy.findByRole('dialog', { name: 'archive.zst' })
87+
.should('not.exist')
88+
89+
// and no download
90+
fileNotDownloaded('archive.zst')
91+
})
92+
93+
it('open details for folder', () => {
94+
cy.visit(`/apps/files/files/${folderId}?opendetails`)
95+
96+
// see sidebar
97+
sidebarIsOpen('folder')
98+
99+
// but no viewer
100+
cy.findByRole('dialog', { name: 'folder' })
101+
.should('not.exist')
102+
103+
// and no download
104+
fileNotDownloaded('folder')
105+
})
106+
})
107+
108+
describe('"openfile"', function() {
109+
/** Check the viewer is open and shows the image */
110+
function viewerShowsImage(): void {
111+
cy.findByRole('dialog', { name: 'image.jpg' })
112+
.should('be.visible')
113+
.find(`img[src*="fileId=${imageId}"]`)
114+
.should('be.visible')
115+
}
116+
117+
it('opens files with default action', function() {
118+
skipIfViewerDisabled.call(this)
119+
120+
cy.visit(`/apps/files/files/${imageId}?openfile`)
121+
viewerShowsImage()
122+
})
123+
124+
it('opens files with default action using explicit query state', function() {
125+
skipIfViewerDisabled.call(this)
126+
127+
cy.visit(`/apps/files/files/${imageId}?openfile=true`)
128+
viewerShowsImage()
129+
})
130+
131+
it('does not open files with default action when using explicitly query value `false`', function() {
132+
skipIfViewerDisabled.call(this)
133+
134+
cy.visit(`/apps/files/files/${imageId}?openfile=false`)
135+
getRowForFileId(imageId)
136+
.should('be.visible')
137+
.and('have.class', 'files-list__row--active')
138+
139+
cy.findByRole('dialog', { name: 'image.jpg' })
140+
.should('not.exist')
141+
})
142+
143+
it('does not open folders but shows details', () => {
144+
cy.visit(`/apps/files/files/${folderId}?openfile`)
145+
146+
// See the URL was replaced
147+
cy.url()
148+
.should('match', /[?&]opendetails(&|=|$)/)
149+
.and('not.match', /openfile/)
150+
151+
// See the sidebar is correctly opened
152+
cy.get('[data-cy-sidebar]')
153+
.should('be.visible')
154+
.findByRole('heading', { name: 'folder' })
155+
.should('be.visible')
156+
157+
// see the folder was not changed
158+
getRowForFileId(imageId).should('exist')
159+
})
160+
161+
it('does not open unknown file types but shows details', () => {
162+
cy.visit(`/apps/files/files/${archiveId}?openfile`)
163+
164+
// See the URL was replaced
165+
cy.url()
166+
.should('match', /[?&]opendetails(&|=|$)/)
167+
.and('not.match', /openfile/)
168+
169+
// See the sidebar is correctly opened
170+
cy.get('[data-cy-sidebar]')
171+
.should('be.visible')
172+
.findByRole('heading', { name: 'archive.zst' })
173+
.should('be.visible')
174+
175+
// See no file was downloaded
176+
const downloadsFolder = Cypress.config('downloadsFolder')
177+
cy.readFile(join(downloadsFolder, 'archive.zst')).should('not.exist')
178+
})
179+
})
180+
})

‎cypress/support/commands.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,12 @@ Cypress.Commands.add('enableUser', (user: User, enable = true) => {
130130
*/
131131
Cypress.Commands.add('uploadFile', (user, fixture = 'image.jpg', mimeType = 'image/jpeg', target = `/${fixture}`) => {
132132
// get fixture
133-
return cy.fixture(fixture, 'base64').then(async file => {
134-
// convert the base64 string to a blob
135-
const blob = Cypress.Blob.base64StringToBlob(file, mimeType)
136-
137-
cy.uploadContent(user, blob, mimeType, target)
138-
})
133+
return cy.fixture(fixture, 'base64')
134+
.then((file) => (
135+
// convert the base64 string to a blob
136+
Cypress.Blob.base64StringToBlob(file, mimeType)
137+
))
138+
.then((blob) => cy.uploadContent(user, blob, mimeType, target))
139139
})
140140

141141
Cypress.Commands.add('setFileAsFavorite', (user: User, target: string, favorite = true) => {
@@ -174,7 +174,7 @@ Cypress.Commands.add('setFileAsFavorite', (user: User, target: string, favorite
174174

175175
Cypress.Commands.add('mkdir', (user: User, target: string) => {
176176
// eslint-disable-next-line cypress/unsafe-to-chain-command
177-
cy.clearCookies()
177+
return cy.clearCookies()
178178
.then(async () => {
179179
try {
180180
const rootPath = `${Cypress.env('baseUrl')}/remote.php/dav/files/${encodeURIComponent(user.userId)}`
@@ -188,6 +188,7 @@ Cypress.Commands.add('mkdir', (user: User, target: string) => {
188188
},
189189
})
190190
cy.log(`Created directory ${target}`, response)
191+
return response
191192
} catch (error) {
192193
cy.log('error', error)
193194
throw new Error('Unable to create directory')

0 commit comments

Comments
 (0)