From e25112ad357a74dce22187784fd2a3cb03527e90 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 30 Aug 2016 14:30:12 +0100 Subject: [PATCH] Fix exceptions when dealing with redactions When we got a redaction event, we were adding the entire (circular) MatrixEvent object for the redaction to the redacted event, which would then cause exceptions down the line (particularly when dealing with gappy timelines). We should only be adding the raw event. Fixes (hopefully) https://github.com/vector-im/vector-web/issues/1389. --- lib/models/event.js | 16 ++++- .../matrix-client-event-timeline.spec.js | 66 +++++++++++++++++++ spec/test-utils.js | 12 ++-- 3 files changed, 86 insertions(+), 8 deletions(-) diff --git a/lib/models/event.js b/lib/models/event.js index 33ba03e72..92a9a5398 100644 --- a/lib/models/event.js +++ b/lib/models/event.js @@ -248,13 +248,25 @@ module.exports.MatrixEvent.prototype = { * Update the content of an event in the same way it would be by the server * if it were redacted before it was sent to us * - * @param {Object} the raw event causing the redaction + * @param {module:models/event.MatrixEvent} redaction_event + * event causing the redaction */ makeRedacted: function(redaction_event) { + // quick sanity-check + if (!redaction_event.event) { + throw new Error("invalid redaction_event in makeRedacted"); + } + + // we attempt to replicate what we would see from the server if + // the event had been redacted before we saw it. + // + // The server removes (most of) the content of the event, and adds a + // "redacted_because" key to the unsigned section containing the + // redacted event. if (!this.event.unsigned) { this.event.unsigned = {}; } - this.event.unsigned.redacted_because = redaction_event; + this.event.unsigned.redacted_because = redaction_event.event; var key; for (key in this.event) { diff --git a/spec/integ/matrix-client-event-timeline.spec.js b/spec/integ/matrix-client-event-timeline.spec.js index db94541fb..6d77cd4d8 100644 --- a/spec/integ/matrix-client-event-timeline.spec.js +++ b/spec/integ/matrix-client-event-timeline.spec.js @@ -655,4 +655,70 @@ describe("MatrixClient event timelines", function() { }).catch(utils.failTest); }); }); + + + it("should handle gappy syncs after redactions", function(done) { + // https://github.com/vector-im/vector-web/issues/1389 + + // a state event, followed by a redaction thereof + var event = utils.mkMembership({ + room: roomId, mship: "join", user: otherUserId + }); + var redaction = utils.mkEvent({ + type: "m.room.redaction", + room_id: roomId, + sender: otherUserId, + content: {} + }); + redaction.redacts = event.event_id; + + var syncData = { + next_batch: "batch1", + rooms: { + join: {}, + }, + }; + syncData.rooms.join[roomId] = { + timeline: { + events: [ + event, + redaction, + ], + limited: false, + }, + }; + httpBackend.when("GET", "/sync").respond(200, syncData); + + httpBackend.flush().then(function() { + var room = client.getRoom(roomId); + var tl = room.getLiveTimeline(); + expect(tl.getEvents().length).toEqual(3); + expect(tl.getEvents()[1].isRedacted()).toBe(true); + + var sync2 = { + next_batch: "batch2", + rooms: { + join: {}, + }, + }; + sync2.rooms.join[roomId] = { + timeline: { + events: [ + utils.mkMessage({ + room: roomId, user: otherUserId, msg: "world" + }), + ], + limited: true, + prev_batch: "newerTok", + }, + }; + httpBackend.when("GET", "/sync").respond(200, sync2); + + return httpBackend.flush(); + }).then(function() { + var room = client.getRoom(roomId); + var tl = room.getLiveTimeline(); + expect(tl.getEvents().length).toEqual(1); + }).catch(utils.failTest).done(done); + }); }); diff --git a/spec/test-utils.js b/spec/test-utils.js index 39559187a..ecee27331 100644 --- a/spec/test-utils.js +++ b/spec/test-utils.js @@ -48,7 +48,7 @@ module.exports.mock = function(constr, name) { * @param {Object} opts Values for the event. * @param {string} opts.type The event.type * @param {string} opts.room The event.room_id - * @param {string} opts.user The event.user_id + * @param {string} opts.sender The event.sender * @param {string} opts.skey Optional. The state key (auto inserts empty string) * @param {Object} opts.content The event.content * @param {boolean} opts.event True to make a MatrixEvent. @@ -61,7 +61,7 @@ module.exports.mkEvent = function(opts) { var event = { type: opts.type, room_id: opts.room, - sender: opts.user, + sender: opts.sender || opts.user, // opts.user for backwards-compat content: opts.content, event_id: "$" + Math.random() + "-" + Math.random() }; @@ -88,7 +88,7 @@ module.exports.mkPresence = function(opts) { var event = { event_id: "$" + Math.random() + "-" + Math.random(), type: "m.presence", - sender: opts.user, + sender: opts.sender || opts.user, // opts.user for backwards-compat content: { avatar_url: opts.url, displayname: opts.name, @@ -104,8 +104,8 @@ module.exports.mkPresence = function(opts) { * @param {Object} opts Values for the membership. * @param {string} opts.room The room ID for the event. * @param {string} opts.mship The content.membership for the event. - * @param {string} opts.user The user ID for the event. - * @param {string} opts.skey The other user ID for the event if applicable + * @param {string} opts.sender The sender user ID for the event. + * @param {string} opts.skey The target user ID for the event if applicable * e.g. for invites/bans. * @param {string} opts.name The content.displayname for the event. * @param {string} opts.url The content.avatar_url for the event. @@ -115,7 +115,7 @@ module.exports.mkPresence = function(opts) { module.exports.mkMembership = function(opts) { opts.type = "m.room.member"; if (!opts.skey) { - opts.skey = opts.user; + opts.skey = opts.sender || opts.user; } if (!opts.mship) { throw new Error("Missing .mship => " + JSON.stringify(opts));