From 3e88eeddc418c30ba7f38465a45212ff0f6b89d2 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Sat, 19 Aug 2023 16:22:54 +0300 Subject: [PATCH 1/5] Handle potential CORS issues during file downloads - Added logic to resolve URLs using `tryResolveUrlFromApiRoot`. - For URLs not originating from the API root (3rd party), leverage the browser's built-in functionality by opening them in a new tab to avoid CORS limitations during direct downloads. --- src/libs/fileDownload/index.js | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/libs/fileDownload/index.js b/src/libs/fileDownload/index.js index 0b86daf7141f..c100f73c2fcf 100644 --- a/src/libs/fileDownload/index.js +++ b/src/libs/fileDownload/index.js @@ -1,5 +1,7 @@ import * as Link from '@userActions/Link'; +import * as ApiUtils from '../ApiUtils'; import * as FileUtils from './FileUtils'; +import tryResolveUrlFromApiRoot from '../tryResolveUrlFromApiRoot'; /** * Downloading attachment in web, desktop @@ -8,8 +10,15 @@ import * as FileUtils from './FileUtils'; * @returns {Promise} */ export default function fileDownload(url, fileName) { - return new Promise((resolve) => { - fetch(url) + const resolvedUrl = tryResolveUrlFromApiRoot(url); + if (!resolvedUrl.startsWith(ApiUtils.getApiRoot())) { + // Different origin URLs might pose a CORS issue during direct downloads. + // Opening in a new tab avoids this limitation, letting the browser handle the download. + Link.openExternalLink(url); + return Promise.resolve(); + } + + return fetch(url) .then((response) => response.blob()) .then((blob) => { // Create blob link to download @@ -35,12 +44,7 @@ export default function fileDownload(url, fileName) { // Clean up and remove the link URL.revokeObjectURL(link.href); link.parentNode.removeChild(link); - return resolve(); }) - .catch(() => { - // file could not be downloaded, open sourceURL in new tab - Link.openExternalLink(url); - return resolve(); - }); - }); + // file could not be downloaded, open sourceURL in new tab + .catch(() => Link.openExternalLink(url)); } From 006038059efe160de4979d930409c21cf3d8822c Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 21 Aug 2023 11:21:29 +0300 Subject: [PATCH 2/5] Fix: Handle undefined fileName during attachment download - Addressed issue where attachments, especially those added via drag and drop, lacked the `fileName` attribute, causing download failures. - Enhanced the fallback mechanism to generate file name from URL when `fileName` is missing. - Updated the fileName logic in `fileDownload` across iOS, Android, and web implementations. --- src/libs/fileDownload/index.android.js | 6 +++--- src/libs/fileDownload/index.ios.js | 4 ++-- src/libs/fileDownload/index.js | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libs/fileDownload/index.android.js b/src/libs/fileDownload/index.android.js index c3528b579f67..5f50ad27ef70 100644 --- a/src/libs/fileDownload/index.android.js +++ b/src/libs/fileDownload/index.android.js @@ -32,7 +32,7 @@ function hasAndroidPermission() { /** * Handling the download * @param {String} url - * @param {String} fileName + * @param {String} [fileName] * @returns {Promise} */ function handleDownload(url, fileName) { @@ -41,7 +41,7 @@ function handleDownload(url, fileName) { // Android files will download to Download directory const path = dirs.DownloadDir; - const attachmentName = FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(url); + const attachmentName = fileName ? FileUtils.appendTimeToFileName(fileName) : FileUtils.getAttachmentName(url); const isLocalFile = url.startsWith('file://'); @@ -96,7 +96,7 @@ function handleDownload(url, fileName) { /** * Checks permission and downloads the file for Android * @param {String} url - * @param {String} fileName + * @param {String} [fileName] * @returns {Promise} */ export default function fileDownload(url, fileName) { diff --git a/src/libs/fileDownload/index.ios.js b/src/libs/fileDownload/index.ios.js index 1599e919d28a..252c40797e7a 100644 --- a/src/libs/fileDownload/index.ios.js +++ b/src/libs/fileDownload/index.ios.js @@ -68,14 +68,14 @@ function downloadVideo(fileUrl, fileName) { /** * Download the file based on type(image, video, other file types)for iOS * @param {String} fileUrl - * @param {String} fileName + * @param {String} [fileName] * @returns {Promise} */ export default function fileDownload(fileUrl, fileName) { return new Promise((resolve) => { let fileDownloadPromise = null; const fileType = FileUtils.getFileType(fileUrl); - const attachmentName = FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(fileUrl); + const attachmentName = fileName ? FileUtils.appendTimeToFileName(fileName) : FileUtils.getAttachmentName(fileUrl); switch (fileType) { case CONST.ATTACHMENT_FILE_TYPE.IMAGE: diff --git a/src/libs/fileDownload/index.js b/src/libs/fileDownload/index.js index c100f73c2fcf..35e22519d137 100644 --- a/src/libs/fileDownload/index.js +++ b/src/libs/fileDownload/index.js @@ -6,7 +6,7 @@ import tryResolveUrlFromApiRoot from '../tryResolveUrlFromApiRoot'; /** * Downloading attachment in web, desktop * @param {String} url - * @param {String} fileName + * @param {String} [fileName] * @returns {Promise} */ export default function fileDownload(url, fileName) { @@ -32,7 +32,7 @@ export default function fileDownload(url, fileName) { link.style.display = 'none'; link.setAttribute( 'download', - FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(url), // generating the file name + fileName ? FileUtils.appendTimeToFileName(fileName) : FileUtils.getAttachmentName(url), // generating the file name ); // Append to html link element page From 2db81721c6ed95e3238c328a6ed8dce2642d7de8 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 21 Aug 2023 13:15:33 +0300 Subject: [PATCH 3/5] Run prettier --- src/libs/fileDownload/index.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libs/fileDownload/index.js b/src/libs/fileDownload/index.js index 35e22519d137..48309377df81 100644 --- a/src/libs/fileDownload/index.js +++ b/src/libs/fileDownload/index.js @@ -1,7 +1,7 @@ +import * as ApiUtils from '@libs/ApiUtils'; +import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot'; import * as Link from '@userActions/Link'; -import * as ApiUtils from '../ApiUtils'; import * as FileUtils from './FileUtils'; -import tryResolveUrlFromApiRoot from '../tryResolveUrlFromApiRoot'; /** * Downloading attachment in web, desktop @@ -18,7 +18,8 @@ export default function fileDownload(url, fileName) { return Promise.resolve(); } - return fetch(url) + return ( + fetch(url) .then((response) => response.blob()) .then((blob) => { // Create blob link to download @@ -46,5 +47,6 @@ export default function fileDownload(url, fileName) { link.parentNode.removeChild(link); }) // file could not be downloaded, open sourceURL in new tab - .catch(() => Link.openExternalLink(url)); + .catch(() => Link.openExternalLink(url)) + ); } From c1536f941d96d2e108c0dd1cdc9d3fe93ed0bab9 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 24 Oct 2023 21:13:44 +0300 Subject: [PATCH 4/5] Fix: Add fallback name for attachments lacking the original filename --- src/components/ImageView/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/ImageView/index.js b/src/components/ImageView/index.js index 09656d700917..6fa8737cfbcd 100644 --- a/src/components/ImageView/index.js +++ b/src/components/ImageView/index.js @@ -5,6 +5,7 @@ import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator'; import Image from '@components/Image'; import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback'; import * as DeviceCapabilities from '@libs/DeviceCapabilities'; +import * as FileUtils from '@libs/fileDownload/FileUtils'; import styles from '@styles/styles'; import * as StyleUtils from '@styles/StyleUtils'; import CONST from '@src/CONST'; @@ -21,7 +22,7 @@ const propTypes = { url: PropTypes.string.isRequired, /** image file name */ - fileName: PropTypes.string.isRequired, + fileName: PropTypes.string, onError: PropTypes.func, }; @@ -29,6 +30,7 @@ const propTypes = { const defaultProps = { isAuthTokenRequired: false, onError: () => {}, + fileName: '', }; function ImageView({isAuthTokenRequired, url, fileName, onError}) { @@ -49,6 +51,7 @@ function ImageView({isAuthTokenRequired, url, fileName, onError}) { const scrollableRef = useRef(null); const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen(); + const accessibilityLabel = fileName || FileUtils.getAttachmentName(url); /** * @param {Number} newContainerWidth @@ -263,7 +266,7 @@ function ImageView({isAuthTokenRequired, url, fileName, onError}) { onPressIn={onContainerPressIn} onPress={onContainerPress} accessibilityRole={CONST.ACCESSIBILITY_ROLE.IMAGE} - accessibilityLabel={fileName} + accessibilityLabel={accessibilityLabel} > Date: Mon, 6 Nov 2023 11:57:15 +0200 Subject: [PATCH 5/5] Revert "Fix: Handle undefined fileName during attachment download" This reverts commit 006038059efe160de4979d930409c21cf3d8822c. Revert "Fix: Add fallback name for attachments lacking the original filename" This reverts commit c1536f941d96d2e108c0dd1cdc9d3fe93ed0bab9. --- src/components/ImageView/index.js | 7 ++----- src/libs/fileDownload/index.android.js | 6 +++--- src/libs/fileDownload/index.ios.js | 4 ++-- src/libs/fileDownload/index.js | 4 ++-- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/components/ImageView/index.js b/src/components/ImageView/index.js index 6fa8737cfbcd..09656d700917 100644 --- a/src/components/ImageView/index.js +++ b/src/components/ImageView/index.js @@ -5,7 +5,6 @@ import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator'; import Image from '@components/Image'; import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback'; import * as DeviceCapabilities from '@libs/DeviceCapabilities'; -import * as FileUtils from '@libs/fileDownload/FileUtils'; import styles from '@styles/styles'; import * as StyleUtils from '@styles/StyleUtils'; import CONST from '@src/CONST'; @@ -22,7 +21,7 @@ const propTypes = { url: PropTypes.string.isRequired, /** image file name */ - fileName: PropTypes.string, + fileName: PropTypes.string.isRequired, onError: PropTypes.func, }; @@ -30,7 +29,6 @@ const propTypes = { const defaultProps = { isAuthTokenRequired: false, onError: () => {}, - fileName: '', }; function ImageView({isAuthTokenRequired, url, fileName, onError}) { @@ -51,7 +49,6 @@ function ImageView({isAuthTokenRequired, url, fileName, onError}) { const scrollableRef = useRef(null); const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen(); - const accessibilityLabel = fileName || FileUtils.getAttachmentName(url); /** * @param {Number} newContainerWidth @@ -266,7 +263,7 @@ function ImageView({isAuthTokenRequired, url, fileName, onError}) { onPressIn={onContainerPressIn} onPress={onContainerPress} accessibilityRole={CONST.ACCESSIBILITY_ROLE.IMAGE} - accessibilityLabel={accessibilityLabel} + accessibilityLabel={fileName} > } */ function handleDownload(url, fileName) { @@ -41,7 +41,7 @@ function handleDownload(url, fileName) { // Android files will download to Download directory const path = dirs.DownloadDir; - const attachmentName = fileName ? FileUtils.appendTimeToFileName(fileName) : FileUtils.getAttachmentName(url); + const attachmentName = FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(url); const isLocalFile = url.startsWith('file://'); @@ -96,7 +96,7 @@ function handleDownload(url, fileName) { /** * Checks permission and downloads the file for Android * @param {String} url - * @param {String} [fileName] + * @param {String} fileName * @returns {Promise} */ export default function fileDownload(url, fileName) { diff --git a/src/libs/fileDownload/index.ios.js b/src/libs/fileDownload/index.ios.js index 252c40797e7a..1599e919d28a 100644 --- a/src/libs/fileDownload/index.ios.js +++ b/src/libs/fileDownload/index.ios.js @@ -68,14 +68,14 @@ function downloadVideo(fileUrl, fileName) { /** * Download the file based on type(image, video, other file types)for iOS * @param {String} fileUrl - * @param {String} [fileName] + * @param {String} fileName * @returns {Promise} */ export default function fileDownload(fileUrl, fileName) { return new Promise((resolve) => { let fileDownloadPromise = null; const fileType = FileUtils.getFileType(fileUrl); - const attachmentName = fileName ? FileUtils.appendTimeToFileName(fileName) : FileUtils.getAttachmentName(fileUrl); + const attachmentName = FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(fileUrl); switch (fileType) { case CONST.ATTACHMENT_FILE_TYPE.IMAGE: diff --git a/src/libs/fileDownload/index.js b/src/libs/fileDownload/index.js index 48309377df81..d1fa968b665f 100644 --- a/src/libs/fileDownload/index.js +++ b/src/libs/fileDownload/index.js @@ -6,7 +6,7 @@ import * as FileUtils from './FileUtils'; /** * Downloading attachment in web, desktop * @param {String} url - * @param {String} [fileName] + * @param {String} fileName * @returns {Promise} */ export default function fileDownload(url, fileName) { @@ -33,7 +33,7 @@ export default function fileDownload(url, fileName) { link.style.display = 'none'; link.setAttribute( 'download', - fileName ? FileUtils.appendTimeToFileName(fileName) : FileUtils.getAttachmentName(url), // generating the file name + FileUtils.appendTimeToFileName(fileName) || FileUtils.getAttachmentName(url), // generating the file name ); // Append to html link element page