Merge remote-tracking branch 'origin/develop' into rav/async_crypto/1

CHANGELOG.md

@@ -1,5 +1,6 @@
-Unreleased changes
+Changes in [0.8.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.8.0) (2017-08-15)
-==================
+================================================================================================
+[Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v0.7.13...v0.8.0)
 
 BREAKING CHANGE
 ---------------
@@ -36,6 +37,91 @@ applications which support end-to-end encryption:
    in a slightly different format.
 
 
+ * Make bits of `olmlib` asynchronous
+   [\#521](https://github.com/matrix-org/matrix-js-sdk/pull/521)
+ * Make some of DeviceList asynchronous
+   [\#520](https://github.com/matrix-org/matrix-js-sdk/pull/520)
+ * Make methods in crypto/algorithms async
+   [\#519](https://github.com/matrix-org/matrix-js-sdk/pull/519)
+ * Avoid sending unencrypted messages in e2e room
+   [\#518](https://github.com/matrix-org/matrix-js-sdk/pull/518)
+ * Make tests wait for syncs to happen
+   [\#517](https://github.com/matrix-org/matrix-js-sdk/pull/517)
+ * Make a load of methods in the 'Crypto' module asynchronous
+   [\#510](https://github.com/matrix-org/matrix-js-sdk/pull/510)
+ * Set `rawDisplayName` to `userId` if membership has `displayname=null`
+   [\#515](https://github.com/matrix-org/matrix-js-sdk/pull/515)
+ * Refactor handling of crypto events for async
+   [\#508](https://github.com/matrix-org/matrix-js-sdk/pull/508)
+ * Let event decryption be asynchronous
+   [\#509](https://github.com/matrix-org/matrix-js-sdk/pull/509)
+ * Transform `async` functions to bluebird promises
+   [\#511](https://github.com/matrix-org/matrix-js-sdk/pull/511)
+ * Add more group APIs
+   [\#512](https://github.com/matrix-org/matrix-js-sdk/pull/512)
+ * Retrying test: wait for localEchoUpdated event
+   [\#507](https://github.com/matrix-org/matrix-js-sdk/pull/507)
+ * Fix member events breaking on timeline reset, 2
+   [\#504](https://github.com/matrix-org/matrix-js-sdk/pull/504)
+ * Make bits of the js-sdk api asynchronous
+   [\#503](https://github.com/matrix-org/matrix-js-sdk/pull/503)
+ * Yet more js-sdk test deflakification
+   [\#499](https://github.com/matrix-org/matrix-js-sdk/pull/499)
+ * Fix racy 'matrixclient retrying' test
+   [\#497](https://github.com/matrix-org/matrix-js-sdk/pull/497)
+ * Fix spamming of key-share-requests
+   [\#495](https://github.com/matrix-org/matrix-js-sdk/pull/495)
+ * Add progress handler to `uploadContent`
+   [\#500](https://github.com/matrix-org/matrix-js-sdk/pull/500)
+ * Switch matrix-js-sdk to bluebird
+   [\#490](https://github.com/matrix-org/matrix-js-sdk/pull/490)
+ * Fix some more flakey tests
+   [\#492](https://github.com/matrix-org/matrix-js-sdk/pull/492)
+ * make the npm test script windows-friendly
+   [\#489](https://github.com/matrix-org/matrix-js-sdk/pull/489)
+ * Fix a bunch of races in the tests
+   [\#488](https://github.com/matrix-org/matrix-js-sdk/pull/488)
+ * Fix early return in MatrixClient.setGuestAccess
+   [\#487](https://github.com/matrix-org/matrix-js-sdk/pull/487)
+ * Remove testUtils.failTest
+   [\#486](https://github.com/matrix-org/matrix-js-sdk/pull/486)
+ * Add test:watch script
+   [\#485](https://github.com/matrix-org/matrix-js-sdk/pull/485)
+ * Make it possible to use async/await
+   [\#484](https://github.com/matrix-org/matrix-js-sdk/pull/484)
+ * Remove m.new_device support
+   [\#483](https://github.com/matrix-org/matrix-js-sdk/pull/483)
+ * Use access-token in header
+   [\#478](https://github.com/matrix-org/matrix-js-sdk/pull/478)
+ * Sanity-check response from /thirdparty/protocols
+   [\#482](https://github.com/matrix-org/matrix-js-sdk/pull/482)
+ * Avoid parsing plain-text errors as JSON
+   [\#479](https://github.com/matrix-org/matrix-js-sdk/pull/479)
+ * Use external mock-request
+   [\#481](https://github.com/matrix-org/matrix-js-sdk/pull/481)
+ * Fix some races in the tests
+   [\#480](https://github.com/matrix-org/matrix-js-sdk/pull/480)
+ * Fall back to MemoryCryptoStore if indexeddb fails
+   [\#475](https://github.com/matrix-org/matrix-js-sdk/pull/475)
+ * Fix load failure in firefox when indexedDB is disabled
+   [\#474](https://github.com/matrix-org/matrix-js-sdk/pull/474)
+ * Fix a race in a test
+   [\#471](https://github.com/matrix-org/matrix-js-sdk/pull/471)
+ * Avoid throwing an unhandled error when the indexeddb is deleted
+   [\#470](https://github.com/matrix-org/matrix-js-sdk/pull/470)
+ * fix jsdoc
+   [\#469](https://github.com/matrix-org/matrix-js-sdk/pull/469)
+ * Handle m.forwarded_room_key events
+   [\#468](https://github.com/matrix-org/matrix-js-sdk/pull/468)
+ * Improve error reporting from indexeddbstore.clearDatabase
+   [\#466](https://github.com/matrix-org/matrix-js-sdk/pull/466)
+ * Implement sharing of megolm keys
+   [\#454](https://github.com/matrix-org/matrix-js-sdk/pull/454)
+ * Process received room key requests
+   [\#449](https://github.com/matrix-org/matrix-js-sdk/pull/449)
+ * Send m.room_key_request events when we fail to decrypt an event
+   [\#448](https://github.com/matrix-org/matrix-js-sdk/pull/448)
+
 Changes in [0.7.13](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v0.7.13) (2017-06-22)
 ==================================================================================================
 [Full Changelog](https://github.com/matrix-org/matrix-js-sdk/compare/v0.7.12...v0.7.13)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "matrix-js-sdk",
-  "version": "0.7.13",
+  "version": "0.8.0",
   "description": "Matrix Client-Server SDK for Javascript",
   "main": "index.js",
   "scripts": {

spec/unit/crypto/algorithms/megolm.spec.js

@@ -71,9 +71,13 @@ describe("MegolmDecryption", function() {
             groupSession = new global.Olm.OutboundGroupSession();
             groupSession.create();
 
-            const event = new MatrixEvent({});
+            // construct a fake decrypted key event via the use of a mocked
-            event.setClearData(
+            // 'crypto' implementation.
-                {
+            const event = new MatrixEvent({
+                type: 'm.room.encrypted',
+            });
+            const decryptedData = {
+                clearEvent: {
                     type: 'm.room_key',
                     content: {
                         algorithm: 'm.megolm.v1.aes-sha2',
@@ -82,12 +86,20 @@ describe("MegolmDecryption", function() {
                         session_key: groupSession.session_key(),
                     },
                 },
-                "SENDER_CURVE25519",
+                senderCurve25519Key: "SENDER_CURVE25519",
-                "SENDER_ED25519",
+                claimedEd25519Key: "SENDER_ED25519",
-            );
+            };
 
+            const mockCrypto = {
+                decryptEvent: function() {
+                    return Promise.resolve(decryptedData);
+                },
+            };
+
+            return event.attemptDecryption(mockCrypto).then(() => {
                 megolmDecryption.onRoomKeyEvent(event);
             });
+        });
 
         it('can decrypt an event', function() {
             const event = new MatrixEvent({
@@ -104,8 +116,8 @@ describe("MegolmDecryption", function() {
                 },
             });
 
-            return megolmDecryption.decryptEvent(event).then(() => {
+            return megolmDecryption.decryptEvent(event).then((res) => {
-                expect(event.getContent()).toEqual('testytest');
+                expect(res.clearEvent.content).toEqual('testytest');
             });
         });
 

spec/unit/event.spec.js

@@ -0,0 +1,82 @@
+/*
+Copyright 2017 New Vector Ltd
+
+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.
+*/
+
+import sdk from '../..';
+const MatrixEvent = sdk.MatrixEvent;
+
+import testUtils from '../test-utils';
+
+import expect from 'expect';
+import Promise from 'bluebird';
+
+describe("MatrixEvent", () => {
+    beforeEach(function() {
+        testUtils.beforeEach(this); // eslint-disable-line no-invalid-this
+    });
+
+    describe(".attemptDecryption", () => {
+        let encryptedEvent;
+
+        beforeEach(() => {
+            encryptedEvent = new MatrixEvent({
+                id: 'test_encrypted_event',
+                type: 'm.room.encrypted',
+                content: {
+                    ciphertext: 'secrets',
+                },
+            });
+        });
+
+        it('should retry decryption if a retry is queued', () => {
+            let callCount = 0;
+
+            let prom2;
+
+            const crypto = {
+                decryptEvent: function() {
+                    ++callCount;
+                    console.log(`decrypt: ${callCount}`);
+                    if (callCount == 1) {
+                        // schedule a second decryption attempt while
+                        // the first one is still running.
+                        prom2 = encryptedEvent.attemptDecryption(crypto);
+
+                        const error = new Error("nope");
+                        error.name = 'DecryptionError';
+                        return Promise.reject(error);
+                    } else {
+                        expect(prom2.isFulfilled()).toBe(
+                            false, 'second attemptDecryption resolved too soon');
+
+                        return Promise.resolve({
+                            clearEvent: {
+                                type: 'm.room.message',
+                            },
+                        });
+                    }
+                },
+            };
+
+            return encryptedEvent.attemptDecryption(crypto).then(() => {
+                expect(callCount).toEqual(2);
+                expect(encryptedEvent.getType()).toEqual('m.room.message');
+
+                // make sure the second attemptDecryption resolves
+                return prom2;
+            });
+        });
+    });
+});

src/crypto/algorithms/base.js

@@ -117,7 +117,8 @@ class DecryptionAlgorithm {
      *
      * @param {MatrixEvent} event undecrypted event
      *
-     * @return {Promise} resolves once we have finished decrypting. Rejects with an
+     * @return {Promise<module:crypto~EventDecryptionResult>} promise which
+     * resolves once we have finished decrypting. Rejects with an
      * `algorithms.DecryptionError` if there is a problem decrypting the event.
      */
 

src/crypto/algorithms/megolm.js

@@ -535,18 +535,12 @@ utils.inherits(MegolmDecryption, base.DecryptionAlgorithm);
  *
  * @param {MatrixEvent} event
  *
- * @return {Promise} resolves once we have finished decrypting. Rejects with an
+ * returns a promise which resolves to a
- * `algorithms.DecryptionError` if there is a problem decrypting the event.
+ * {@link module:crypto~EventDecryptionResult} once we have finished
+ * decrypting, or rejects with an `algorithms.DecryptionError` if there is a
+ * problem decrypting the event.
  */
-MegolmDecryption.prototype.decryptEvent = function(event) {
+MegolmDecryption.prototype.decryptEvent = async function(event) {
-    return this._decryptEvent(event, true);
-};
-
-
-// helper for the real decryptEvent and for _retryDecryption. If
-// requestKeysOnFail is true, we'll send an m.room_key_request when we fail
-// to decrypt the event due to missing megolm keys.
-MegolmDecryption.prototype._decryptEvent = async function(event, requestKeysOnFail) {
     const content = event.getWireContent();
 
     if (!content.sender_key || !content.session_id ||
@@ -563,10 +557,8 @@ MegolmDecryption.prototype._decryptEvent = async function(event, requestKeysOnFa
     } catch (e) {
         if (e.message === 'OLM.UNKNOWN_MESSAGE_INDEX') {
             this._addEventToPendingList(event);
-            if (requestKeysOnFail) {
             this._requestKeysForEvent(event);
         }
-        }
         throw new base.DecryptionError(
             e.toString(), {
                 session: content.sender_key + '|' + content.session_id,
@@ -577,9 +569,7 @@ MegolmDecryption.prototype._decryptEvent = async function(event, requestKeysOnFa
     if (res === null) {
         // We've got a message for a session we don't have.
         this._addEventToPendingList(event);
-        if (requestKeysOnFail) {
         this._requestKeysForEvent(event);
-        }
         throw new base.DecryptionError(
             "The sender's device has not sent us the keys for this message.",
             {
@@ -599,8 +589,12 @@ MegolmDecryption.prototype._decryptEvent = async function(event, requestKeysOnFa
         );
     }
 
-    event.setClearData(payload, res.senderKey, res.keysClaimed.ed25519,
+    return {
-        res.forwardingCurve25519KeyChain);
+        clearEvent: payload,
+        senderCurve25519Key: res.senderKey,
+        claimedEd25519Key: res.keysClaimed.ed25519,
+        forwardingCurve25519KeyChain: res.forwardingCurve25519KeyChain,
+    };
 };
 
 MegolmDecryption.prototype._requestKeysForEvent = function(event) {

src/crypto/algorithms/olm.js

@@ -157,8 +157,10 @@ utils.inherits(OlmDecryption, base.DecryptionAlgorithm);
  *
  * @param {MatrixEvent} event
  *
- * returns a promise which resolves once we have finished decrypting. Rejects with an
+ * returns a promise which resolves to a
- * `algorithms.DecryptionError` if there is a problem decrypting the event.
+ * {@link module:crypto~EventDecryptionResult} once we have finished
+ * decrypting. Rejects with an `algorithms.DecryptionError` if there is a
+ * problem decrypting the event.
  */
 OlmDecryption.prototype.decryptEvent = async function(event) {
     const content = event.getWireContent();
@@ -227,9 +229,13 @@ OlmDecryption.prototype.decryptEvent = async function(event) {
     }
 
     const claimedKeys = payload.keys || {};
-    event.setClearData(payload, deviceKey, claimedKeys.ed25519 || null);
-};
 
+    return {
+        clearEvent: payload,
+        senderCurve25519Key: deviceKey,
+        claimedEd25519Key: claimedKeys.ed25519 || null,
+    };
+};
 
 /**
  * Attempt to decrypt an Olm message

src/crypto/index.js

@@ -772,8 +772,9 @@ Crypto.prototype.encryptEvent = function(event, room) {
  *
  * @param {MatrixEvent} event
  *
- * @return {Promise} resolves once we have finished decrypting. Rejects with an
+ * @return {Promise<module:crypto~EventDecryptionResult>} resolves once we have
- * `algorithms.DecryptionError` if there is a problem decrypting the event.
+ *  finished decrypting. Rejects with an `algorithms.DecryptionError` if there
+ *  is a problem decrypting the event.
  */
 Crypto.prototype.decryptEvent = function(event) {
     const content = event.getWireContent();
@@ -1295,6 +1296,27 @@ class IncomingRoomKeyRequestCancellation {
     }
 }
 
+/**
+ * The result of a (successful) call to decryptEvent.
+ *
+ * @typedef {Object} EventDecryptionResult
+ *
+ * @property {Object} clearEvent The plaintext payload for the event
+ *     (typically containing <tt>type</tt> and <tt>content</tt> fields).
+ *
+ * @property {?string} senderCurve25519Key Key owned by the sender of this
+ *    event.  See {@link module:models/event.MatrixEvent#getSenderKey}.
+ *
+ * @property {?string} claimedEd25519Key ed25519 key claimed by the sender of
+ *    this event. See
+ *    {@link module:models/event.MatrixEvent#getClaimedEd25519Key}.
+ *
+ * @property {?Array<string>} forwardingCurve25519KeyChain list of curve25519
+ *     keys involved in telling us about the senderCurve25519Key and
+ *     claimedEd25519Key. See
+ *     {@link module:models/event.MatrixEvent#getForwardingCurve25519KeyChain}.
+ */
+
 /**
  * Fires when we receive a room key request
  *

src/models/event.js

@@ -21,11 +21,9 @@ limitations under the License.
  * @module models/event
  */
 
-const EventEmitter = require("events").EventEmitter;
-
-const utils = require('../utils.js');
-
 import Promise from 'bluebird';
+import {EventEmitter} from 'events';
+import utils from '../utils.js';
 
 /**
  * Enum for event statuses.
@@ -131,8 +129,10 @@ module.exports.MatrixEvent = function MatrixEvent(
      */
     this._forwardingCurve25519KeyChain = [];
 
-    /* flag to indicate if we have a process decrypting this event */
+    /* if we have a process decrypting this event, a Promise which resolves
-    this._decrypting = false;
+     * when it is finished. Normally null.
+     */
+    this._decryptionPromise = null;
 
     /* flag to indicate if we should retry decrypting this event after the
      * first attempt (eg, we have received new data which means that a second
@@ -315,7 +315,7 @@ utils.extend(module.exports.MatrixEvent.prototype, {
      * @return {boolean} True if this event is currently being decrypted, else false.
      */
     isBeingDecrypted: function() {
-        return this._decrypting;
+        return this._decryptionPromise != null;
     },
 
     /**
@@ -326,13 +326,12 @@ utils.extend(module.exports.MatrixEvent.prototype, {
      * @internal
      *
      * @param {module:crypto} crypto crypto module
+     *
+     * @returns {Promise} promise which resolves (to undefined) when the decryption
+     * attempt is completed.
      */
-    attemptDecryption: function(crypto) {
+    attemptDecryption: async function(crypto) {
-        if (!crypto) {
+        // start with a couple of sanity checks.
-            this._badEncryptedMessage("Encryption not enabled");
-            return;
-        }
-
         if (!this.isEncrypted()) {
             throw new Error("Attempt to decrypt event which isn't encrypted");
         }
@@ -347,60 +346,112 @@ utils.extend(module.exports.MatrixEvent.prototype, {
             );
         }
 
-        if (this._decrypting) {
+        // if we already have a decryption attempt in progress, then it may
+        // fail because it was using outdated info. We now have reason to
+        // succeed where it failed before, but we don't want to have multiple
+        // attempts going at the same time, so just set a flag that says we have
+        // new info.
+        //
+        if (this._decryptionPromise) {
             console.log(
                 `Event ${this.getId()} already being decrypted; queueing a retry`,
             );
             this._retryDecryption = true;
-            return;
+            return this._decryptionPromise;
         }
 
-        this._decrypting = true;
+        this._decryptionPromise = this._decryptionLoop(crypto);
-
+        return this._decryptionPromise;
-        this._doDecryption(crypto).finally(() => {
-            this._decrypting = false;
-            this._retryDecryption = false;
-        });
     },
 
-    _doDecryption: function(crypto) {
+    _decryptionLoop: async function(crypto) {
-        return Promise.try(() => {
+        // make sure that this method never runs completely synchronously.
-            return crypto.decryptEvent(this);
+        // (doing so would mean that we would clear _decryptionPromise *before*
-        }).catch((e) => {
+        // it is set in attemptDecryption - and hence end up with a stuck
+        // `_decryptionPromise`).
+        await Promise.resolve();
+
+        while (true) {
+            this._retryDecryption = false;
+
+            let res;
+            try {
+                if (!crypto) {
+                    res = this._badEncryptedMessage("Encryption not enabled");
+                } else {
+                    res = await crypto.decryptEvent(this);
+                }
+            } catch (e) {
                 if (e.name !== "DecryptionError") {
-                // not a decryption error: log the whole exception as an error.
+                    // not a decryption error: log the whole exception as an error
+                    // (and don't bother with a retry)
                     console.error(
                         `Error decrypting event (id=${this.getId()}): ${e.stack || e}`,
                     );
-                return null;
+                    this._decryptionPromise = null;
-            } else if (this._retryDecryption) {
+                    this._retryDecryption = false;
+                    return;
+                }
+
+                // see if we have a retry queued.
+                //
+                // NB: make sure to keep this check in the same tick of the
+                //   event loop as `_decryptionPromise = null` below - otherwise we
+                //   risk a race:
+                //
+                //   * A: we check _retryDecryption here and see that it is
+                //        false
+                //   * B: we get a second call to attemptDecryption, which sees
+                //        that _decryptionPromise is set so sets
+                //        _retryDecryption
+                //   * A: we continue below, clear _decryptionPromise, and
+                //        never do the retry.
+                //
+                if (this._retryDecryption) {
                     // decryption error, but we have a retry queued.
                     console.log(
-                    `Got error decrypting event (id=${this.getId()}), but retrying`,
+                        `Got error decrypting event (id=${this.getId()}: ` +
+                        `${e.message}), but retrying`,
                     );
-                this._retryDecryption = false;
+                    continue;
-                return this._doDecryption(crypto);
+                }
-            } else {
+
                 // decryption error, no retries queued. Warn about the error and
                 // set it to m.bad.encrypted.
                 console.warn(
                     `Error decrypting event (id=${this.getId()}): ${e}`,
                 );
 
-                this._badEncryptedMessage(e.message);
+                res = this._badEncryptedMessage(e.message);
-                return null;
+            }
+
+            // at this point, we've either successfully decrypted the event, or have given up
+            // (and set res to a 'badEncryptedMessage'). Either way, we can now set the
+            // cleartext of the event and raise Event.decrypted.
+            //
+            // make sure we clear '_decryptionPromise' before sending the 'Event.decrypted' event,
+            // otherwise the app will be confused to see `isBeingDecrypted` still set when
+            // there isn't an `Event.decrypted` on the way.
+            //
+            // see also notes on _retryDecryption above.
+            //
+            this._decryptionPromise = null;
+            this._retryDecryption = false;
+            this._setClearData(res);
+            return;
         }
-        });
     },
 
     _badEncryptedMessage: function(reason) {
-        this.setClearData({
+        return {
+            clearEvent: {
                 type: "m.room.message",
                 content: {
                     msgtype: "m.bad.encrypted",
                     body: "** Unable to decrypt: " + reason + " **",
                 },
-        });
+            },
+        };
     },
 
     /**
@@ -412,29 +463,17 @@ utils.extend(module.exports.MatrixEvent.prototype, {
      *
      * @fires module:models/event.MatrixEvent#"Event.decrypted"
      *
-     * @param {Object} clearEvent The plaintext payload for the event
+     * @param {module:crypto~EventDecryptionResult} decryptionResult
-     *     (typically containing <tt>type</tt> and <tt>content</tt> fields).
+     *     the decryption result, including the plaintext and some key info
-     *
-     * @param {string=} senderCurve25519Key Key owned by the sender of this event.
-     *    See {@link module:models/event.MatrixEvent#getSenderKey}.
-     *
-     * @param {string=} claimedEd25519Key ed25519 key claimed by the sender of
-     *    this event. See {@link module:models/event.MatrixEvent#getClaimedEd25519Key}.
-     *
-     * @param {Array<string>=} forwardingCurve25519KeyChain list of curve25519 keys
-     *     involved in telling us about the senderCurve25519Key and claimedEd25519Key.
-     *     See {@link module:models/event.MatrixEvent#getForwardingCurve25519KeyChain}.
      */
-    setClearData: function(
+    _setClearData: function(decryptionResult) {
-        clearEvent,
+        this._clearEvent = decryptionResult.clearEvent;
-        senderCurve25519Key,
+        this._senderCurve25519Key =
-        claimedEd25519Key,
+            decryptionResult.senderCurve25519Key || null;
-        forwardingCurve25519KeyChain,
+        this._claimedEd25519Key =
-    ) {
+            decryptionResult.claimedEd25519Key || null;
-        this._clearEvent = clearEvent;
+        this._forwardingCurve25519KeyChain =
-        this._senderCurve25519Key = senderCurve25519Key || null;
+            decryptionResult.forwardingCurve25519KeyChain || [];
-        this._claimedEd25519Key = claimedEd25519Key || null;
-        this._forwardingCurve25519KeyChain = forwardingCurve25519KeyChain || [];
         this.emit("Event.decrypted", this);
     },