diff --git a/app/attachments.js b/app/attachments.js index 403049185..a1303e2ea 100644 --- a/app/attachments.js +++ b/app/attachments.js @@ -46,7 +46,6 @@ exports.createReader = root => { if (!isString(relativePath)) { throw new TypeError("'relativePath' must be a string"); } - console.warn(`readFile: ${relativePath}`); const absolutePath = path.join(root, relativePath); const normalized = path.normalize(absolutePath); if (!normalized.startsWith(root)) { diff --git a/js/modules/types/visual_attachment.js b/js/modules/types/visual_attachment.js index e57a8a13d..f118d9536 100644 --- a/js/modules/types/visual_attachment.js +++ b/js/modules/types/visual_attachment.js @@ -30,7 +30,7 @@ exports.getImageDimensions = ({ objectUrl, logger }) => reject(error); }); //FIXME image/jpeg is hard coded - DecryptedAttachmentsManager.getDecryptedAttachmentUrl( + DecryptedAttachmentsManager.getDecryptedMediaUrl( objectUrl, 'image/jpg' ).then(decryptedUrl => { @@ -80,7 +80,7 @@ exports.makeImageThumbnail = ({ reject(error); }); - DecryptedAttachmentsManager.getDecryptedAttachmentUrl( + DecryptedAttachmentsManager.getDecryptedMediaUrl( objectUrl, contentType ).then(decryptedUrl => { @@ -117,8 +117,9 @@ exports.makeVideoScreenshot = ({ logger.error('makeVideoScreenshot error', toLogFormat(error)); reject(error); }); + //FIXME image/jpeg is hard coded - DecryptedAttachmentsManager.getDecryptedAttachmentUrl( + DecryptedAttachmentsManager.getDecryptedMediaUrl( objectUrl, 'image/jpg' ).then(decryptedUrl => { diff --git a/test/app/attachments_test.js b/test/app/attachments_test.js index 4b4c0c966..0901e63f4 100644 --- a/test/app/attachments_test.js +++ b/test/app/attachments_test.js @@ -30,7 +30,6 @@ describe('Attachments', () => { tempRootDirectory, 'Attachments_createWriterForNew' ); - const outputPath = await Attachments.createWriterForNew(tempDirectory)( input ); diff --git a/ts/components/Avatar.tsx b/ts/components/Avatar.tsx index cc46b2db9..1cd0097bd 100644 --- a/ts/components/Avatar.tsx +++ b/ts/components/Avatar.tsx @@ -83,17 +83,17 @@ const AvatarImage = (props: { export const Avatar = (props: Props) => { const { avatarPath, size, memberAvatars, name } = props; const [imageBroken, setImageBroken] = useState(false); + // contentType is not important + const { urlToLoad } = useEncryptedFileFetch(avatarPath || '', ''); const handleImageError = () => { window.log.warn( - 'Avatar: Image failed to load; failing over to placeholder' + 'Avatar: Image failed to load; failing over to placeholder', + urlToLoad ); setImageBroken(true); }; - // contentType is not important - const { urlToLoad } = useEncryptedFileFetch(avatarPath || '', ''); - const isClosedGroupAvatar = memberAvatars && memberAvatars.length; const hasImage = urlToLoad && !imageBroken && !isClosedGroupAvatar; diff --git a/ts/components/Lightbox.tsx b/ts/components/Lightbox.tsx index afecb0fbe..e00304c71 100644 --- a/ts/components/Lightbox.tsx +++ b/ts/components/Lightbox.tsx @@ -252,7 +252,13 @@ export const LightboxObject = ({ }); if (isImageTypeSupported) { - return {window.i18n('lightboxImageAlt')}; + return ( + {window.i18n('lightboxImageAlt')} + ); } const isVideoTypeSupported = GoogleChrome.isVideoTypeSupported(contentType); diff --git a/ts/components/session/ActionsPanel.tsx b/ts/components/session/ActionsPanel.tsx index b8a202fab..91ce3ecf4 100644 --- a/ts/components/session/ActionsPanel.tsx +++ b/ts/components/session/ActionsPanel.tsx @@ -1,14 +1,12 @@ -import React, { useEffect } from 'react'; +import React, { useEffect, useState } from 'react'; import { SessionIconButton, SessionIconSize, SessionIconType } from './icon'; import { Avatar, AvatarSize } from '../Avatar'; import { darkTheme, lightTheme } from '../../state/ducks/SessionTheme'; import { SessionToastContainer } from './SessionToastContainer'; -import { ConversationType } from '../../state/ducks/conversations'; -import { DefaultTheme } from 'styled-components'; import { ConversationController } from '../../session/conversations'; import { UserUtils } from '../../session/utils'; import { syncConfigurationIfNeeded } from '../../session/utils/syncUtils'; -import { DAYS } from '../../session/utils/Number'; +import { DAYS, MINUTES, SECONDS } from '../../session/utils/Number'; import { getItemById, hasSyncedInitialConfigurationItem, @@ -29,6 +27,8 @@ import { getFocusedSection } from '../../state/selectors/section'; import { useInterval } from '../../hooks/useInterval'; import { clearSearch } from '../../state/ducks/search'; import { showLeftPaneSection } from '../../state/ducks/section'; +import { cleanUpOldDecryptedMedias } from '../../session/crypto/DecryptedAttachmentsManager'; +import { useTimeoutFn } from 'react-use'; // tslint:disable-next-line: no-import-side-effect no-submodule-imports export enum SectionType { @@ -126,16 +126,20 @@ const showResetSessionIDDialogIfNeeded = async () => { window.showResetSessionIdDialog(); }; +const cleanUpMediasInterval = MINUTES * 30; + /** * ActionsPanel is the far left banner (not the left pane). * The panel with buttons to switch between the message/contact/settings/theme views */ export const ActionsPanel = () => { const dispatch = useDispatch(); + const [startCleanUpMedia, setStartCleanUpMedia] = useState(false); const ourPrimaryConversation = useSelector(getOurPrimaryConversation); // this maxi useEffect is called only once: when the component is mounted. + // For the action panel, it means this is called only one per app start/with a user loggedin useEffect(() => { void window.setClockParams(); if ( @@ -174,10 +178,27 @@ export const ActionsPanel = () => { }; // trigger a sync message if needed for our other devices - void syncConfiguration(); }, []); + // wait for cleanUpMediasInterval and then start cleaning up medias + // this would be way easier to just be able to not trigger a call with the setInterval + useEffect(() => { + const timeout = global.setTimeout( + () => setStartCleanUpMedia(true), + cleanUpMediasInterval + ); + + return () => global.clearTimeout(timeout); + }, []); + + useInterval( + () => { + cleanUpOldDecryptedMedias(); + }, + startCleanUpMedia ? cleanUpMediasInterval : null + ); + if (!ourPrimaryConversation) { window.log.warn('ActionsPanel: ourPrimaryConversation is not set'); return <>; diff --git a/ts/components/session/conversation/SessionConversation.tsx b/ts/components/session/conversation/SessionConversation.tsx index 8cefd6486..430a107cb 100644 --- a/ts/components/session/conversation/SessionConversation.tsx +++ b/ts/components/session/conversation/SessionConversation.tsx @@ -34,7 +34,7 @@ import { getPubkeysInPublicConversation, } from '../../../data/data'; import autoBind from 'auto-bind'; -import { getDecryptedAttachmentUrl } from '../../../session/crypto/DecryptedAttachmentsManager'; +import { getDecryptedMediaUrl } from '../../../session/crypto/DecryptedAttachmentsManager'; interface State { // Message sending progress @@ -940,7 +940,7 @@ export class SessionConversation extends React.Component { index?: number; }) { const { getAbsoluteAttachmentPath } = window.Signal.Migrations; - attachment.url = await getDecryptedAttachmentUrl( + attachment.url = await getDecryptedMediaUrl( attachment.url, attachment.contentType ); diff --git a/ts/components/session/conversation/SessionRightPanel.tsx b/ts/components/session/conversation/SessionRightPanel.tsx index 5cfc486d7..6ebaab2ba 100644 --- a/ts/components/session/conversation/SessionRightPanel.tsx +++ b/ts/components/session/conversation/SessionRightPanel.tsx @@ -21,7 +21,7 @@ import { getMessagesWithFileAttachments, getMessagesWithVisualMediaAttachments, } from '../../../data/data'; -import { getDecryptedAttachmentUrl } from '../../../session/crypto/DecryptedAttachmentsManager'; +import { getDecryptedMediaUrl } from '../../../session/crypto/DecryptedAttachmentsManager'; interface Props { id: string; @@ -196,7 +196,7 @@ class SessionRightPanel extends React.Component { const saveAttachment = async ({ attachment, message }: any = {}) => { const timestamp = message.received_at; - attachment.url = await getDecryptedAttachmentUrl( + attachment.url = await getDecryptedMediaUrl( attachment.url, attachment.contentType ); diff --git a/ts/hooks/useEncryptedFileFetch.ts b/ts/hooks/useEncryptedFileFetch.ts index 2919ff9a5..f5178f31e 100644 --- a/ts/hooks/useEncryptedFileFetch.ts +++ b/ts/hooks/useEncryptedFileFetch.ts @@ -1,6 +1,6 @@ import { useEffect, useState } from 'react'; -import { getDecryptedAttachmentUrl } from '../session/crypto/DecryptedAttachmentsManager'; +import { getDecryptedMediaUrl } from '../session/crypto/DecryptedAttachmentsManager'; export const useEncryptedFileFetch = (url: string, contentType: string) => { // tslint:disable-next-line: no-bitwise @@ -8,7 +8,7 @@ export const useEncryptedFileFetch = (url: string, contentType: string) => { const [loading, setLoading] = useState(true); async function fetchUrl() { - const decryptedUrl = await getDecryptedAttachmentUrl(url, contentType); + const decryptedUrl = await getDecryptedMediaUrl(url, contentType); setUrlToLoad(decryptedUrl); setLoading(false); diff --git a/ts/session/crypto/DecryptedAttachmentsManager.ts b/ts/session/crypto/DecryptedAttachmentsManager.ts index 681d4dd79..15c3590df 100644 --- a/ts/session/crypto/DecryptedAttachmentsManager.ts +++ b/ts/session/crypto/DecryptedAttachmentsManager.ts @@ -1,18 +1,47 @@ /** * This file handles attachments for us. * If the attachment filepath is an encrypted one. It will decrypt it, cache it, and return the blob url to it. + * An interval is run from time to time to cleanup old blobs loaded and not needed anymore (based on last access timestamp). + * + * */ import toArrayBuffer from 'to-arraybuffer'; import * as fse from 'fs-extra'; import { decryptAttachmentBuffer } from '../../types/Attachment'; +import { HOURS, MINUTES, SECONDS } from '../utils/Number'; // FIXME. // add a way to clean those from time to time (like every hours?) // add a way to remove the blob when the attachment file path is removed (message removed?) -const urlToDecryptedBlobMap = new Map(); +// do not hardcode the password +// a few FIXME image/jpeg is hard coded +const urlToDecryptedBlobMap = new Map< + string, + { decrypted: string; lastAccessTimestamp: number } +>(); -export const getDecryptedAttachmentUrl = async ( +export const cleanUpOldDecryptedMedias = () => { + const currentTimestamp = Date.now(); + let countCleaned = 0; + let countKept = 0; + window.log.info('Starting cleaning of medias blobs...'); + for (const iterator of urlToDecryptedBlobMap) { + // if the last access is older than one hour, revoke the url and remove it. + if (iterator[1].lastAccessTimestamp < currentTimestamp - HOURS * 1) { + URL.revokeObjectURL(iterator[1].decrypted); + urlToDecryptedBlobMap.delete(iterator[0]); + countCleaned++; + } else { + countKept++; + } + } + window.log.info( + `Clean medias blobs: cleaned/kept: ${countCleaned}:${countKept}` + ); +}; + +export const getDecryptedMediaUrl = async ( url: string, contentType: string ): Promise => { @@ -28,10 +57,18 @@ export const getDecryptedAttachmentUrl = async ( // this is a file encoded by session on our current attachments path. // we consider the file is encrypted. // if it's not, the hook caller has to fallback to setting the img src as an url to the file instead and load it - console.warn('url:', url, ' has:', urlToDecryptedBlobMap.has(url)); if (urlToDecryptedBlobMap.has(url)) { + // refresh the last access timestamp so we keep the one being currently in use + const existingObjUrl = urlToDecryptedBlobMap.get(url) + ?.decrypted as string; + + urlToDecryptedBlobMap.set(url, { + decrypted: existingObjUrl, + lastAccessTimestamp: Date.now(), + }); // typescript does not realize that the has above makes sure the get is not undefined - return urlToDecryptedBlobMap.get(url) as string; + + return existingObjUrl; } else { const encryptedFileContent = await fse.readFile(url); const decryptedContent = await decryptAttachmentBuffer( @@ -41,19 +78,23 @@ export const getDecryptedAttachmentUrl = async ( const arrayBuffer = decryptedContent.buffer; const { makeObjectUrl } = window.Signal.Types.VisualAttachment; const obj = makeObjectUrl(arrayBuffer, contentType); - console.warn('makeObjectUrl: ', obj, contentType); if (!urlToDecryptedBlobMap.has(url)) { - urlToDecryptedBlobMap.set(url, obj); + urlToDecryptedBlobMap.set(url, { + decrypted: obj, + lastAccessTimestamp: Date.now(), + }); } return obj; } else { // failed to decrypt, fallback to url image loading + // it might be a media we received before the update encrypting attachments locally. return url; } } } else { // Not sure what we got here. Just return the file. + return url; } };