From 1584ab42c27cb95e1b142d811c47233348c8db0e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 13 May 2019 14:12:58 -0600 Subject: [PATCH 1/2] Support a backup room ID in PermalinkCreator In the case of room upgrades, it is possible the client is trying to render the room create event, but the user has never been in the old room. This results in an error because the PermalinkCreator cannot possibly figure out a room ID. Instead, we'll feed the creator an alternate room ID to try if the room object can't be provided. Fixes https://github.com/vector-im/riot-web/issues/9636 --- src/components/views/messages/RoomCreate.js | 2 +- src/matrix-to.js | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/components/views/messages/RoomCreate.js b/src/components/views/messages/RoomCreate.js index f9dc3df2dc..95254323fa 100644 --- a/src/components/views/messages/RoomCreate.js +++ b/src/components/views/messages/RoomCreate.js @@ -49,7 +49,7 @@ module.exports = React.createClass({ return
; // We should never have been instaniated in this case } const prevRoom = MatrixClientPeg.get().getRoom(predecessor['room_id']); - const permalinkCreator = new RoomPermalinkCreator(prevRoom); + const permalinkCreator = new RoomPermalinkCreator(prevRoom, predecessor['room_id']); permalinkCreator.load(); const predecessorPermalink = permalinkCreator.forEvent(predecessor['event_id']); return
diff --git a/src/matrix-to.js b/src/matrix-to.js index a198bb422e..14467cb4c5 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -70,8 +70,12 @@ const MAX_SERVER_CANDIDATES = 3; // the list and magically have the link work. export class RoomPermalinkCreator { - constructor(room) { + // We support being given a roomId as a fallback in the event the `room` object + // doesn't exist or is not healthy for us to rely on. For example, loading a + // permalink to a room which the MatrixClient doesn't know about. + constructor(room, roomId=null) { this._room = room; + this._roomId = room ? room.roomId : roomId; this._highestPlUserId = null; this._populationMap = null; this._bannedHostsRegexps = null; @@ -79,6 +83,10 @@ export class RoomPermalinkCreator { this._serverCandidates = null; this._started = false; + if (!this._roomId) { + throw new Error("Failed to resolve a roomId for the permalink creator to use"); + } + this.onMembership = this.onMembership.bind(this); this.onRoomState = this.onRoomState.bind(this); } @@ -116,13 +124,13 @@ export class RoomPermalinkCreator { } forEvent(eventId) { - const roomId = this._room.roomId; + const roomId = this._roomId; const permalinkBase = `${baseUrl}/#/${roomId}/${eventId}`; return `${permalinkBase}${encodeServerCandidates(this._serverCandidates)}`; } forRoom() { - const roomId = this._room.roomId; + const roomId = this._roomId; const permalinkBase = `${baseUrl}/#/${roomId}`; return `${permalinkBase}${encodeServerCandidates(this._serverCandidates)}`; } @@ -246,7 +254,6 @@ export class RoomPermalinkCreator { } } - export function makeUserPermalink(userId) { return `${baseUrl}/#/${userId}`; } From f86ccae4a0f3c6b5ade3ea861f1b07d2c2484ce7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 13 May 2019 14:31:40 -0600 Subject: [PATCH 2/2] Give all the matrix.to tests a room ID to abuse --- test/matrix-to-test.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/matrix-to-test.js b/test/matrix-to-test.js index 45b5f74c5e..33947703ef 100644 --- a/test/matrix-to-test.js +++ b/test/matrix-to-test.js @@ -74,7 +74,7 @@ describe('matrix-to', function() { }); it('should pick no candidate servers when the room has no members', function() { - const room = mockRoom(null, []); + const room = mockRoom("!fake:example.org", []); const creator = new RoomPermalinkCreator(room); creator.load(); expect(creator._serverCandidates).toBeTruthy(); @@ -82,7 +82,7 @@ describe('matrix-to', function() { }); it('should pick a candidate server for the highest power level user in the room', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:pl_50", powerLevel: 50, @@ -109,7 +109,7 @@ describe('matrix-to', function() { userId: "@alice:pl_95", powerLevel: 95, }; - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:pl_50", powerLevel: 50, @@ -132,7 +132,7 @@ describe('matrix-to', function() { }); it('should pick candidate servers based on user population', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:first", powerLevel: 0, @@ -168,7 +168,7 @@ describe('matrix-to', function() { }); it('should pick prefer candidate servers with higher power levels', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:first", powerLevel: 100, @@ -195,7 +195,7 @@ describe('matrix-to', function() { }); it('should pick a maximum of 3 candidate servers', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:alpha", powerLevel: 100, @@ -224,7 +224,7 @@ describe('matrix-to', function() { }); it('should not consider IPv4 hosts', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:127.0.0.1", powerLevel: 100, @@ -237,7 +237,7 @@ describe('matrix-to', function() { }); it('should not consider IPv6 hosts', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:[::1]", powerLevel: 100, @@ -250,7 +250,7 @@ describe('matrix-to', function() { }); it('should not consider IPv4 hostnames with ports', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:127.0.0.1:8448", powerLevel: 100, @@ -263,7 +263,7 @@ describe('matrix-to', function() { }); it('should not consider IPv6 hostnames with ports', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:[::1]:8448", powerLevel: 100, @@ -276,7 +276,7 @@ describe('matrix-to', function() { }); it('should work with hostnames with ports', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:example.org:8448", powerLevel: 100, @@ -291,7 +291,7 @@ describe('matrix-to', function() { }); it('should not consider servers explicitly denied by ACLs', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:evilcorp.com", powerLevel: 100, @@ -311,7 +311,7 @@ describe('matrix-to', function() { }); it('should not consider servers not allowed by ACLs', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:evilcorp.com", powerLevel: 100, @@ -331,7 +331,7 @@ describe('matrix-to', function() { }); it('should consider servers not explicitly banned by ACLs', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:evilcorp.com", powerLevel: 100, @@ -352,7 +352,7 @@ describe('matrix-to', function() { }); it('should consider servers not disallowed by ACLs', function() { - const room = mockRoom(null, [ + const room = mockRoom("!fake:example.org", [ { userId: "@alice:evilcorp.com", powerLevel: 100,