From a0eea2a2713a6f32f40ad0cf8f6d56b404a60f62 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 27 Jun 2017 00:06:14 +0100 Subject: [PATCH 01/15] a completely untested and guaranteed broken EXIF stripper --- src/ContentMessages.js | 78 ++++++++++++++++++++++- src/components/structures/UserSettings.js | 4 ++ 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index 315c312b9f7..64e91564a8e 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -1,5 +1,6 @@ /* Copyright 2015, 2016 OpenMarket Ltd +Copyright 2017 Vector Creations Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -25,6 +26,7 @@ import { _t } from './languageHandler'; var Modal = require('./Modal'); var encrypt = require("browser-encrypt-attachment"); +var UserSettingsStore = require('./UserSettingsStore'); // Polyfill for Canvas.toBlob API using Canvas.toDataURL require("blueimp-canvas-to-blob"); @@ -222,6 +224,51 @@ function readFileAsArrayBuffer(file) { return deferred.promise; } +/** + * Strip EXIF metadata from a JPEG + * Taken from http://jsfiddle.net/mowglisanu/frhwm2xe/3/ + * + * @param {ArrayBuffer} data the image data + * @return {ArrayBuffer} the stripped image data + */ +function stripJpegMetadata(data) { + var dv = new DataView(data); + var offset = 0, recess = 0; + var pieces = []; + var i = 0; + + // FIXME: check this isn't stripping off any EXIF color profile data + // as that will break the colorimetry of the image. We're stripping + // this for privacy purposes rather than filesize. + if (dv.getUint16(offset) == 0xffd8) { + offset += 2; + var app1 = dv.getUint16(offset); + offset += 2; + while (offset < dv.byteLength){ + if (app1 == 0xffe1) { + pieces[i] = { recess:recess, offset:offset-2 }; + recess = offset + dv.getUint16(offset); + i++; + } + else if (app1 == 0xffda) { + break; + } + offset += dv.getUint16(offset); + var app1 = dv.getUint16(offset); + offset += 2; + } + if (pieces.length > 0) { + var newPieces = []; + pieces.forEach(function(v){ + newPieces.push(this.result.slice(v.recess, v.offset)); + }, this); + newPieces.push(this.result.slice(recess)); + } + } + + return newPieces ? newPieces : data; +} + /** * Upload the file to the content repository. * If the room is encrypted then encrypt the file before uploading. @@ -234,10 +281,28 @@ function readFileAsArrayBuffer(file) { * If the file is encrypted then the object will have a "file" key. */ function uploadFile(matrixClient, roomId, file) { + var readDataPromise; + + // strip metadata where we can (n.b. we don't want to strip colorspace metadata) + if (file.type === 'image/jpeg' && + UserSettingsStore.getSyncedSetting('stripFileMetadata', false)) + { + readDataPromise = readFileAsArrayBuffer(file).then(function(data) { + // fix up arraybuffer + data = stripJpegMetadata(data); + return q(data); + }); + } + if (matrixClient.isRoomEncrypted(roomId)) { // If the room is encrypted then encrypt the file before uploading it. // First read the file into memory. - return readFileAsArrayBuffer(file).then(function(data) { + + if (!readDataPromise) { + readDataPromise = readFileAsArrayBuffer(file); + } + + return readDataPromise.then(function(data) { // Then encrypt the file. return encrypt.encryptAttachment(data); }).then(function(encryptResult) { @@ -257,12 +322,21 @@ function uploadFile(matrixClient, roomId, file) { }); }); } else { - const basePromise = matrixClient.uploadContent(file); + if (readDataPromise) { + const basePromise = readDataPromise.then(function(data) { + return matrixClient.uploadContent(new Blob([data])); + }); + } + else { + const basePromise = matrixClient.uploadContent(file); + } + const promise1 = basePromise.then(function(url) { // If the attachment isn't encrypted then include the URL directly. return {"url": url}; }); // XXX: copy over the abort method to the new promise + // FIXME: This is probably broken if you have a readDataPromise defined promise1.abort = basePromise.abort; return promise1; } diff --git a/src/components/structures/UserSettings.js b/src/components/structures/UserSettings.js index 9a5eb07cde5..a3214612106 100644 --- a/src/components/structures/UserSettings.js +++ b/src/components/structures/UserSettings.js @@ -93,6 +93,10 @@ const SETTINGS_LABELS = [ id: 'disableMarkdown', label: 'Disable markdown formatting', }, + { + id: 'stripFileMetadata', + label: 'Strip metadata from file uploads', + }, /* { id: 'useFixedWidthFont', From 2b3ec4ba425ee1311eea88506de7b8f152966b60 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 27 Jun 2017 00:09:39 +0100 Subject: [PATCH 02/15] i18n --- src/i18n/strings/en_EN.json | 1 + 1 file changed, 1 insertion(+) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index a4dcb2873fd..708fdfd4a68 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -522,6 +522,7 @@ "Start a chat": "Start a chat", "Start authentication": "Start authentication", "Start Chat": "Start Chat", + "Strip metadata from file uploads": "Strip metadata from file uploads", "Submit": "Submit", "Success": "Success", "tag as %(tagName)s": "tag as %(tagName)s", From 1003ff47e9f5bbe183ffc167530d9837a2916797 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 27 Jun 2017 00:27:02 +0100 Subject: [PATCH 03/15] fix const thinko --- src/ContentMessages.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index 64e91564a8e..094be4c2a76 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -322,13 +322,14 @@ function uploadFile(matrixClient, roomId, file) { }); }); } else { + let basePromise; if (readDataPromise) { - const basePromise = readDataPromise.then(function(data) { + basePromise = readDataPromise.then(function(data) { return matrixClient.uploadContent(new Blob([data])); }); } else { - const basePromise = matrixClient.uploadContent(file); + basePromise = matrixClient.uploadContent(file); } const promise1 = basePromise.then(function(url) { From fe899aded99e7a7de7add3b98660effd0e2444f6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 15 Aug 2017 22:45:21 +0100 Subject: [PATCH 04/15] remove debug --- src/ContentMessages.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index b62ffcc6e68..e695d7fde6d 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -259,7 +259,6 @@ function stripJpegMetadata(data) { app1 = dv.getUint16(offset); offset += 2; } - console.log("DEBUG", data); if (pieces.length > 0) { pieces.forEach(function(v) { @@ -269,8 +268,6 @@ function stripJpegMetadata(data) { } } - console.log("DEBUG", newPieces.length, newPieces, data); - return newPieces.length ? new Blob( newPieces, { type: 'image/jpeg' }) : data; } From b7869d02f698825704bd774180fd8d26f633799e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 10 May 2020 14:01:28 +0100 Subject: [PATCH 05/15] add stripFileMetadata to settings --- .../views/settings/tabs/user/PreferencesUserSettingsTab.js | 1 + src/settings/Settings.js | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js b/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js index bdb2a9ffc4a..0a9ff7aaa0a 100644 --- a/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js @@ -56,6 +56,7 @@ export default class PreferencesUserSettingsTab extends React.Component { static ADVANCED_SETTINGS = [ 'alwaysShowEncryptionIcons', 'Pill.shouldShowPillAvatar', + 'stripFileMetadata', 'TagPanel.enableTagPanel', 'promptBeforeInviteUnknownUsers', // Start automatically after startup (electron-only) diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 5c6d843349e..411051ca047 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -229,6 +229,11 @@ export const SETTINGS = { displayName: _td('Enable automatic language detection for syntax highlighting'), default: false, }, + "stripFileMetadata": { + supportedLevels: LEVELS_ACCOUNT_SETTINGS, + displayName: _td('Strip metadata from file uploads'), + default: true, + }, "Pill.shouldShowPillAvatar": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, displayName: _td('Show avatars in user and room mentions'), From 2bf22967c73fd66be61ece639bc83de11f660466 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 10 May 2020 14:04:43 +0100 Subject: [PATCH 06/15] fix whitespace --- src/ContentMessages.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index 8a12f7a9c67..4c442b8a4c4 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -23,7 +23,6 @@ import dis from './dispatcher'; import {MatrixClientPeg} from './MatrixClientPeg'; import * as sdk from './index'; import { _t } from './languageHandler'; - import Modal from './Modal'; import RoomViewStore from './stores/RoomViewStore'; import encrypt from "browser-encrypt-attachment"; @@ -367,8 +366,8 @@ function uploadFile(matrixClient, roomId, file, progressHandler) { } else { basePromise = matrixClient.uploadContent(file, { - progressHandler, - }); + progressHandler, + }); } const promise1 = basePromise.then(function(url) { // If the attachment isn't encrypted then include the URL directly. From d919a2490f7c8f31b89f3fe1b927850e10b64dee Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 10 May 2020 15:24:20 +0100 Subject: [PATCH 07/15] make it work --- src/ContentMessages.js | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index 4c442b8a4c4..6b30ae25642 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -25,6 +25,7 @@ import * as sdk from './index'; import { _t } from './languageHandler'; import Modal from './Modal'; import RoomViewStore from './stores/RoomViewStore'; +import SettingsStore, {SettingLevel} from "./settings/SettingsStore"; import encrypt from "browser-encrypt-attachment"; import extractPngChunks from "png-chunks-extract"; @@ -269,7 +270,7 @@ function stripJpegMetadata(data) { offset += 2; while (offset < dv.byteLength) { if (app1 == 0xffe1) { - pieces[i] = { recess:recess, offset:offset-2 }; + pieces[i] = { recess: recess, offset: offset - 2 }; recess = offset + dv.getUint16(offset); i++; } @@ -289,7 +290,23 @@ function stripJpegMetadata(data) { } } - return newPieces.length ? new Blob( newPieces, { type: 'image/jpeg' }) : data; + if (newPieces.length) { + // Concatenate the slices back together. + // XXX: is there a more efficient way of doing this? + // Turning them into a blob & back again is apparently slower. + // according to https://stackoverflow.com/a/24549974 + let byteLength = 0; + newPieces.forEach(piece => { byteLength += piece.byteLength }); + data = new Uint8Array(byteLength); + let offset = 0; + newPieces.forEach(piece => { + data.set(new Uint8Array(piece), offset); + offset += piece.byteLength; + }); + data = data.buffer; + } + + return data; } /** @@ -306,11 +323,11 @@ function stripJpegMetadata(data) { * If the file is encrypted then the object will have a "file" key. */ function uploadFile(matrixClient, roomId, file, progressHandler) { - var readDataPromise; + let readDataPromise; // strip metadata where we can (n.b. we don't want to strip colorspace metadata) if (file.type === 'image/jpeg' && - UserSettingsStore.getSyncedSetting('stripFileMetadata', false)) + SettingsStore.getValueAt(SettingLevel.ACCOUNT, 'stripFileMetadata')) { readDataPromise = readFileAsArrayBuffer(file).then(function(data) { // fix up arraybuffer @@ -318,13 +335,18 @@ function uploadFile(matrixClient, roomId, file, progressHandler) { return Promise.resolve(data); }); } + if (matrixClient.isRoomEncrypted(roomId)) { // If the room is encrypted then encrypt the file before uploading it. // First read the file into memory. let canceled = false; let uploadPromise; let encryptInfo; - const prom = readFileAsArrayBuffer(file).then(function(data) { + + if (!readDataPromise) { + readDataPromise = readFileAsArrayBuffer(file); + } + const prom = readDataPromise.then(function(data) { if (canceled) throw new UploadCanceledError(); // Then encrypt the file. return encrypt.encryptAttachment(data); @@ -357,6 +379,8 @@ function uploadFile(matrixClient, roomId, file, progressHandler) { return prom; } else { let basePromise; + // N.B. that if we have no custom readPromise, we just upload the file + // rather than raw data, hence factoring out the concept of a basePromise if (readDataPromise) { basePromise = readDataPromise.then(function(data) { return matrixClient.uploadContent(new Blob([data]), { From 3f2c4de338c5a4531eefed4c7434ac853726f8c3 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 10 May 2020 15:27:47 +0100 Subject: [PATCH 08/15] fascist linter --- src/ContentMessages.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index 6b30ae25642..b9dcf1d607a 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -254,10 +254,11 @@ function readFileAsArrayBuffer(file) { * @return {ArrayBuffer} the stripped image data */ function stripJpegMetadata(data) { - var dv = new DataView(data); - var offset = 0, recess = 0; - var pieces = []; - var i = 0; + const dv = new DataView(data); + let offset = 0; + let recess = 0; + const pieces = []; + let i = 0; const newPieces = []; @@ -266,15 +267,14 @@ function stripJpegMetadata(data) { // this for privacy purposes rather than filesize. if (dv.getUint16(offset) == 0xffd8) { offset += 2; - var app1 = dv.getUint16(offset); + let app1 = dv.getUint16(offset); offset += 2; while (offset < dv.byteLength) { if (app1 == 0xffe1) { pieces[i] = { recess: recess, offset: offset - 2 }; recess = offset + dv.getUint16(offset); i++; - } - else if (app1 == 0xffda) { + } else if (app1 == 0xffda) { break; } offset += dv.getUint16(offset); @@ -296,7 +296,7 @@ function stripJpegMetadata(data) { // Turning them into a blob & back again is apparently slower. // according to https://stackoverflow.com/a/24549974 let byteLength = 0; - newPieces.forEach(piece => { byteLength += piece.byteLength }); + newPieces.forEach(piece => { byteLength += piece.byteLength; }); data = new Uint8Array(byteLength); let offset = 0; newPieces.forEach(piece => { @@ -327,8 +327,7 @@ function uploadFile(matrixClient, roomId, file, progressHandler) { // strip metadata where we can (n.b. we don't want to strip colorspace metadata) if (file.type === 'image/jpeg' && - SettingsStore.getValueAt(SettingLevel.ACCOUNT, 'stripFileMetadata')) - { + SettingsStore.getValueAt(SettingLevel.ACCOUNT, 'stripFileMetadata')) { readDataPromise = readFileAsArrayBuffer(file).then(function(data) { // fix up arraybuffer data = stripJpegMetadata(data); @@ -387,8 +386,7 @@ function uploadFile(matrixClient, roomId, file, progressHandler) { progressHandler, }); }); - } - else { + } else { basePromise = matrixClient.uploadContent(file, { progressHandler, }); From 1bb71de32daccd3c4f3fd8b93e17b5823004ff2e Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 10 May 2020 15:31:53 +0100 Subject: [PATCH 09/15] rename setting as stripJpegMetadata, as that's what it does --- src/ContentMessages.js | 2 +- .../views/settings/tabs/user/PreferencesUserSettingsTab.js | 2 +- src/settings/Settings.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index b9dcf1d607a..9a6d5961903 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -327,7 +327,7 @@ function uploadFile(matrixClient, roomId, file, progressHandler) { // strip metadata where we can (n.b. we don't want to strip colorspace metadata) if (file.type === 'image/jpeg' && - SettingsStore.getValueAt(SettingLevel.ACCOUNT, 'stripFileMetadata')) { + SettingsStore.getValueAt(SettingLevel.ACCOUNT, 'stripJpegMetadata')) { readDataPromise = readFileAsArrayBuffer(file).then(function(data) { // fix up arraybuffer data = stripJpegMetadata(data); diff --git a/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js b/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js index 0a9ff7aaa0a..0509988681d 100644 --- a/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js @@ -56,7 +56,7 @@ export default class PreferencesUserSettingsTab extends React.Component { static ADVANCED_SETTINGS = [ 'alwaysShowEncryptionIcons', 'Pill.shouldShowPillAvatar', - 'stripFileMetadata', + 'stripJpegMetadata', 'TagPanel.enableTagPanel', 'promptBeforeInviteUnknownUsers', // Start automatically after startup (electron-only) diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 411051ca047..160360a8812 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -229,9 +229,9 @@ export const SETTINGS = { displayName: _td('Enable automatic language detection for syntax highlighting'), default: false, }, - "stripFileMetadata": { + "stripJpegMetadata": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, - displayName: _td('Strip metadata from file uploads'), + displayName: _td('Strip metadata from JPEG uploads'), default: true, }, "Pill.shouldShowPillAvatar": { From a246909de960181506c43a897a85db80bb93ab98 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 10 May 2020 15:35:39 +0100 Subject: [PATCH 10/15] i18n --- src/i18n/strings/en_EN.json | 1 + 1 file changed, 1 insertion(+) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 27b4252f770..1cddb147027 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -420,6 +420,7 @@ "Always show encryption icons": "Always show encryption icons", "Show a reminder to enable Secure Message Recovery in encrypted rooms": "Show a reminder to enable Secure Message Recovery in encrypted rooms", "Enable automatic language detection for syntax highlighting": "Enable automatic language detection for syntax highlighting", + "Strip metadata from JPEG uploads": "Strip metadata from JPEG uploads", "Show avatars in user and room mentions": "Show avatars in user and room mentions", "Enable big emoji in chat": "Enable big emoji in chat", "Send typing notifications": "Send typing notifications", From dd794245c3a7ae1a90748c7b45cfd5e2349b21a2 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 10 May 2020 16:18:03 +0100 Subject: [PATCH 11/15] preserve the mime type when uploading stripped jpeg --- src/ContentMessages.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index 9a6d5961903..ea7f300033c 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -382,7 +382,7 @@ function uploadFile(matrixClient, roomId, file, progressHandler) { // rather than raw data, hence factoring out the concept of a basePromise if (readDataPromise) { basePromise = readDataPromise.then(function(data) { - return matrixClient.uploadContent(new Blob([data]), { + return matrixClient.uploadContent(new Blob([data], { type: file.type }), { progressHandler, }); }); From 0db3fa3b5b55d1f1dae443368f7c2666cca2305f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 15 May 2020 00:50:03 +0100 Subject: [PATCH 12/15] s/stripJpegMetadata/stripImageMetadata/ Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/ContentMessages.js | 2 +- .../views/settings/tabs/user/PreferencesUserSettingsTab.js | 2 +- src/settings/Settings.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index ea7f300033c..4d5d41612bf 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -327,7 +327,7 @@ function uploadFile(matrixClient, roomId, file, progressHandler) { // strip metadata where we can (n.b. we don't want to strip colorspace metadata) if (file.type === 'image/jpeg' && - SettingsStore.getValueAt(SettingLevel.ACCOUNT, 'stripJpegMetadata')) { + SettingsStore.getValueAt(SettingLevel.ACCOUNT, 'stripImageMetadata')) { readDataPromise = readFileAsArrayBuffer(file).then(function(data) { // fix up arraybuffer data = stripJpegMetadata(data); diff --git a/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js b/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js index 0509988681d..686251dbda5 100644 --- a/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js +++ b/src/components/views/settings/tabs/user/PreferencesUserSettingsTab.js @@ -56,7 +56,7 @@ export default class PreferencesUserSettingsTab extends React.Component { static ADVANCED_SETTINGS = [ 'alwaysShowEncryptionIcons', 'Pill.shouldShowPillAvatar', - 'stripJpegMetadata', + 'stripImageMetadata', 'TagPanel.enableTagPanel', 'promptBeforeInviteUnknownUsers', // Start automatically after startup (electron-only) diff --git a/src/settings/Settings.js b/src/settings/Settings.js index 160360a8812..cf597c4a49c 100644 --- a/src/settings/Settings.js +++ b/src/settings/Settings.js @@ -229,7 +229,7 @@ export const SETTINGS = { displayName: _td('Enable automatic language detection for syntax highlighting'), default: false, }, - "stripJpegMetadata": { + "stripImageMetadata": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, displayName: _td('Strip metadata from JPEG uploads'), default: true, From 7dbe7362122956a34ab852d5001b321a91570780 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 9 Jul 2021 20:01:49 +0100 Subject: [PATCH 13/15] Tidy up ContentMessages uploadFile method --- src/@types/browser-encrypt-attachment.d.ts | 35 ++++++ src/@types/png-chunks-extract.d.ts | 24 ++++ src/ContentMessages.tsx | 134 ++++++++++++--------- src/i18n/strings/en_EN.json | 1 + src/settings/Settings.tsx | 5 + 5 files changed, 141 insertions(+), 58 deletions(-) create mode 100644 src/@types/browser-encrypt-attachment.d.ts create mode 100644 src/@types/png-chunks-extract.d.ts diff --git a/src/@types/browser-encrypt-attachment.d.ts b/src/@types/browser-encrypt-attachment.d.ts new file mode 100644 index 00000000000..46ac57107d0 --- /dev/null +++ b/src/@types/browser-encrypt-attachment.d.ts @@ -0,0 +1,35 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +declare module "browser-encrypt-attachment" { + interface IEncryptedAttachment { + data: ArrayBuffer; + info: IEncryptedAttachmentInfo; + } + + export interface IEncryptedAttachmentInfo { + key: string; + iv: string; + hashes: { + sha256: string; + }; + } + + function encryptAttachment(plaintextBuffer: ArrayBuffer): Promise; + function decryptAttachment(ciphertextBuffer: ArrayBuffer, info: IEncryptedAttachmentInfo): Promise; + + export { encryptAttachment, decryptAttachment }; +} diff --git a/src/@types/png-chunks-extract.d.ts b/src/@types/png-chunks-extract.d.ts new file mode 100644 index 00000000000..72f25d1e959 --- /dev/null +++ b/src/@types/png-chunks-extract.d.ts @@ -0,0 +1,24 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +declare module "png-chunks-extract" { + interface IChunk { + name: string; + data: Uint8Array; + } + + export default function extractPngChunks(data: Uint8Array): IChunk[]; +} diff --git a/src/ContentMessages.tsx b/src/ContentMessages.tsx index c44d5a4985b..d9bbc96c2d1 100644 --- a/src/ContentMessages.tsx +++ b/src/ContentMessages.tsx @@ -19,17 +19,18 @@ limitations under the License. import React from "react"; import { encode } from "blurhash"; import { MatrixClient } from "matrix-js-sdk/src/client"; +import { IImageInfo } from "matrix-js-sdk/src/@types/partials"; +import { IUploadOpts } from "matrix-js-sdk/src/@types/requests"; +import encrypt, { IEncryptedAttachmentInfo } from "browser-encrypt-attachment"; +import extractPngChunks from "png-chunks-extract"; import dis from './dispatcher/dispatcher'; import * as sdk from './index'; import { _t } from './languageHandler'; import Modal from './Modal'; import RoomViewStore from './stores/RoomViewStore'; -import SettingsStore, {SettingLevel} from "./settings/SettingsStore"; -import encrypt from "browser-encrypt-attachment"; -import extractPngChunks from "png-chunks-extract"; +import SettingsStore from "./settings/SettingsStore"; import Spinner from "./components/views/elements/Spinner"; - import { Action } from "./dispatcher/actions"; import CountlyAnalytics from "./CountlyAnalytics"; import { @@ -40,7 +41,6 @@ import { UploadStartedPayload, } from "./dispatcher/payloads/UploadPayload"; import { IUpload } from "./models/IUpload"; -import { IImageInfo } from "matrix-js-sdk/src/@types/partials"; const MAX_WIDTH = 800; const MAX_HEIGHT = 600; @@ -367,18 +367,28 @@ function stripJpegMetadata(data: ArrayBuffer): ArrayBuffer { // according to https://stackoverflow.com/a/24549974 let byteLength = 0; newPieces.forEach(piece => { byteLength += piece.byteLength; }); - data = new Uint8Array(byteLength); + const uArray = new Uint8Array(byteLength); let offset = 0; newPieces.forEach(piece => { - data.set(new Uint8Array(piece), offset); + uArray.set(new Uint8Array(piece), offset); offset += piece.byteLength; }); - data = data.buffer; + data = uArray.buffer; } return data; } +interface IEncryptedFile extends IEncryptedAttachmentInfo { + url: string; + mimetype?: string; +} + +interface IUploadedFile { + url?: string; + file?: IEncryptedFile; +} + /** * Upload the file to the content repository. * If the room is encrypted then encrypt the file before uploading. @@ -396,41 +406,64 @@ export function uploadFile( matrixClient: MatrixClient, roomId: string, file: File | Blob, - progressHandler?: any, // TODO: Types -): Promise<{url?: string, file?: any}> { // TODO: Types - let readDataPromise: Promise; + progressHandler?: IUploadOpts["progressHandler"], +): Promise { + // uploadPromise must be a reference to the exact promise returned from MatrixClient::uploadContent + // this promise is special as it bears an `abort` method we must copy to `promise` which we return. + let uploadPromise: IAbortablePromise; + let promise: Promise; // we will make this an abortable promise before we return it + + const isRoomEncrypted = matrixClient.isRoomEncrypted(roomId); + const shouldStripMetadata = file.type === "image/jpeg" && SettingsStore.getValue("stripImageMetadata"); + + // if the room isn't encrypted and we have no metadata to strip then upload without loading into memory here + // this path is handled first to simplify the rest of the flow which has to load the file into an ArrayBuffer + if (!isRoomEncrypted && !shouldStripMetadata) { + uploadPromise = matrixClient.uploadContent(file, { progressHandler }) as IAbortablePromise; + promise = uploadPromise.then(url => { + // If the attachment isn't encrypted then include the URL directly. + return { url }; + }) as IAbortablePromise; - // strip metadata where we can (n.b. we don't want to strip colorspace metadata) - if (file.type === 'image/jpeg' && SettingsStore.getValue('stripImageMetadata')) { - readDataPromise = readFileAsArrayBuffer(file).then(stripJpegMetadata); + // XXX: copy over the abort method to the new promise + (promise as IAbortablePromise).abort = uploadPromise.abort; + return promise; } + // Track if the abort method has been called, bail out of any work early if it has. let canceled = false; - if (matrixClient.isRoomEncrypted(roomId)) { - // If the room is encrypted then encrypt the file before uploading it. - // First read the file into memory. - let uploadPromise; - let encryptInfo; - - if (!readDataPromise) { - readDataPromise = readFileAsArrayBuffer(file); - } - const prom = readDataPromise.then(function(data) { + + // First read the file into memory. + let dataPromise = readFileAsArrayBuffer(file); + + // Strip metadata where we can (n.b. we don't want to strip colorspace metadata) + if (shouldStripMetadata) { + dataPromise = dataPromise.then(data => { + if (canceled) throw new UploadCanceledError(); + return stripJpegMetadata(data); + }); + } + + if (isRoomEncrypted) { + let encryptInfo: IEncryptedFile; + + promise = dataPromise.then(data => { if (canceled) throw new UploadCanceledError(); // Then encrypt the file. return encrypt.encryptAttachment(data); - }).then(function(encryptResult) { + }).then(encryptResult => { if (canceled) throw new UploadCanceledError(); // Record the information needed to decrypt the attachment. - encryptInfo = encryptResult.info; + encryptInfo = encryptResult.info as IEncryptedFile; + // Pass the encrypted data as a Blob to the uploader. const blob = new Blob([encryptResult.data]); uploadPromise = matrixClient.uploadContent(blob, { - progressHandler: progressHandler, + progressHandler, includeFilename: false, - }); + }) as IAbortablePromise; return uploadPromise; - }).then(function(url) { + }).then(url => { if (canceled) throw new UploadCanceledError(); // If the attachment is encrypted then bundle the URL along // with the information needed to decrypt the attachment and @@ -439,41 +472,26 @@ export function uploadFile( if (file.type) { encryptInfo.mimetype = file.type; } - return { "file": encryptInfo }; + return { file: encryptInfo }; }); - (prom as IAbortablePromise).abort = () => { - canceled = true; - if (uploadPromise) matrixClient.cancelUpload(uploadPromise); - }; - return prom; } else { - let basePromise; - // N.B. that if we have no custom readPromise, we just upload the file - // rather than raw data, hence factoring out the concept of a basePromise - if (readDataPromise) { - basePromise = readDataPromise.then(function(data) { - return matrixClient.uploadContent(new Blob([data], { type: file.type }), { - progressHandler, - }); - }); - } else { - basePromise = matrixClient.uploadContent(file, { - progressHandler, - }); - } - const promise1 = basePromise.then(function(url) { + promise = dataPromise.then(data => { + const blob = new Blob([data], { type: file.type }); + uploadPromise = matrixClient.uploadContent(blob, { progressHandler }) as IAbortablePromise; + return uploadPromise; + }).then(url => { if (canceled) throw new UploadCanceledError(); - // If the attachment isn't encrypted then include the URL directly. return { url }; }); - // XXX: copy over the abort method to the new promise - // FIXME: This is probably broken if you have a readDataPromise defined - (promise1 as any).abort = () => { - canceled = true; - matrixClient.cancelUpload(basePromise); - }; - return promise1; } + + // XXX: wrap the original abort method onto the new promise, this way we can use the signal to bail work early + (promise as IAbortablePromise).abort = () => { + canceled = true; + if (uploadPromise) matrixClient.cancelUpload(uploadPromise); + }; + + return promise; } export default class ContentMessages { diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 7795bb26100..dac844071e7 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -836,6 +836,7 @@ "Expand code blocks by default": "Expand code blocks by default", "Show line numbers in code blocks": "Show line numbers in code blocks", "Jump to the bottom of the timeline when you send a message": "Jump to the bottom of the timeline when you send a message", + "Strip metadata from JPEG uploads": "Strip metadata from JPEG uploads", "Show avatars in user and room mentions": "Show avatars in user and room mentions", "Enable big emoji in chat": "Enable big emoji in chat", "Send typing notifications": "Send typing notifications", diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 1751eddb2cb..237eadb3b17 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -422,6 +422,11 @@ export const SETTINGS: {[setting: string]: ISetting} = { displayName: _td('Jump to the bottom of the timeline when you send a message'), default: true, }, + "stripImageMetadata": { + supportedLevels: LEVELS_ACCOUNT_SETTINGS, + displayName: _td('Strip metadata from JPEG uploads'), + default: true, + }, "Pill.shouldShowPillAvatar": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, displayName: _td('Show avatars in user and room mentions'), From 443364f6f49e5e5cb77a213dd754b087ba4432b8 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 9 Jul 2021 20:23:05 +0100 Subject: [PATCH 14/15] Fix typing --- src/@types/browser-encrypt-attachment.d.ts | 20 +++++++++------- src/ContentMessages.tsx | 24 +++++++------------ .../models/IMediaEventContent.ts | 17 ++----------- src/utils/DecryptFile.ts | 3 ++- 4 files changed, 25 insertions(+), 39 deletions(-) diff --git a/src/@types/browser-encrypt-attachment.d.ts b/src/@types/browser-encrypt-attachment.d.ts index 46ac57107d0..0c322c59c2e 100644 --- a/src/@types/browser-encrypt-attachment.d.ts +++ b/src/@types/browser-encrypt-attachment.d.ts @@ -15,19 +15,23 @@ limitations under the License. */ declare module "browser-encrypt-attachment" { + interface IEncryptedAttachmentInfo { + key: { + alg: string; + key_ops: string[]; // eslint-disable-line camelcase + kty: string; + k: string; + ext: boolean; + }; + iv: string; + hashes: { [alg: string]: string }; + } + interface IEncryptedAttachment { data: ArrayBuffer; info: IEncryptedAttachmentInfo; } - export interface IEncryptedAttachmentInfo { - key: string; - iv: string; - hashes: { - sha256: string; - }; - } - function encryptAttachment(plaintextBuffer: ArrayBuffer): Promise; function decryptAttachment(ciphertextBuffer: ArrayBuffer, info: IEncryptedAttachmentInfo): Promise; diff --git a/src/ContentMessages.tsx b/src/ContentMessages.tsx index d9bbc96c2d1..7ed2f7c5a82 100644 --- a/src/ContentMessages.tsx +++ b/src/ContentMessages.tsx @@ -21,7 +21,8 @@ import { encode } from "blurhash"; import { MatrixClient } from "matrix-js-sdk/src/client"; import { IImageInfo } from "matrix-js-sdk/src/@types/partials"; import { IUploadOpts } from "matrix-js-sdk/src/@types/requests"; -import encrypt, { IEncryptedAttachmentInfo } from "browser-encrypt-attachment"; +import { IEncryptedFile } from "matrix-js-sdk/src/@types/event"; +import encrypt from "browser-encrypt-attachment"; import extractPngChunks from "png-chunks-extract"; import dis from './dispatcher/dispatcher'; @@ -41,6 +42,7 @@ import { UploadStartedPayload, } from "./dispatcher/payloads/UploadPayload"; import { IUpload } from "./models/IUpload"; +import { IMediaEventContent } from "./customisations/models/IMediaEventContent"; const MAX_WIDTH = 800; const MAX_HEIGHT = 600; @@ -379,15 +381,7 @@ function stripJpegMetadata(data: ArrayBuffer): ArrayBuffer { return data; } -interface IEncryptedFile extends IEncryptedAttachmentInfo { - url: string; - mimetype?: string; -} - -interface IUploadedFile { - url?: string; - file?: IEncryptedFile; -} +type UploadedFile = Pick; /** * Upload the file to the content repository. @@ -407,11 +401,11 @@ export function uploadFile( roomId: string, file: File | Blob, progressHandler?: IUploadOpts["progressHandler"], -): Promise { +): Promise { // uploadPromise must be a reference to the exact promise returned from MatrixClient::uploadContent // this promise is special as it bears an `abort` method we must copy to `promise` which we return. let uploadPromise: IAbortablePromise; - let promise: Promise; // we will make this an abortable promise before we return it + let promise: Promise; // we will make this an abortable promise before we return it const isRoomEncrypted = matrixClient.isRoomEncrypted(roomId); const shouldStripMetadata = file.type === "image/jpeg" && SettingsStore.getValue("stripImageMetadata"); @@ -423,10 +417,10 @@ export function uploadFile( promise = uploadPromise.then(url => { // If the attachment isn't encrypted then include the URL directly. return { url }; - }) as IAbortablePromise; + }) as IAbortablePromise; // XXX: copy over the abort method to the new promise - (promise as IAbortablePromise).abort = uploadPromise.abort; + (promise as IAbortablePromise).abort = uploadPromise.abort; return promise; } @@ -486,7 +480,7 @@ export function uploadFile( } // XXX: wrap the original abort method onto the new promise, this way we can use the signal to bail work early - (promise as IAbortablePromise).abort = () => { + (promise as IAbortablePromise).abort = () => { canceled = true; if (uploadPromise) matrixClient.cancelUpload(uploadPromise); }; diff --git a/src/customisations/models/IMediaEventContent.ts b/src/customisations/models/IMediaEventContent.ts index fb05d76a4d5..599d9f2d3d0 100644 --- a/src/customisations/models/IMediaEventContent.ts +++ b/src/customisations/models/IMediaEventContent.ts @@ -14,22 +14,9 @@ * limitations under the License. */ -// TODO: These types should be elsewhere. +import { IEncryptedFile } from "matrix-js-sdk/src/@types/event"; -export interface IEncryptedFile { - url: string; - mimetype?: string; - key: { - alg: string; - key_ops: string[]; // eslint-disable-line camelcase - kty: string; - k: string; - ext: boolean; - }; - iv: string; - hashes: {[alg: string]: string}; - v: string; -} +// TODO: These types should be elsewhere. export interface IMediaEventContent { url?: string; // required on unencrypted media diff --git a/src/utils/DecryptFile.ts b/src/utils/DecryptFile.ts index e66db4ffb2d..b82f648ac4b 100644 --- a/src/utils/DecryptFile.ts +++ b/src/utils/DecryptFile.ts @@ -16,8 +16,9 @@ limitations under the License. // Pull in the encryption lib so that we can decrypt attachments. import encrypt from 'browser-encrypt-attachment'; +import { IEncryptedFile } from 'matrix-js-sdk/src/@types/event'; + import { mediaFromContent } from "../customisations/Media"; -import { IEncryptedFile } from "../customisations/models/IMediaEventContent"; import { getBlobSafeMimeType } from "./blobs"; /** From 85e82c49653b4f4c24774a157bb8096e4a457dc6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 9 Jul 2021 22:39:30 +0100 Subject: [PATCH 15/15] make code more readable --- src/@types/browser-encrypt-attachment.d.ts | 9 ++--- src/ContentMessages.tsx | 38 ++++++++++++++-------- src/utils/DecryptFile.ts | 2 +- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/@types/browser-encrypt-attachment.d.ts b/src/@types/browser-encrypt-attachment.d.ts index 0c322c59c2e..cb78411a306 100644 --- a/src/@types/browser-encrypt-attachment.d.ts +++ b/src/@types/browser-encrypt-attachment.d.ts @@ -32,8 +32,9 @@ declare module "browser-encrypt-attachment" { info: IEncryptedAttachmentInfo; } - function encryptAttachment(plaintextBuffer: ArrayBuffer): Promise; - function decryptAttachment(ciphertextBuffer: ArrayBuffer, info: IEncryptedAttachmentInfo): Promise; - - export { encryptAttachment, decryptAttachment }; + export function encryptAttachment(plaintextBuffer: ArrayBuffer): Promise; + export function decryptAttachment( + ciphertextBuffer: ArrayBuffer, + info: IEncryptedAttachmentInfo, + ): Promise; } diff --git a/src/ContentMessages.tsx b/src/ContentMessages.tsx index 7ed2f7c5a82..26ef4b25199 100644 --- a/src/ContentMessages.tsx +++ b/src/ContentMessages.tsx @@ -22,7 +22,7 @@ import { MatrixClient } from "matrix-js-sdk/src/client"; import { IImageInfo } from "matrix-js-sdk/src/@types/partials"; import { IUploadOpts } from "matrix-js-sdk/src/@types/requests"; import { IEncryptedFile } from "matrix-js-sdk/src/@types/event"; -import encrypt from "browser-encrypt-attachment"; +import * as encrypt from "browser-encrypt-attachment"; import extractPngChunks from "png-chunks-extract"; import dis from './dispatcher/dispatcher'; @@ -318,6 +318,15 @@ function readFileAsArrayBuffer(file: File | Blob): Promise { }); } +interface IChunk { + begin: number; + end: number; +} + +const JPEG_SOI_MARKER = 0xFFD8; +const JPEG_APP1_MARKER = 0xFFE1; +const JPEG_SOS_MARKER = 0xFFDA; + /** * Strip EXIF metadata from a JPEG * Taken from http://jsfiddle.net/mowglisanu/frhwm2xe/3/ @@ -329,35 +338,38 @@ function stripJpegMetadata(data: ArrayBuffer): ArrayBuffer { const dv = new DataView(data); let offset = 0; let recess = 0; - const pieces = []; + const pieces: IChunk[] = []; let i = 0; + let blockSize: number; - const newPieces = []; + const newPieces: ArrayBuffer[] = []; // FIXME: check this isn't stripping off any EXIF color profile data // as that will break the colorimetry of the image. We're stripping // this for privacy purposes rather than filesize. - if (dv.getUint16(offset) == 0xffd8) { + if (dv.getUint16(offset) === JPEG_SOI_MARKER) { offset += 2; let app1 = dv.getUint16(offset); offset += 2; while (offset < dv.byteLength) { - if (app1 == 0xffe1) { - pieces[i] = { recess: recess, offset: offset - 2 }; - recess = offset + dv.getUint16(offset); - i++; - } else if (app1 == 0xffda) { + blockSize = dv.getUint16(offset); + if (app1 === JPEG_APP1_MARKER) { + // if the marker we are in is an APP1 marker then mark it for extraction + pieces[i++] = { begin: recess, end: offset - 2 }; + recess = offset + blockSize; + } else if (app1 === JPEG_SOS_MARKER) { break; } - offset += dv.getUint16(offset); + offset += blockSize; // jump to the next marker app1 = dv.getUint16(offset); - offset += 2; + offset += 2; // enter the next marker block } if (pieces.length > 0) { - pieces.forEach(function(v) { - newPieces.push(data.slice(v.recess, v.offset)); + pieces.forEach(piece => { + newPieces.push(data.slice(piece.begin, piece.end)); }); + newPieces.push(data.slice(recess)); } } diff --git a/src/utils/DecryptFile.ts b/src/utils/DecryptFile.ts index b82f648ac4b..73a656a2a04 100644 --- a/src/utils/DecryptFile.ts +++ b/src/utils/DecryptFile.ts @@ -15,7 +15,7 @@ limitations under the License. */ // Pull in the encryption lib so that we can decrypt attachments. -import encrypt from 'browser-encrypt-attachment'; +import * as encrypt from 'browser-encrypt-attachment'; import { IEncryptedFile } from 'matrix-js-sdk/src/@types/event'; import { mediaFromContent } from "../customisations/Media";