From ef4dc7d588d74880bcea0240afdd9ffc41b465b2 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Sun, 8 Sep 2024 21:17:21 +0200 Subject: [PATCH] fix: Load preview URL for download-disabled shares This was possible on Nextcloud 30 and previous due to a "bug": The `download` permission was simply not rejected for public shares, just a `hide` flag was set on the public share. Now the permissions are correctly set, so loading a preview is not possible. The work-around is to allow previews when the correct header is set. Signed-off-by: Ferdinand Thiessen --- cypress/e2e/download-forbidden.cy.ts | 3 +- .../e2e/sharing/download-share-disabled.cy.ts | 6 +- src/components/Images.vue | 125 ++++++++++-------- src/mixins/Mime.js | 15 ++- src/mixins/PreviewUrl.js | 56 -------- src/utils/canDownload.ts | 4 +- src/utils/fileUtils.ts | 51 +++---- src/utils/livePhotoUtils.ts | 10 +- src/utils/models.ts | 17 --- src/utils/numberUtil.ts | 13 -- src/utils/previewUtils.ts | 62 ++++++--- src/views/Viewer.vue | 3 +- tsconfig.json | 7 +- 13 files changed, 164 insertions(+), 208 deletions(-) delete mode 100644 src/mixins/PreviewUrl.js delete mode 100644 src/utils/models.ts delete mode 100644 src/utils/numberUtil.ts diff --git a/cypress/e2e/download-forbidden.cy.ts b/cypress/e2e/download-forbidden.cy.ts index 2ebef5f87..6fa568469 100644 --- a/cypress/e2e/download-forbidden.cy.ts +++ b/cypress/e2e/download-forbidden.cy.ts @@ -35,8 +35,7 @@ describe('Disable download button if forbidden', { testIsolation: true }, () => .should('contain', 'image1 .jpg') }) - // TODO: Fix no-download files on server - it.skip('See the image can be shown', () => { + it('See the image can be shown', () => { cy.getFile('image1.jpg').should('be.visible') cy.openFile('image1.jpg') cy.get('body > .viewer').should('be.visible') diff --git a/cypress/e2e/sharing/download-share-disabled.cy.ts b/cypress/e2e/sharing/download-share-disabled.cy.ts index ba23b94f8..fa5b08262 100644 --- a/cypress/e2e/sharing/download-share-disabled.cy.ts +++ b/cypress/e2e/sharing/download-share-disabled.cy.ts @@ -93,16 +93,14 @@ describe(`Download ${fileName} in viewer`, function() { cy.get('body > .viewer').should('be.visible') }) - // TODO: FIX DOWNLOAD DISABLED SHARES - it.skip('Does not see a loading animation', function() { + it('Does not see a loading animation', function() { cy.get('body > .viewer', { timeout: 10000 }) .should('be.visible') .and('have.class', 'modal-mask') .and('not.have.class', 'icon-loading') }) - // TODO: FIX DOWNLOAD DISABLED SHARES - it.skip('See the title on the viewer header but not the Download nor the menu button', function() { + it('See the title on the viewer header but not the Download nor the menu button', function() { cy.get('body > .viewer .modal-header__name').should('contain', 'image1.jpg') cy.get('body a[download="image1.jpg"]').should('not.exist') cy.get('body > .viewer .modal-header button.action-item__menutoggle').should('not.exist') diff --git a/src/components/Images.vue b/src/components/Images.vue index e2655d728..9654a0552 100644 --- a/src/components/Images.vue +++ b/src/components/Images.vue @@ -11,63 +11,63 @@ :fileid="fileid" @close="onClose" /> - @@ -76,6 +76,7 @@ import Vue from 'vue' import AsyncComputed from 'vue-async-computed' import PlayCircleOutline from 'vue-material-design-icons/PlayCircleOutline.vue' +import IconImageBroken from 'vue-material-design-icons/ImageBroken.vue' import axios from '@nextcloud/axios' import { basename } from '@nextcloud/paths' @@ -85,6 +86,7 @@ import { NcLoadingIcon } from '@nextcloud/vue' import ImageEditor from './ImageEditor.vue' import { findLivePhotoPeerFromFileId } from '../utils/livePhotoUtils' import { getDavPath } from '../utils/fileUtils' +import { getPreviewIfAny } from '../utils/previewUtils' Vue.use(AsyncComputed) @@ -92,6 +94,7 @@ export default { name: 'Images', components: { + IconImageBroken, ImageEditor, PlayCircleOutline, NcLoadingIcon, @@ -175,23 +178,35 @@ export default { // Load the raw gif instead of the static preview if (this.mime === 'image/gif') { + // if the source failed fallback to the preview + if (this.fallback) { + return this.previewPath + } return this.src } - // If there is no preview and we have a direct source - // load it instead - if (this.source && !this.hasPreview && !this.previewUrl) { - return this.source + // First try the preview if any + if (!this.fallback && this.previewPath) { + return this.previewPath } // If loading the preview failed once, let's load the original file - if (this.fallback) { - return this.src - } + return this.src + }, - return this.previewPath + async previewPath() { + return await getPreviewIfAny({ + ...this.$attrs, + fileid: this.fileid, + filename: this.filename, + previewUrl: this.previewUrl, + hasPreview: this.hasPreview, + davPath: this.davPath, + etag: this.$attrs.etag, + }) }, }, + watch: { active(val, old) { // the item was hidden before and is now the current view diff --git a/src/mixins/Mime.js b/src/mixins/Mime.js index 7e530190d..5c10093b0 100644 --- a/src/mixins/Mime.js +++ b/src/mixins/Mime.js @@ -3,12 +3,11 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ import debounce from 'debounce' -import PreviewUrl from '../mixins/PreviewUrl.js' import parsePath from 'path-parse' +import { getDavPath } from '../utils/fileUtils.ts' export default { inheritAttrs: false, - mixins: [PreviewUrl], props: { // Is the current component shown active: { @@ -98,6 +97,18 @@ export default { }, computed: { + /** + * Absolute dav remote path of the file + * + * @return {string} + */ + davPath() { + return getDavPath({ + filename: this.filename, + basename: this.basename, + }) + }, + name() { return parsePath(this.basename).name }, diff --git a/src/mixins/PreviewUrl.js b/src/mixins/PreviewUrl.js deleted file mode 100644 index 65d0be4e8..000000000 --- a/src/mixins/PreviewUrl.js +++ /dev/null @@ -1,56 +0,0 @@ -/** - * SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ -import { getPreviewIfAny } from '../utils/previewUtils.ts' -import { getDavPath } from '../utils/fileUtils.ts' - -export default { - computed: { - /** - * Link to the preview path if the file have a preview - * - * @return {string} - */ - previewPath() { - return this.getPreviewIfAny({ - fileid: this.fileid, - filename: this.filename, - previewUrl: this.previewUrl, - hasPreview: this.hasPreview, - davPath: this.davPath, - etag: this.$attrs.etag, - }) - }, - - /** - * Absolute dav remote path of the file - * - * @return {string} - */ - davPath() { - return getDavPath({ - filename: this.filename, - basename: this.basename, - }) - }, - }, - methods: { - /** - * Return the preview url if the file have an existing - * preview or the absolute dav remote path if none. - * - * @param {object} data destructuring object - * @param {string} data.fileid the file id - * @param {string} [data.previewUrl] URL of the file preview - * @param {boolean} data.hasPreview have the file an existing preview ? - * @param {string} data.davPath the absolute dav path - * @param {string} data.filename the file name - * @param {string|null} data.etag the etag of the file - * @return {string} the absolute url - */ - getPreviewIfAny(data) { - return getPreviewIfAny(data) - }, - }, -} diff --git a/src/utils/canDownload.ts b/src/utils/canDownload.ts index 0caa1fed1..4dc71e91b 100644 --- a/src/utils/canDownload.ts +++ b/src/utils/canDownload.ts @@ -12,7 +12,9 @@ import type { FileInfo } from './fileUtils' export function canDownload(fileInfo: FileInfo) { // TODO: This should probably be part of `@nextcloud/sharing` // check share attributes - const shareAttributes = typeof fileInfo?.shareAttributes === 'string' ? JSON.parse(fileInfo.shareAttributes || '[]') : fileInfo?.shareAttributes + const shareAttributes = typeof fileInfo?.shareAttributes === 'string' + ? JSON.parse(fileInfo.shareAttributes || '[]') + : fileInfo?.shareAttributes if (shareAttributes && shareAttributes.length > 0) { const downloadAttribute = shareAttributes.find(({ scope, key }) => scope === 'permissions' && key === 'download') diff --git a/src/utils/fileUtils.ts b/src/utils/fileUtils.ts index 50167879f..d4ce29a03 100644 --- a/src/utils/fileUtils.ts +++ b/src/utils/fileUtils.ts @@ -9,8 +9,6 @@ import { getLanguage } from '@nextcloud/l10n' import { encodePath } from '@nextcloud/paths' import camelcase from 'camelcase' -import { isNumber } from './numberUtil' - export interface FileInfo { /** ID of the file (not unique if shared, use source instead) */ fileid?: number @@ -33,7 +31,7 @@ export interface FileInfo { /** File type */ type: 'directory'|'file' /** Attributes for file shares */ - shareAttributes?: string|Array<{value:boolean|string|number|null|object|Array, key: string, scope: string}> + shareAttributes?: string|Array<{ value: unknown, key: string, scope: string }> // custom attributes not fetch from API @@ -78,13 +76,7 @@ export function sortCompare(fileInfo1: FileInfo, fileInfo2: FileInfo, key: strin return 1 } - // if this is a number, let's sort by integer - if (isNumber(fileInfo1[key]) && isNumber(fileInfo2[key])) { - const result = Number(fileInfo1[key]) - Number(fileInfo2[key]) - return asc ? result : -result - } - - // else we sort by string, so let's sort directories first + // let's sort directories first if (fileInfo1.type === 'directory' && fileInfo2.type !== 'directory') { return -1 } else if (fileInfo1.type !== 'directory' && fileInfo2.type === 'directory') { @@ -97,8 +89,8 @@ export function sortCompare(fileInfo1: FileInfo, fileInfo2: FileInfo, key: strin } // finally sort by name return asc - ? fileInfo1[key].localeCompare(fileInfo2[key], getLanguage(), { numeric: true }) - : -fileInfo1[key].localeCompare(fileInfo2[key], getLanguage(), { numeric: true }) + ? String(fileInfo1[key]).localeCompare(fileInfo2[key], getLanguage(), { numeric: true }) + : -String(fileInfo1[key]).localeCompare(fileInfo2[key], getLanguage(), { numeric: true }) } /** @@ -107,29 +99,20 @@ export function sortCompare(fileInfo1: FileInfo, fileInfo2: FileInfo, key: strin * @param obj The stat response to convert */ export function genFileInfo(obj: FileStat): FileInfo { - const fileInfo = {} - - Object.keys(obj).forEach(key => { - const data = obj[key] - - // flatten object if any - if (!!data && typeof data === 'object' && !Array.isArray(data)) { - Object.assign(fileInfo, genFileInfo(data)) - } else { - // format key and add it to the fileInfo - if (data === 'false') { - fileInfo[camelcase(key)] = false - } else if (data === 'true') { - fileInfo[camelcase(key)] = true - } else { - fileInfo[camelcase(key)] = isNumber(data) - ? Number(data) - : data - } - } - }) + const fileStat = { + ...(obj.props ?? {}), + ...obj, + props: undefined, + } - return fileInfo as FileInfo + const fileInfo = Object.entries(fileStat) + // Make property names camel case + .map(([key, value]) => [camelcase(key), value]) + // Convert boolean - Numbers are already parsed by the WebDAV client + .map(([key, value]) => [key, ['true', 'false'].includes(value as never) ? value === 'true' : value]) + // remove undefined properties + .filter(([, value]) => value !== undefined) + return Object.fromEntries(fileInfo) } /** diff --git a/src/utils/livePhotoUtils.ts b/src/utils/livePhotoUtils.ts index 432252b15..430b8c79b 100644 --- a/src/utils/livePhotoUtils.ts +++ b/src/utils/livePhotoUtils.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { BasicFileInfo } from './models' +import type { FileInfo } from './fileUtils' const livePictureExt = ['jpg', 'jpeg', 'png'] const livePictureExtRegex = new RegExp(`\\.(${livePictureExt.join('|')})$`, 'i') @@ -13,8 +13,8 @@ const livePictureExtRegex = new RegExp(`\\.(${livePictureExt.join('|')})$`, 'i') * @param peerFileId * @param fileList */ -export function findLivePhotoPeerFromFileId(peerFileId: number, fileList: BasicFileInfo[]): BasicFileInfo | undefined { - return fileList.find(file => file.fileid === peerFileId) +export function findLivePhotoPeerFromFileId(peerFileId: number, fileList: FileInfo[]): FileInfo | undefined { + return fileList.find((file) => file.fileid === peerFileId) } /** @@ -22,10 +22,10 @@ export function findLivePhotoPeerFromFileId(peerFileId: number, fileList: BasicF * @param referenceFile * @param fileList */ -export function findLivePhotoPeerFromName(referenceFile: BasicFileInfo, fileList: BasicFileInfo[]): BasicFileInfo | undefined { +export function findLivePhotoPeerFromName(referenceFile: FileInfo, fileList: FileInfo[]): FileInfo | undefined { return fileList.find(comparedFile => { // if same filename and extension is allowed return comparedFile.filename !== referenceFile.filename - && (comparedFile.basename.startsWith(referenceFile.name) && livePictureExtRegex.test(comparedFile.basename)) + && (comparedFile.basename.startsWith(referenceFile.name ?? referenceFile.filename) && livePictureExtRegex.test(comparedFile.basename)) }) } diff --git a/src/utils/models.ts b/src/utils/models.ts deleted file mode 100644 index 7f947bb9f..000000000 --- a/src/utils/models.ts +++ /dev/null @@ -1,17 +0,0 @@ -/** - * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -export interface BasicFileInfo { - fileid: number // The file id - filename: string // The file name, ex: /a/b/c/file.txt - basename: string // The base name, ex: file.txt - name: string // The name, ex: file - source?: string // The source of the file, ex: https://example.org/remote.php/dav/files/userId/fileName.jpg - previewUrl?: string // Optional URL of the file preview - hasPreview: boolean // Does the file has an existing preview ? - davPath: string // The absolute dav path - etag: string|null // The etag of the file - metadataFilesLivePhoto?: number // The id of the peer live photo -} diff --git a/src/utils/numberUtil.ts b/src/utils/numberUtil.ts deleted file mode 100644 index 039dc5c8e..000000000 --- a/src/utils/numberUtil.ts +++ /dev/null @@ -1,13 +0,0 @@ -/** - * SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -const isNumber = function(num): boolean { - if (!num) { - return false - } - return Number(num).toString() === num.toString() -} - -export { isNumber } diff --git a/src/utils/previewUtils.ts b/src/utils/previewUtils.ts index 3ffaeeedc..09a54f490 100644 --- a/src/utils/previewUtils.ts +++ b/src/utils/previewUtils.ts @@ -3,38 +3,68 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { BasicFileInfo } from './models' +import type { FileInfo } from './fileUtils.js' + import { encodePath } from '@nextcloud/paths' import { generateUrl } from '@nextcloud/router' import { getSharingToken, isPublicShare } from '@nextcloud/sharing/public' +import { canDownload } from './canDownload' + +import axios from '@nextcloud/axios' +import logger from '../services/logger.js' + +/** + * Fetch a preview for download-disabled shares + * @param url The preview URL to fetch + */ +async function loadPrivatePreview(url: string): Promise { + try { + const { data } = await axios.get(url, { headers: { 'X-NC-Preview': 'true' }, responseType: 'blob' }) + const { promise, resolve } = Promise.withResolvers() + const reader = new FileReader() + reader.addEventListener('load', () => resolve(reader.result!.toString()), false) + reader.readAsDataURL(data) + + return promise + } catch (error) { + logger.error('Could not fetch preview image', { error }) + throw error + } +} /** - * @param root0 - * @param root0.fileid - * @param root0.filename - * @param root0.previewUrl - * @param root0.hasPreview - * @param root0.davPath - * @param root0.etag + * @param fileInfo The file info * @return the preview url if the file have an existing preview or the absolute dav remote path if none. */ -export function getPreviewIfAny({ fileid, filename, previewUrl, hasPreview, davPath, etag }: BasicFileInfo): string { - if (previewUrl) { - return previewUrl +export async function getPreviewIfAny(fileInfo: FileInfo): Promise { + if (fileInfo.previewUrl) { + return fileInfo.previewUrl } + const { fileid, filename, hasPreview, davPath, etag } = fileInfo const searchParams = `fileId=${fileid}` + `&x=${Math.floor(screen.width * devicePixelRatio)}` + `&y=${Math.floor(screen.height * devicePixelRatio)}` + '&a=true' - + (etag !== null ? `&etag=${etag.replace(/"/g, '')}` : '') + + (etag ? `&etag=${etag.replace(/"/g, '')}` : '') if (hasPreview) { - // TODO: find a nicer standard way of doing this? + let url: string if (isPublicShare()) { - return generateUrl(`/apps/files_sharing/publicpreview/${getSharingToken()}?file=${encodePath(filename)}&${searchParams}`) + url = generateUrl(`/apps/files_sharing/publicpreview/${getSharingToken()}?file=${encodePath(filename)}&${searchParams}`) + } else { + url = generateUrl(`/core/preview?${searchParams}`) + } + + // Best case: We have download permission, so we can just use the URL + if (canDownload(fileInfo)) { + return url } - return generateUrl(`/core/preview?${searchParams}`) + + // If not set correct header and fetch the image manually + return await loadPrivatePreview(url) } - return davPath + + // If nothing worked, we fallback to the dav path + return davPath! } diff --git a/src/views/Viewer.vue b/src/views/Viewer.vue index 93b93ec07..c5fbd772b 100644 --- a/src/views/Viewer.vue +++ b/src/views/Viewer.vue @@ -1058,9 +1058,8 @@ export default { this.trapElements = [] }, - // Update etag of updated file to break cache. /** - * + * Update ETag of updated file to break cache. * @param {Node} node */ async handleFileUpdated(node) { diff --git a/tsconfig.json b/tsconfig.json index 1bd4175b4..9601f74f1 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,12 +4,17 @@ "compilerOptions": { "lib": [ "DOM", - "ES2015" + "ESNext" ], "rootDir": "src", "noImplicitAny": false, }, "vueCompilerOptions": { "target": 2.7 + }, + "ts-node": { + "compilerOptions": { + "target": "ES2015" + } } }