From 00508367f6c90a8b9cd931aad70ae8d886941beb Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 15 May 2019 13:47:48 +0100 Subject: [PATCH 1/7] Fix Single Sign-on https://github.com/matrix-org/matrix-react-sdk/pull/2826 checked that we had data in the crypto store if the had credentials in localStorage. However, SSO stores creds in localStorage and then redirects the browser to remove the loginToken parameter from the URL without starting crypto, so after the redirect, we see creds in localStorage but no crypto data, and error. Fix by marking when we've successfully initialised crypto and only erroring if that flag is set. Fixes https://github.com/vector-im/riot-web/issues/9695 --- src/Lifecycle.js | 2 +- src/MatrixClientPeg.js | 1 + src/utils/StorageManager.js | 22 ++++++++++++++++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/Lifecycle.js b/src/Lifecycle.js index 0e2389fd1c..a7f90f847d 100644 --- a/src/Lifecycle.js +++ b/src/Lifecycle.js @@ -369,7 +369,7 @@ async function _doSetLoggedIn(credentials, clearStorage) { // If there's an inconsistency between account data in local storage and the // crypto store, we'll be generally confused when handling encrypted data. // Show a modal recommending a full reset of storage. - if (results.dataInLocalStorage && !results.dataInCryptoStore) { + if (results.dataInLocalStorage && results.cryptoInited && !results.dataInCryptoStore) { const signOut = await _showStorageEvictedDialog(); if (signOut) { await _clearStorage(); diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index 763eddbd5d..d3cf19ef48 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -121,6 +121,7 @@ class MatrixClientPeg { // check that we have a version of the js-sdk which includes initCrypto if (this.matrixClient.initCrypto) { await this.matrixClient.initCrypto(); + StorageManager.setCryptoInitialised(true); } } catch (e) { if (e && e.name === 'InvalidCryptoStoreError') { diff --git a/src/utils/StorageManager.js b/src/utils/StorageManager.js index 1c0931273b..d0fcd58106 100644 --- a/src/utils/StorageManager.js +++ b/src/utils/StorageManager.js @@ -50,11 +50,15 @@ export async function checkConsistency() { let dataInLocalStorage = false; let dataInCryptoStore = false; + let cryptoInited = false; let healthy = true; if (localStorage) { dataInLocalStorage = localStorage.length > 0; log(`Local storage contains data? ${dataInLocalStorage}`); + + cryptoInited = localStorage.getItem("mx_crypto_initialised"); + log(`Crypto initialised? ${cryptoInited}`); } else { healthy = false; error("Local storage cannot be used on this browser"); @@ -84,10 +88,11 @@ export async function checkConsistency() { track("Crypto store disabled"); } - if (dataInLocalStorage && !dataInCryptoStore) { + if (dataInLocalStorage && cryptoInited && !dataInCryptoStore) { healthy = false; error( - "Data exists in local storage but not in crypto store. " + + "Data exists in local storage and crypto is marked as initialised " + + " but no data found in crypto store. " + "IndexedDB storage has likely been evicted by the browser!", ); track("Crypto store evicted"); @@ -104,6 +109,7 @@ export async function checkConsistency() { return { dataInLocalStorage, dataInCryptoStore, + cryptoInited, healthy, }; } @@ -155,3 +161,15 @@ export function trackStores(client) { }); } } + +/** + * Sets whether crypto has ever been successfully + * initialised on this client. + * StorageManager uses this to determine whether indexeddb + * has been wiped by the browser: this flag is saved to localStorage + * and if it is true and not crypto data is found, an error is + * presented to the user. + */ +export function setCryptoInitialised(cryptoInited) { + localStorage.setItem("mx_crypto_initialised", cryptoInited); +} From 0cde4bf2cd7f50ec8c06a3a8a07884b570721c3f Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 15 May 2019 14:00:37 +0100 Subject: [PATCH 2/7] lint --- src/utils/StorageManager.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/utils/StorageManager.js b/src/utils/StorageManager.js index d0fcd58106..49a120a470 100644 --- a/src/utils/StorageManager.js +++ b/src/utils/StorageManager.js @@ -169,6 +169,8 @@ export function trackStores(client) { * has been wiped by the browser: this flag is saved to localStorage * and if it is true and not crypto data is found, an error is * presented to the user. + * + * @param {bool} cryptoInited True if crypto has been set up */ export function setCryptoInitialised(cryptoInited) { localStorage.setItem("mx_crypto_initialised", cryptoInited); From 3ea187a524ca6eaa17417407043b4b9dc6a3d7df Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Mon, 13 May 2019 18:00:52 +0100 Subject: [PATCH 3/7] Save `content.info` as a local for readability --- src/components/views/messages/MImageBody.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index 9fd42fb31d..cb12259c9b 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -198,9 +198,11 @@ export default class MImageBody extends React.Component { // so we'll need to download the original image for this to work // well for now. First, let's try a few cases that let us avoid // downloading the original: - if (pixelRatio === 1.0 || - (!content.info || !content.info.w || - !content.info.h || !content.info.size)) { + const info = content.info; + if ( + pixelRatio === 1.0 || + (!info || !info.w || !info.h || !info.size) + ) { // always thumbnail. it may look a bit worse, but it'll save bandwidth. // which is probably desirable on a lo-dpi device anyway. return this.context.matrixClient.mxcUrlToHttp(content.url, thumbWidth, thumbHeight); @@ -215,10 +217,10 @@ export default class MImageBody extends React.Component { // timeline (e.g. >1MB). const isLargerThanThumbnail = ( - content.info.w > thumbWidth || - content.info.h > thumbHeight + info.w > thumbWidth || + info.h > thumbHeight ); - const isLargeFileSize = content.info.size > 1*1024*1024; + const isLargeFileSize = info.size > 1*1024*1024; if (isLargeFileSize && isLargerThanThumbnail) { // image is too large physically and bytewise to clutter our timeline so From 6ea590cf1fef8ece21c532ba692e8bd5b1f6c2dc Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Mon, 13 May 2019 18:28:57 +0100 Subject: [PATCH 4/7] Always thumbnail for GIFs When displaying a GIF, we always want to thumbnail so that we can properly respect the user's GIF autoplay setting (which relies on thumbnailing to produce the static preview image). Fixes https://github.com/vector-im/riot-web/issues/9658 --- src/components/views/messages/MImageBody.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index cb12259c9b..4b5e1c20fa 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -200,6 +200,7 @@ export default class MImageBody extends React.Component { // downloading the original: const info = content.info; if ( + this._isGif() || pixelRatio === 1.0 || (!info || !info.w || !info.h || !info.size) ) { From ef5ac27ddc969a01eb90df474fa1484232cf8e98 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 14 May 2019 13:55:38 +0100 Subject: [PATCH 5/7] Add comment about thumbnailing for GIFs --- src/components/views/messages/MImageBody.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/components/views/messages/MImageBody.js b/src/components/views/messages/MImageBody.js index 4b5e1c20fa..2f12022140 100644 --- a/src/components/views/messages/MImageBody.js +++ b/src/components/views/messages/MImageBody.js @@ -197,15 +197,18 @@ export default class MImageBody extends React.Component { // synapse only supports 800x600 thumbnails for now though, // so we'll need to download the original image for this to work // well for now. First, let's try a few cases that let us avoid - // downloading the original: + // downloading the original, including: + // - When displaying a GIF, we always want to thumbnail so that we can + // properly respect the user's GIF autoplay setting (which relies on + // thumbnailing to produce the static preview image) + // - On a low DPI device, always thumbnail to save bandwidth + // - If there's no sizing info in the event, default to thumbnail const info = content.info; if ( this._isGif() || pixelRatio === 1.0 || (!info || !info.w || !info.h || !info.size) ) { - // always thumbnail. it may look a bit worse, but it'll save bandwidth. - // which is probably desirable on a lo-dpi device anyway. return this.context.matrixClient.mxcUrlToHttp(content.url, thumbWidth, thumbHeight); } else { // we should only request thumbnails if the image is bigger than 800x600 From 77384497b0b3934e4f9b6fa5884f6abcc6e83465 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 15 May 2019 14:30:20 +0100 Subject: [PATCH 6/7] Prepare changelog for v1.1.2 --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 988a85fd43..245d0c7e60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +Changes in [1.1.2](https://github.com/matrix-org/matrix-react-sdk/releases/tag/v1.1.2) (2019-05-15) +=================================================================================================== +[Full Changelog](https://github.com/matrix-org/matrix-react-sdk/compare/v1.1.1...v1.1.2) + + * Always thumbnail for GIFs + [\#2976](https://github.com/matrix-org/matrix-react-sdk/pull/2976) + * Fix Single Sign-on + [\#2975](https://github.com/matrix-org/matrix-react-sdk/pull/2975) + Changes in [1.1.1](https://github.com/matrix-org/matrix-react-sdk/releases/tag/v1.1.1) (2019-05-14) =================================================================================================== [Full Changelog](https://github.com/matrix-org/matrix-react-sdk/compare/v1.1.0...v1.1.1) From 4e3c39cd60f35fadbe87ed80b9a462e65affb63d Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 15 May 2019 14:30:21 +0100 Subject: [PATCH 7/7] v1.1.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index df51ac6526..f008366e87 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "matrix-react-sdk", - "version": "1.1.1", + "version": "1.1.2", "description": "SDK for matrix.org using React", "author": "matrix.org", "repository": {