From 69aad564213dc0584bc343935a73b6e40acd07aa Mon Sep 17 00:00:00 2001 From: Nolan Lawson Date: Tue, 24 Nov 2020 15:37:10 -0800 Subject: [PATCH] fix: fix tainted canvas error with OCR (#1902) * fix: fix tainted canvas error with OCR fixes #1901 * fix: minor tweaks --- src/routes/_actions/compose.js | 3 + src/routes/_actions/media.js | 4 +- .../dialog/components/MediaAltEditor.html | 7 +- src/routes/_database/knownInstances.js | 12 +-- .../_thirdparty/idb-keyval/idb-keyval.js | 30 +++++-- src/routes/_utils/mediaUploadFileCache.js | 86 ++++++++++++++++++- tests/unit/test-media-cache.js | 60 +++++++++++++ 7 files changed, 176 insertions(+), 26 deletions(-) create mode 100644 tests/unit/test-media-cache.js diff --git a/src/routes/_actions/compose.js b/src/routes/_actions/compose.js index 80afa882..aa17480a 100644 --- a/src/routes/_actions/compose.js +++ b/src/routes/_actions/compose.js @@ -6,6 +6,8 @@ import { database } from '../_database/database' import { emit } from '../_utils/eventBus' import { putMediaMetadata } from '../_api/media' import uniqBy from 'lodash-es/uniqBy' +import { deleteCachedMediaFile } from '../_utils/mediaUploadFileCache' +import { scheduleIdleTask } from '../_utils/scheduleIdleTask' export async function insertHandleForReply (statusId) { const { currentInstance } = store.get() @@ -58,6 +60,7 @@ export async function postStatus (realm, text, inReplyToId, mediaIds, addStatusOrNotification(currentInstance, 'home', status) store.clearComposeData(realm) emit('postedStatus', realm, inReplyToUuid) + scheduleIdleTask(() => (mediaIds || []).forEach(mediaId => deleteCachedMediaFile(mediaId))) // clean up media cache } catch (e) { console.error(e) toast.say('Unable to post status: ' + (e.message || '')) diff --git a/src/routes/_actions/media.js b/src/routes/_actions/media.js index b9a0ff20..d3638829 100644 --- a/src/routes/_actions/media.js +++ b/src/routes/_actions/media.js @@ -2,7 +2,7 @@ import { store } from '../_store/store' import { uploadMedia } from '../_api/media' import { toast } from '../_components/toast/toast' import { scheduleIdleTask } from '../_utils/scheduleIdleTask' -import { mediaUploadFileCache } from '../_utils/mediaUploadFileCache' +import { setCachedMediaFile } from '../_utils/mediaUploadFileCache' export async function doMediaUpload (realm, file) { const { currentInstance, accessToken } = store.get() @@ -13,7 +13,7 @@ export async function doMediaUpload (realm, file) { if (composeMedia.length === 4) { throw new Error('Only 4 media max are allowed') } - mediaUploadFileCache.set(response.url, file) + await setCachedMediaFile(response.id, file) composeMedia.push({ data: response, file: { name: file.name }, diff --git a/src/routes/_components/dialog/components/MediaAltEditor.html b/src/routes/_components/dialog/components/MediaAltEditor.html index e1d1d564..21e29baf 100644 --- a/src/routes/_components/dialog/components/MediaAltEditor.html +++ b/src/routes/_components/dialog/components/MediaAltEditor.html @@ -106,7 +106,7 @@ import { runTesseract } from '../../../_utils/runTesseract' import SvgIcon from '../../SvgIcon.html' import { toast } from '../../toast/toast' - import { mediaUploadFileCache } from '../../../_utils/mediaUploadFileCache' + import { getCachedMediaFile } from '../../../_utils/mediaUploadFileCache' const updateRawTextInStore = throttleTimer(requestPostAnimationFrame) @@ -131,6 +131,7 @@ length: ({ rawText }) => length(rawText || ''), overLimit: ({ mediaAltCharLimit, length }) => length > mediaAltCharLimit, url: ({ media, index }) => get(media, [index, 'data', 'url']), + mediaId: ({ media, index }) => get(media, [index, 'data', 'id']), extractButtonText: ({ extracting }) => extracting ? 'Extracting text…' : 'Extract text from image', extractButtonLabel: ({ extractButtonText, extractionProgress, extracting }) => { if (extracting) { @@ -183,13 +184,13 @@ async onClick () { this.set({ extracting: true }) try { - const { url } = this.get() + const { url, mediaId } = this.get() const onProgress = progress => { requestAnimationFrame(() => { this.set({ extractionProgress: progress * 100 }) }) } - const file = mediaUploadFileCache.get(url) + const file = await getCachedMediaFile(mediaId) let text if (file) { // Avoid downloading from the network a file that the user *just* uploaded const fileUrl = URL.createObjectURL(file) diff --git a/src/routes/_database/knownInstances.js b/src/routes/_database/knownInstances.js index 8d917a25..5d276417 100644 --- a/src/routes/_database/knownInstances.js +++ b/src/routes/_database/knownInstances.js @@ -1,5 +1,4 @@ -import { set, keys, del, close } from '../_thirdparty/idb-keyval/idb-keyval' -import { lifecycle } from '../_utils/lifecycle' +import { set, keys, del } from '../_thirdparty/idb-keyval/idb-keyval' const PREFIX = 'known-instance-' @@ -16,12 +15,3 @@ export async function addKnownInstance (instanceName) { export async function deleteKnownInstance (instanceName) { return del(PREFIX + instanceName) } - -if (process.browser) { - lifecycle.addEventListener('statechange', async event => { - if (event.newState === 'frozen') { // page is frozen, close IDB connections - await close() - console.log('closed knownInstances DB') - } - }) -} diff --git a/src/routes/_thirdparty/idb-keyval/idb-keyval.js b/src/routes/_thirdparty/idb-keyval/idb-keyval.js index e54908be..ab6e088e 100644 --- a/src/routes/_thirdparty/idb-keyval/idb-keyval.js +++ b/src/routes/_thirdparty/idb-keyval/idb-keyval.js @@ -1,5 +1,8 @@ // Forked from https://github.com/jakearchibald/idb-keyval/commit/ea7d507 // Adds a function for closing the database, ala https://github.com/jakearchibald/idb-keyval/pull/65 +// Also hooks it into the lifecycle frozen event +import { lifecycle } from '../../_utils/lifecycle' + class Store { constructor (dbName = 'keyval-store', storeName = 'keyval') { this.storeName = storeName @@ -51,32 +54,37 @@ function getDefaultStore () { return store } -function get (key, store = getDefaultStore()) { +function get (key) { + const store = getDefaultStore() let req return store._withIDBStore('readonly', store => { req = store.get(key) }).then(() => req.result) } -function set (key, value, store = getDefaultStore()) { +function set (key, value) { + const store = getDefaultStore() return store._withIDBStore('readwrite', store => { store.put(value, key) }) } -function del (key, store = getDefaultStore()) { +function del (key) { + const store = getDefaultStore() return store._withIDBStore('readwrite', store => { store.delete(key) }) } -function clear (store = getDefaultStore()) { +function clear () { + const store = getDefaultStore() return store._withIDBStore('readwrite', store => { store.clear() }) } -function keys (store = getDefaultStore()) { +function keys () { + const store = getDefaultStore() const keys = [] return store._withIDBStore('readonly', store => { // This would be store.getAllKeys(), but it isn't supported by Edge or Safari. @@ -91,8 +99,18 @@ function keys (store = getDefaultStore()) { }).then(() => keys) } -function close (store = getDefaultStore()) { +function close () { + const store = getDefaultStore() return store._close() } +if (process.browser) { + lifecycle.addEventListener('statechange', async event => { + if (event.newState === 'frozen') { // page is frozen, close IDB connections + await close() + console.log('closed keyval DB') + } + }) +} + export { Store, get, set, del, clear, keys, close } diff --git a/src/routes/_utils/mediaUploadFileCache.js b/src/routes/_utils/mediaUploadFileCache.js index cdada303..143799ea 100644 --- a/src/routes/_utils/mediaUploadFileCache.js +++ b/src/routes/_utils/mediaUploadFileCache.js @@ -1,6 +1,84 @@ -// keep a cache of files for the most recent uploads to avoid -// re-downloading them for OCR +// Keep an LRU cache of recently-uploaded files for OCR. +// We keep them in IDB to avoid tainted canvas errors after a refresh. +// https://github.com/nolanlawson/pinafore/issues/1901 -import { QuickLRU } from '../_thirdparty/quick-lru/quick-lru' +import { get, set, keys, del } from '../_thirdparty/idb-keyval/idb-keyval' -export const mediaUploadFileCache = new QuickLRU({ maxSize: 4 }) +const PREFIX = 'media-cache-' +const DELIMITER = '-cache-' +const LIMIT = 4 // you can upload 4 images per post, this seems reasonable despite cross-instance usage +export const DELETE_AFTER = 604800000 // 7 days + +let deleteAfter = DELETE_AFTER + +function keyToData (key) { + key = key.substring(PREFIX.length) + const index = key.indexOf(DELIMITER) + // avoiding str.split() to not have to worry about ids containing the delimiter string somehow + return [key.substring(0, index), key.substring(index + DELIMITER.length)] +} + +function dataToKey (timestamp, id) { + return `${PREFIX}${timestamp}${DELIMITER}${id}` +} + +async function getAllKeys () { + return (await keys()).filter(key => key.startsWith(PREFIX)).sort() +} + +export async function getCachedMediaFile (id) { + const allKeys = await getAllKeys() + + for (const key of allKeys) { + const otherId = keyToData(key)[1] + if (id === otherId) { + return get(key) + } + } +} + +export async function setCachedMediaFile (id, file) { + const allKeys = await getAllKeys() + + if (allKeys.map(keyToData).map(_ => _[1]).includes(id)) { + return // do nothing, it's already in there + } + + while (allKeys.length >= LIMIT) { + // already sorted in chronological order, so delete the oldest + await del(allKeys.shift()) + } + + // delete anything that's too old, while we're at it + for (const key of allKeys) { + const timestamp = keyToData(key)[0] + if (Date.now() - Date.parse(timestamp) >= deleteAfter) { + await del(key) + } + } + + const key = dataToKey(new Date().toISOString(), id) + + await set(key, file) +} + +export async function deleteCachedMediaFile (id) { + const allKeys = await getAllKeys() + + for (const key of allKeys) { + const otherId = keyToData(key)[1] + if (otherId === id) { + await del(key) + } + } +} + +// The following are only used in tests + +export async function getAllCachedFileIds () { + return (await getAllKeys()).map(keyToData).map(_ => _[1]) +} + +export function setDeleteAfter (newDeleteAfter) { + deleteAfter = newDeleteAfter +} diff --git a/tests/unit/test-media-cache.js b/tests/unit/test-media-cache.js new file mode 100644 index 00000000..1ebf671d --- /dev/null +++ b/tests/unit/test-media-cache.js @@ -0,0 +1,60 @@ +/* global it describe beforeEach */ + +import '../indexedDBShims' +import assert from 'assert' +import { + getCachedMediaFile, setCachedMediaFile, deleteCachedMediaFile, getAllCachedFileIds, setDeleteAfter, DELETE_AFTER +} from '../../src/routes/_utils/mediaUploadFileCache' + +describe('test-database.js', function () { + this.timeout(60000) + + beforeEach(async () => { + for (const key of await getAllCachedFileIds()) { + await deleteCachedMediaFile(key) + } + setDeleteAfter(DELETE_AFTER) + }) + + it('can store media files', async () => { + await setCachedMediaFile('woot', 'woot') + const result = await getCachedMediaFile('woot') + assert.deepStrictEqual(result, 'woot') + await deleteCachedMediaFile('woot') + const result2 = await getCachedMediaFile('woot') + assert.deepStrictEqual(result2, undefined) + }) + + it('does nothing if you set() the same id twice', async () => { + await setCachedMediaFile('woot', 'woot') + await setCachedMediaFile('woot', 'woot2') + const result = await getCachedMediaFile('woot') + assert.deepStrictEqual(result, 'woot') + }) + + it('returns undefined if not found', async () => { + const result = await getCachedMediaFile('woot') + assert.deepStrictEqual(result, undefined) + }) + + it('does nothing when deleting an unfound key', async () => { + await deleteCachedMediaFile('doesnt-exist') + }) + + it('only stores up to 4 files', async () => { + for (let i = 0; i < 10; i++) { + await new Promise(resolve => setTimeout(resolve, 4)) // delay to avoid timing collisions + await setCachedMediaFile(i.toString(), i) + } + const ids = await getAllCachedFileIds() + assert.deepStrictEqual(ids, [6, 7, 8, 9].map(_ => _.toString())) + }) + + it('deletes old files during set()', async () => { + setDeleteAfter(0) + await setCachedMediaFile('woot', 'woot') + await setCachedMediaFile('woot2', 'woot2') + assert.deepStrictEqual(await getCachedMediaFile('woot'), undefined) + assert.deepStrictEqual(await getCachedMediaFile('woot2'), 'woot2') + }) +})