From c99f5c0df87a0894c61888a057a280019326aa99 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 3 May 2018 16:28:46 +0100 Subject: [PATCH 1/8] Check upload limits before trying to upload large files --- src/components/views/rooms/MessageComposer.js | 63 +++++++++++++++---- src/i18n/strings/en_EN.json | 5 +- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index bac996e65c..ae72908b82 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -16,6 +16,7 @@ limitations under the License. */ import React from 'react'; import PropTypes from 'prop-types'; +import filesize from "filesize"; import { _t } from '../../../languageHandler'; import CallHandler from '../../../CallHandler'; import MatrixClientPeg from '../../../MatrixClientPeg'; @@ -97,18 +98,40 @@ export default class MessageComposer extends React.Component { } onUploadFileSelected(files) { - this.uploadFiles(files.target.files); + const tfiles = files.target.files; + MatrixClientPeg.get().getMediaLimits().then((limits) => { + this.uploadFiles(tfiles, limits); + }); } - uploadFiles(files) { + isFileUploadAllowed(file, limits) { + const sizeLimit = limits.size || -1; + if (sizeLimit !== -1 && file.size > sizeLimit) { + return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(sizeLimit)}); + } + return true; + } + + uploadFiles(files, limits) { const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); const TintableSvg = sdk.getComponent("elements.TintableSvg"); const fileList = []; + const acceptedFiles = []; + const failedFiles = []; + for (let i=0; i - { files[i].name || _t('Attachment') } - ); + const fileAcceptedOrError = this.isFileUploadAllowed(files[i], limits); + if (fileAcceptedOrError === true) { + acceptedFiles.push(
  • + { files[i].name || _t('Attachment') } +
    • ); + fileList.push(files[i]); + } else { + failedFiles.push(
  • + { files[i].name || _t('Attachment') } { _t('Reason') + ": " + fileAcceptedOrError} +
    • ); + } } const isQuoting = Boolean(RoomViewStore.getQuotingEvent()); @@ -119,23 +142,39 @@ export default class MessageComposer extends React.Component { }

    ; } + const acceptedFilesPart = acceptedFiles.length === 0 ? null : ( +
    +

    { _t('Are you sure you want to upload the following files?') }

    +
      + { acceptedFiles } +
    +
    + ); + + const failedFilesPart = failedFiles.length === 0 ? null : ( +
    +

    { _t('The following files cannot be uploaded:') }

    +
      + { failedFiles } +
    +
    + ); + Modal.createTrackedDialog('Upload Files confirmation', '', QuestionDialog, { title: _t('Upload Files'), description: (
    -

    { _t('Are you sure you want to upload the following files?') }

    -
      - { fileList } -
    + { acceptedFilesPart } + { failedFilesPart } { replyToWarning }
    ), onFinished: (shouldUpload) => { if (shouldUpload) { // MessageComposer shouldn't have to rely on its parent passing in a callback to upload a file - if (files) { - for (let i=0; i Date: Thu, 3 May 2018 16:37:25 +0100 Subject: [PATCH 2/8] Linting --- src/components/views/rooms/MessageComposer.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index ae72908b82..8194240319 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -101,13 +101,15 @@ export default class MessageComposer extends React.Component { const tfiles = files.target.files; MatrixClientPeg.get().getMediaLimits().then((limits) => { this.uploadFiles(tfiles, limits); + }).catch(() => { + // HS can't or won't provide limits, so don't give any. + this.uploadFiles(tfiles, {}); }); } isFileUploadAllowed(file, limits) { - const sizeLimit = limits.size || -1; - if (sizeLimit !== -1 && file.size > sizeLimit) { - return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(sizeLimit)}); + if (limits.size != null && file.size > limits.size) { + return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(limits.size)}); } return true; } From 7c0811dd921d37f8835d7539aa35e47d315e0a49 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 3 May 2018 17:02:37 +0100 Subject: [PATCH 3/8] size > upload_size as per spec feedback --- src/components/views/rooms/MessageComposer.js | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index 8194240319..7d4b4f690f 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -65,6 +65,13 @@ export default class MessageComposer extends React.Component { // XXX: fragile as all hell - fixme somehow, perhaps with a dedicated Room.encryption event or something. MatrixClientPeg.get().on("event", this.onEvent); this._roomStoreToken = RoomViewStore.addListener(this._onRoomViewStoreUpdate); + + MatrixClientPeg.get().getMediaLimits().then((limits) => { + this._uploadLimits = limits; + }).catch(() => { + // HS can't or won't provide limits, so don't give any. + this._uploadLimits = {}; + }) } componentWillUnmount() { @@ -99,17 +106,12 @@ export default class MessageComposer extends React.Component { onUploadFileSelected(files) { const tfiles = files.target.files; - MatrixClientPeg.get().getMediaLimits().then((limits) => { - this.uploadFiles(tfiles, limits); - }).catch(() => { - // HS can't or won't provide limits, so don't give any. - this.uploadFiles(tfiles, {}); - }); + this.uploadFiles(tfiles); } - isFileUploadAllowed(file, limits) { - if (limits.size != null && file.size > limits.size) { - return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(limits.size)}); + isFileUploadAllowed(file) { + if (this._uploadLimits.upload_size != null && file.size > this._uploadLimits.upload_size) { + return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(this._uploadLimits.upload_size)}); } return true; } @@ -123,7 +125,7 @@ export default class MessageComposer extends React.Component { const failedFiles = []; for (let i=0; i { files[i].name || _t('Attachment') } From 76f0f15e491ad72249e9c8911890bbb02e94d849 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 21 Jun 2018 09:36:41 +0100 Subject: [PATCH 4/8] Fix nitpicks --- src/components/views/rooms/MessageComposer.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index 7d4b4f690f..bb8f7d78d2 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -16,7 +16,7 @@ limitations under the License. */ import React from 'react'; import PropTypes from 'prop-types'; -import filesize from "filesize"; +import filesize from 'filesize'; import { _t } from '../../../languageHandler'; import CallHandler from '../../../CallHandler'; import MatrixClientPeg from '../../../MatrixClientPeg'; @@ -110,13 +110,13 @@ export default class MessageComposer extends React.Component { } isFileUploadAllowed(file) { - if (this._uploadLimits.upload_size != null && file.size > this._uploadLimits.upload_size) { + if (this._uploadLimits.upload_size !== undefined && file.size > this._uploadLimits.upload_size) { return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(this._uploadLimits.upload_size)}); } return true; } - uploadFiles(files, limits) { + uploadFiles(files) { const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); const TintableSvg = sdk.getComponent("elements.TintableSvg"); From caf2086585110de7e606e34118e42bda01f234f0 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 21 Jun 2018 10:08:41 +0100 Subject: [PATCH 5/8] Restructure limits to be set at RoomView, so they may change if the upload fails --- src/components/structures/RoomView.js | 23 ++++++++++++++++++- src/components/views/rooms/MessageComposer.js | 17 ++++++-------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 4beafb099c..cb6b94c2e6 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -49,6 +49,8 @@ import SettingsStore, {SettingLevel} from "../../settings/SettingsStore"; const DEBUG = false; let debuglog = function() {}; +let mediaLimitCache = null; + const BROWSER_SUPPORTS_SANDBOX = 'sandbox' in document.createElement('iframe'); if (DEBUG) { @@ -94,6 +96,9 @@ module.exports = React.createClass({ roomLoading: true, peekLoading: false, shouldPeek: true, + + // Media limits for uploading, this may change. + mediaLimits: {}, // The event to be scrolled to initially initialEventId: null, @@ -147,12 +152,26 @@ module.exports = React.createClass({ MatrixClientPeg.get().on("RoomState.members", this.onRoomStateMember); MatrixClientPeg.get().on("RoomMember.membership", this.onRoomMemberMembership); MatrixClientPeg.get().on("accountData", this.onAccountData); - + this._fetchMediaLimits(); // Start listening for RoomViewStore updates this._roomStoreToken = RoomViewStore.addListener(this._onRoomViewStoreUpdate); this._onRoomViewStoreUpdate(true); }, + _fetchMediaLimits: function(invalidateCache: boolean = false) { + let limits; + if(invalidateCache || mediaLimitCache == null) { + MatrixClientPeg.get().getMediaLimits().then((_limits) => { + limits = _limits; + }).catch(() => { + // Media repo can't or won't report limits, so provide an empty object (no limits). + limits = {}; + }); + mediaLimitCache = limits; + } + this.state.mediaLimits = limits; + }, + _onRoomViewStoreUpdate: function(initial) { if (this.unmounted) { return; @@ -481,6 +500,7 @@ module.exports = React.createClass({ break; case 'notifier_enabled': case 'upload_failed': + this._fetchMediaLimits(true); case 'upload_started': case 'upload_finished': this.forceUpdate(); @@ -1654,6 +1674,7 @@ module.exports = React.createClass({ callState={this.state.callState} disabled={this.props.disabled} showApps={this.state.showApps} + mediaLimits={this.state.mediaLimits} />; } diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index bb8f7d78d2..1a80a0be4f 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -65,13 +65,6 @@ export default class MessageComposer extends React.Component { // XXX: fragile as all hell - fixme somehow, perhaps with a dedicated Room.encryption event or something. MatrixClientPeg.get().on("event", this.onEvent); this._roomStoreToken = RoomViewStore.addListener(this._onRoomViewStoreUpdate); - - MatrixClientPeg.get().getMediaLimits().then((limits) => { - this._uploadLimits = limits; - }).catch(() => { - // HS can't or won't provide limits, so don't give any. - this._uploadLimits = {}; - }) } componentWillUnmount() { @@ -110,8 +103,10 @@ export default class MessageComposer extends React.Component { } isFileUploadAllowed(file) { - if (this._uploadLimits.upload_size !== undefined && file.size > this._uploadLimits.upload_size) { - return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(this._uploadLimits.upload_size)}); + if (this.props.mediaLimits !== undefined && + this.props.mediaLimits.upload_size !== undefined && + file.size > this.props.mediaLimits.upload_size) { + return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(this.mediaLimits.upload_size)}); } return true; } @@ -430,6 +425,8 @@ MessageComposer.propTypes = { // callback when a file to upload is chosen uploadFile: PropTypes.func.isRequired, + mediaLimits: PropTypes.object, + // string representing the current room app drawer state - showApps: PropTypes.bool, + showApps: PropTypes.bool }; From 736b76bbb0ddcedabd348c1d849cba3595535cbe Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Thu, 21 Jun 2018 11:29:06 +0100 Subject: [PATCH 6/8] If HttpStatus == 413, refresh media limits --- src/ContentMessages.js | 4 ++-- src/components/structures/RoomView.js | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ContentMessages.js b/src/ContentMessages.js index fd21977108..f2bbdfafe5 100644 --- a/src/ContentMessages.js +++ b/src/ContentMessages.js @@ -377,9 +377,9 @@ class ContentMessages { } } if (error) { - dis.dispatch({action: 'upload_failed', upload: upload}); + dis.dispatch({action: 'upload_failed', upload, error}); } else { - dis.dispatch({action: 'upload_finished', upload: upload}); + dis.dispatch({action: 'upload_finished', upload}); } }); } diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index cb6b94c2e6..01485deecb 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -500,7 +500,10 @@ module.exports = React.createClass({ break; case 'notifier_enabled': case 'upload_failed': - this._fetchMediaLimits(true); + // 413: File was too big or upset the server in some way. + if(payload.data.error.http_status === 413) { + this._fetchMediaLimits(true); + } case 'upload_started': case 'upload_finished': this.forceUpdate(); From 541f1d71fb3e7f444b00e77f242804c7e8e3fa3c Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Sat, 23 Jun 2018 14:28:20 +0100 Subject: [PATCH 7/8] Move upload verification logic to RoomView. Improve upload dialog ux --- src/components/structures/RoomView.js | 55 ++++++++++++------- src/components/views/rooms/MessageComposer.js | 25 ++++----- 2 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 01485deecb..4866fe3f60 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -26,6 +26,7 @@ const React = require("react"); const ReactDOM = require("react-dom"); import PropTypes from 'prop-types'; import Promise from 'bluebird'; +import filesize from 'filesize'; const classNames = require("classnames"); import { _t } from '../../languageHandler'; @@ -49,8 +50,6 @@ import SettingsStore, {SettingLevel} from "../../settings/SettingsStore"; const DEBUG = false; let debuglog = function() {}; -let mediaLimitCache = null; - const BROWSER_SUPPORTS_SANDBOX = 'sandbox' in document.createElement('iframe'); if (DEBUG) { @@ -96,9 +95,9 @@ module.exports = React.createClass({ roomLoading: true, peekLoading: false, shouldPeek: true, - - // Media limits for uploading, this may change. - mediaLimits: {}, + + // Media limits for uploading. + mediaConfig: undefined, // The event to be scrolled to initially initialEventId: null, @@ -152,24 +151,31 @@ module.exports = React.createClass({ MatrixClientPeg.get().on("RoomState.members", this.onRoomStateMember); MatrixClientPeg.get().on("RoomMember.membership", this.onRoomMemberMembership); MatrixClientPeg.get().on("accountData", this.onAccountData); - this._fetchMediaLimits(); + this._fetchMediaConfig(); // Start listening for RoomViewStore updates this._roomStoreToken = RoomViewStore.addListener(this._onRoomViewStoreUpdate); this._onRoomViewStoreUpdate(true); }, - _fetchMediaLimits: function(invalidateCache: boolean = false) { - let limits; - if(invalidateCache || mediaLimitCache == null) { - MatrixClientPeg.get().getMediaLimits().then((_limits) => { - limits = _limits; - }).catch(() => { - // Media repo can't or won't report limits, so provide an empty object (no limits). - limits = {}; - }); - mediaLimitCache = limits; + _fetchMediaConfig: function(invalidateCache: boolean = false) { + /// NOTE: Using global here so we don't make repeated requests for the + /// config every time we swap room. + if(global.mediaConfig !== undefined && !invalidateCache) { + this.setState({mediaConfig: global.mediaConfig}); + return; } - this.state.mediaLimits = limits; + console.log("[Media Config] Fetching"); + MatrixClientPeg.get().getMediaConfig().then((config) => { + console.log("[Media Config] Fetched config:", config); + return config; + }).catch(() => { + // Media repo can't or won't report limits, so provide an empty object (no limits). + console.log("[Media Config] Could not fetch config, so not limiting uploads."); + return {}; + }).then((config) => { + global.mediaConfig = config; + this.setState({mediaConfig: config}); + }); }, _onRoomViewStoreUpdate: function(initial) { @@ -501,8 +507,8 @@ module.exports = React.createClass({ case 'notifier_enabled': case 'upload_failed': // 413: File was too big or upset the server in some way. - if(payload.data.error.http_status === 413) { - this._fetchMediaLimits(true); + if(payload.error.http_status === 413) { + this._fetchMediaConfig(true); } case 'upload_started': case 'upload_finished': @@ -935,6 +941,15 @@ module.exports = React.createClass({ this.setState({ draggingFile: false }); }, + isFileUploadAllowed(file) { + if (this.state.mediaConfig !== undefined && + this.state.mediaConfig["m.upload.size"] !== undefined && + file.size > this.state.mediaConfig["m.upload.size"]) { + return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(this.state.mediaConfig["m.upload.size"])}); + } + return true; + }, + uploadFile: async function(file) { dis.dispatch({action: 'focus_composer'}); @@ -1677,7 +1692,7 @@ module.exports = React.createClass({ callState={this.state.callState} disabled={this.props.disabled} showApps={this.state.showApps} - mediaLimits={this.state.mediaLimits} + uploadAllowed={this.isFileUploadAllowed} />; } diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index 1a80a0be4f..f0c73a67af 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -16,7 +16,6 @@ limitations under the License. */ import React from 'react'; import PropTypes from 'prop-types'; -import filesize from 'filesize'; import { _t } from '../../../languageHandler'; import CallHandler from '../../../CallHandler'; import MatrixClientPeg from '../../../MatrixClientPeg'; @@ -102,15 +101,6 @@ export default class MessageComposer extends React.Component { this.uploadFiles(tfiles); } - isFileUploadAllowed(file) { - if (this.props.mediaLimits !== undefined && - this.props.mediaLimits.upload_size !== undefined && - file.size > this.props.mediaLimits.upload_size) { - return _t("File is too big. Maximum file size is %(fileSize)s", {fileSize: filesize(this.mediaLimits.upload_size)}); - } - return true; - } - uploadFiles(files) { const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog"); const TintableSvg = sdk.getComponent("elements.TintableSvg"); @@ -120,7 +110,7 @@ export default class MessageComposer extends React.Component { const failedFiles = []; for (let i=0; i { files[i].name || _t('Attachment') } @@ -128,7 +118,7 @@ export default class MessageComposer extends React.Component { fileList.push(files[i]); } else { failedFiles.push(
  • - { files[i].name || _t('Attachment') } { _t('Reason') + ": " + fileAcceptedOrError} + { files[i].name || _t('Attachment') }

    { _t('Reason') + ": " + fileAcceptedOrError}

    • ); } } @@ -158,6 +148,12 @@ export default class MessageComposer extends React.Component { ); + let buttonText; + if (acceptedFiles.length > 0 && failedFiles.length > 0) { + buttonText = "Upload selected" + } else if (failedFiles.length > 0) { + buttonText = "Close" + } Modal.createTrackedDialog('Upload Files confirmation', '', QuestionDialog, { title: _t('Upload Files'), @@ -168,6 +164,8 @@ export default class MessageComposer extends React.Component { { replyToWarning } ), + hasCancelButton: acceptedFiles.length > 0, + button: buttonText, onFinished: (shouldUpload) => { if (shouldUpload) { // MessageComposer shouldn't have to rely on its parent passing in a callback to upload a file @@ -425,7 +423,8 @@ MessageComposer.propTypes = { // callback when a file to upload is chosen uploadFile: PropTypes.func.isRequired, - mediaLimits: PropTypes.object, + // function to test whether a file should be allowed to be uploaded. + uploadAllowed: PropTypes.func.isRequired, // string representing the current room app drawer state showApps: PropTypes.bool From 67d39a40d3c9f5c477bff0310975f0d9b8b6d401 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Tue, 16 Oct 2018 11:26:04 +0100 Subject: [PATCH 8/8] Sneaky , --- src/i18n/strings/en_EN.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 122c798b12..fe70da61d9 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1264,7 +1264,7 @@ "Failed to add tag %(tagName)s to room": "Failed to add tag %(tagName)s to room", "File is too big. Maximum file size is %(fileSize)s": "File is too big. Maximum file size is %(fileSize)s", "Reason": "Reason", - "The following files cannot be uploaded:": "The following files cannot be uploaded:" + "The following files cannot be uploaded:": "The following files cannot be uploaded:", "You've previously used Riot on %(host)s with lazy loading of members enabled. In this version lazy loading is disabled. As the local cache is not compatible between these two settings, Riot needs to resync your account.": "You've previously used Riot on %(host)s with lazy loading of members enabled. In this version lazy loading is disabled. As the local cache is not compatible between these two settings, Riot needs to resync your account.", "If the other version of Riot is still open in another tab, please close it as using Riot on the same host with both lazy loading enabled and disabled simultaneously will cause issues.": "If the other version of Riot is still open in another tab, please close it as using Riot on the same host with both lazy loading enabled and disabled simultaneously will cause issues.", "Incompatible local cache": "Incompatible local cache",