From 3bc5e2beb3f57cce1170e2e9813bf9fe92a85d92 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 26 Oct 2018 19:47:53 -0600 Subject: [PATCH 1/3] Fix and test matrix.to alias permalinks Fixes https://github.com/vector-im/riot-web/issues/7614 Regression of https://github.com/matrix-org/matrix-react-sdk/pull/2250 --- src/matrix-to.js | 17 ++++-- test/matrix-to-test.js | 120 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 133 insertions(+), 4 deletions(-) diff --git a/src/matrix-to.js b/src/matrix-to.js index 944446c4cc..c0b57e725d 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -24,8 +24,13 @@ export const baseUrl = `https://${host}`; const MAX_SERVER_CANDIDATES = 3; export function makeEventPermalink(roomId, eventId) { + const permalinkBase = `${baseUrl}/#/${roomId}/${eventId}`; + + // If the roomId isn't actually a room ID, don't try to list the servers. + // Aliases are already routable, and don't need extra information. + if (roomId[0] !== '!') return permalinkBase; const serverCandidates = pickServerCandidates(roomId); - return `${baseUrl}/#/${roomId}/${eventId}?${encodeServerCandidates(serverCandidates)}`; + return `${permalinkBase}${encodeServerCandidates(serverCandidates)}`; } export function makeUserPermalink(userId) { @@ -33,8 +38,14 @@ export function makeUserPermalink(userId) { } export function makeRoomPermalink(roomId) { + const permalinkBase = `${baseUrl}/#/${roomId}`; + + // If the roomId isn't actually a room ID, don't try to list the servers. + // Aliases are already routable, and don't need extra information. + if (roomId[0] !== '!') return permalinkBase; + const serverCandidates = pickServerCandidates(roomId); - return `${baseUrl}/#/${roomId}?${encodeServerCandidates(serverCandidates)}`; + return `${permalinkBase}${encodeServerCandidates(serverCandidates)}`; } export function makeGroupPermalink(groupId) { @@ -43,7 +54,7 @@ export function makeGroupPermalink(groupId) { export function encodeServerCandidates(candidates) { if (!candidates) return ''; - return `via=${candidates.map(c => encodeURIComponent(c)).join("&via=")}`; + return `?via=${candidates.map(c => encodeURIComponent(c)).join("&via=")}`; } export function pickServerCandidates(roomId) { diff --git a/test/matrix-to-test.js b/test/matrix-to-test.js index d199588b9a..c581880158 100644 --- a/test/matrix-to-test.js +++ b/test/matrix-to-test.js @@ -13,7 +13,13 @@ limitations under the License. import expect from 'expect'; import peg from '../src/MatrixClientPeg'; -import {pickServerCandidates} from "../src/matrix-to"; +import { + makeEventPermalink, + makeGroupPermalink, + makeRoomPermalink, + makeUserPermalink, + pickServerCandidates +} from "../src/matrix-to"; import * as testUtils from "./test-utils"; @@ -228,4 +234,116 @@ describe('matrix-to', function() { expect(pickedServers.length).toBe(1); expect(pickedServers[0]).toBe("example.org:8448"); }); + + it('should generate an event permalink for room IDs with no candidate servers', function() { + peg.get().getRoom = () => null; + const result = makeEventPermalink("!somewhere:example.org", "$something:example.com"); + expect(result).toBe("https://matrix.to/#/!somewhere:example.org/$something:example.com"); + }); + + it('should generate an event permalink for room IDs with some candidate servers', function() { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:first", + powerLevel: 100, + }, + { + userId: "@bob:second", + powerLevel: 0, + }, + ], + }; + }; + const result = makeEventPermalink("!somewhere:example.org", "$something:example.com"); + expect(result).toBe("https://matrix.to/#/!somewhere:example.org/$something:example.com?via=first&via=second"); + }); + + it('should generate a room permalink for room IDs with no candidate servers', function() { + peg.get().getRoom = () => null; + const result = makeRoomPermalink("!somewhere:example.org"); + expect(result).toBe("https://matrix.to/#/!somewhere:example.org"); + }); + + it('should generate a room permalink for room IDs with some candidate servers', function() { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:first", + powerLevel: 100, + }, + { + userId: "@bob:second", + powerLevel: 0, + }, + ], + }; + }; + const result = makeRoomPermalink("!somewhere:example.org"); + expect(result).toBe("https://matrix.to/#/!somewhere:example.org?via=first&via=second"); + }); + + // Technically disallowed but we'll test it anyways + it('should generate an event permalink for room aliases with no candidate servers', function() { + peg.get().getRoom = () => null; + const result = makeEventPermalink("#somewhere:example.org", "$something:example.com"); + expect(result).toBe("https://matrix.to/#/#somewhere:example.org/$something:example.com"); + }); + + // Technically disallowed but we'll test it anyways + it('should generate an event permalink for room aliases without candidate servers even when some are available', function() { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:first", + powerLevel: 100, + }, + { + userId: "@bob:second", + powerLevel: 0, + }, + ], + }; + }; + const result = makeEventPermalink("#somewhere:example.org", "$something:example.com"); + expect(result).toBe("https://matrix.to/#/#somewhere:example.org/$something:example.com"); + }); + + it('should generate a room permalink for room aliases with no candidate servers', function() { + peg.get().getRoom = () => null; + const result = makeRoomPermalink("#somewhere:example.org"); + expect(result).toBe("https://matrix.to/#/#somewhere:example.org"); + }); + + it('should generate a room permalink for room aliases without candidate servers even when some are available', function() { + peg.get().getRoom = () => { + return { + getJoinedMembers: () => [ + { + userId: "@alice:first", + powerLevel: 100, + }, + { + userId: "@bob:second", + powerLevel: 0, + }, + ], + }; + }; + const result = makeRoomPermalink("#somewhere:example.org"); + expect(result).toBe("https://matrix.to/#/#somewhere:example.org"); + }); + + it('should generate a user permalink', function() { + const result = makeUserPermalink("@someone:example.org"); + expect(result).toBe("https://matrix.to/#/@someone:example.org"); + }); + + it('should generate a group permalink', function() { + const result = makeGroupPermalink("+community:example.org"); + expect(result).toBe("https://matrix.to/#/+community:example.org"); + }); }); From 5b22d157a7871440b4fcc593313aada12b852faa Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 26 Oct 2018 20:34:06 -0600 Subject: [PATCH 2/3] Fix candidate server encoding --- src/matrix-to.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/matrix-to.js b/src/matrix-to.js index c0b57e725d..b5827f671a 100644 --- a/src/matrix-to.js +++ b/src/matrix-to.js @@ -29,6 +29,7 @@ export function makeEventPermalink(roomId, eventId) { // If the roomId isn't actually a room ID, don't try to list the servers. // Aliases are already routable, and don't need extra information. if (roomId[0] !== '!') return permalinkBase; + const serverCandidates = pickServerCandidates(roomId); return `${permalinkBase}${encodeServerCandidates(serverCandidates)}`; } @@ -53,7 +54,7 @@ export function makeGroupPermalink(groupId) { } export function encodeServerCandidates(candidates) { - if (!candidates) return ''; + if (!candidates || candidates.length === 0) return ''; return `?via=${candidates.map(c => encodeURIComponent(c)).join("&via=")}`; } From 0cdc44a2054e424fa17b7e01e1c545736c145d89 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 26 Oct 2018 20:49:01 -0600 Subject: [PATCH 3/3] Appease the linter --- test/matrix-to-test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/matrix-to-test.js b/test/matrix-to-test.js index c581880158..70533575c4 100644 --- a/test/matrix-to-test.js +++ b/test/matrix-to-test.js @@ -18,7 +18,7 @@ import { makeGroupPermalink, makeRoomPermalink, makeUserPermalink, - pickServerCandidates + pickServerCandidates, } from "../src/matrix-to"; import * as testUtils from "./test-utils"; @@ -293,7 +293,7 @@ describe('matrix-to', function() { }); // Technically disallowed but we'll test it anyways - it('should generate an event permalink for room aliases without candidate servers even when some are available', function() { + it('should generate an event permalink for room aliases without candidate servers', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [ @@ -318,7 +318,7 @@ describe('matrix-to', function() { expect(result).toBe("https://matrix.to/#/#somewhere:example.org"); }); - it('should generate a room permalink for room aliases without candidate servers even when some are available', function() { + it('should generate a room permalink for room aliases without candidate servers', function() { peg.get().getRoom = () => { return { getJoinedMembers: () => [