Ignore permalink_prefix when serializing pills (#11726)

* Ignore permalink_prefix when serializing Markdown

fixes vector-im/element-web/issues/26002

During serialization of messages, pills were wrongfully serialized to a URL
starting with `permalink_prefix`. This is against the Spec (which mandates
https://matrix.to/#/ links) and the resulting pills were not recognized as
pills in other clients.

Spec-Appendix concerning matrix.to links: https://spec.matrix.org/v1.8/appendices/#matrixto-navigation

Signed-off-by: Lars Wickel <git@herkulessi.de>

* Test for pill serialization with permalink_prefix set

Signed-off-by: Lars Wickel <git@herkulessi.de>

* Test that permalink_prefix is used for permalink creation

Signed-off-by: Lars Wickel <git@herkulessi.de>

* Fix lint issues introduced

Signed-off-by: Lars Wickel <git@herkulessi.de>

* Incorporate requested changes

Signed-off-by: Lars Wickel <git@herkulessi.de>

---------

Signed-off-by: Lars Wickel <git@herkulessi.de>
Co-authored-by: herkulessi <git@herkulessi.de>
Co-authored-by: David Baker <dbkr@users.noreply.github.com>
This commit is contained in:
herkulessi 2024-08-01 13:17:44 +02:00 committed by GitHub
parent e6a3238621
commit fa60edf00f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 82 additions and 11 deletions

View File

@ -37,7 +37,7 @@ export function mdSerialize(model: EditorModel): string {
case Type.AtRoomPill:
return html + part.text;
case Type.RoomPill: {
const url = makeGenericPermalink(part.resourceId);
const url = makeGenericPermalink(part.resourceId, true);
// Escape square brackets and backslashes
// Here we use the resourceId for compatibility with non-rich text clients
// See https://github.com/vector-im/element-web/issues/16660
@ -45,7 +45,7 @@ export function mdSerialize(model: EditorModel): string {
return html + `[${title}](${url})`;
}
case Type.UserPill: {
const url = makeGenericPermalink(part.resourceId);
const url = makeGenericPermalink(part.resourceId, true);
// Escape square brackets and backslashes; convert newlines to HTML
const title = part.text.replace(/[[\\\]]/g, (c) => "\\" + c).replace(/\n/g, "<br>");
return html + `[${title}](${url})`;

View File

@ -274,26 +274,48 @@ export class RoomPermalinkCreator {
};
}
export function makeGenericPermalink(entityId: string): string {
return getPermalinkConstructor().forEntity(entityId);
/**
* Creates a permalink for an Entity. If isPill is set it uses a spec-compliant
* prefix for the permalink, instead of permalink_prefix
* @param {string} entityId The entity to link to.
* @param {boolean} isPill Link should be pillifyable.
* @returns {string|null} The transformed permalink or null if unable.
*/
export function makeGenericPermalink(entityId: string, isPill = false): string {
return getPermalinkConstructor(isPill).forEntity(entityId);
}
export function makeUserPermalink(userId: string): string {
return getPermalinkConstructor().forUser(userId);
/**
* Creates a permalink for a User. If isPill is set it uses a spec-compliant
* prefix for the permalink, instead of permalink_prefix
* @param {string} userId The user to link to.
* @param {boolean} isPill Link should be pillifyable.
* @returns {string|null} The transformed permalink or null if unable.
*/
export function makeUserPermalink(userId: string, isPill = false): string {
return getPermalinkConstructor(isPill).forUser(userId);
}
export function makeRoomPermalink(matrixClient: MatrixClient, roomId: string): string {
/**
* Creates a permalink for a room. If isPill is set it uses a spec-compliant
* prefix for the permalink, instead of permalink_prefix
* @param {MatrixClient} matrixClient The MatrixClient to use
* @param {string} roomId The user to link to.
* @param {boolean} isPill Link should be pillifyable.
* @returns {string|null} The transformed permalink or null if unable.
*/
export function makeRoomPermalink(matrixClient: MatrixClient, roomId: string, isPill = false): string {
if (!roomId) {
throw new Error("can't permalink a falsy 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 getPermalinkConstructor().forRoom(roomId, []);
if (roomId[0] !== "!") return getPermalinkConstructor(isPill).forRoom(roomId, []);
const room = matrixClient.getRoom(roomId);
if (!room) {
return getPermalinkConstructor().forRoom(roomId, []);
return getPermalinkConstructor(isPill).forRoom(roomId, []);
}
const permalinkCreator = new RoomPermalinkCreator(room);
permalinkCreator.load();
@ -414,9 +436,15 @@ export function getPrimaryPermalinkEntity(permalink: string): string | null {
return null;
}
function getPermalinkConstructor(): PermalinkConstructor {
/**
* Returns the correct PermalinkConstructor based on permalink_prefix
* and isPill
* @param {boolean} isPill Should constructed links be pillifyable.
* @returns {string|null} The transformed permalink or null if unable.
*/
function getPermalinkConstructor(isPill = false): PermalinkConstructor {
const elementPrefix = SdkConfig.get("permalink_prefix");
if (elementPrefix && elementPrefix !== matrixtoBaseUrl) {
if (elementPrefix && elementPrefix !== matrixtoBaseUrl && !isPill) {
return new ElementPermalinkConstructor(elementPrefix);
}

View File

@ -14,10 +14,14 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import { mocked } from "jest-mock";
import EditorModel from "../../src/editor/model";
import { htmlSerializeIfNeeded } from "../../src/editor/serialize";
import { createPartCreator } from "./mock";
import { IConfigOptions } from "../../src/IConfigOptions";
import SettingsStore from "../../src/settings/SettingsStore";
import SdkConfig from "../../src/SdkConfig";
describe("editor/serialize", function () {
describe("with markdown", function () {
@ -104,6 +108,32 @@ describe("editor/serialize", function () {
const html = htmlSerializeIfNeeded(model, {});
expect(html).toBe('<ol start="2021">\n<li>foo</li>\n</ol>\n');
});
describe("with permalink_prefix set", function () {
const sdkConfigGet = SdkConfig.get;
beforeEach(() => {
jest.spyOn(SdkConfig, "get").mockImplementation((key: keyof IConfigOptions, altCaseName?: string) => {
if (key === "permalink_prefix") {
return "https://element.fs.tld";
} else return sdkConfigGet(key, altCaseName);
});
});
it("user pill uses matrix.to", function () {
const pc = createPartCreator();
const model = new EditorModel([pc.userPill("Alice", "@alice:hs.tld")], pc);
const html = htmlSerializeIfNeeded(model, {});
expect(html).toBe('<a href="https://matrix.to/#/@alice:hs.tld">Alice</a>');
});
it("room pill uses matrix.to", function () {
const pc = createPartCreator();
const model = new EditorModel([pc.roomPill("#room:hs.tld")], pc);
const html = htmlSerializeIfNeeded(model, {});
expect(html).toBe('<a href="https://matrix.to/#/#room:hs.tld">#room:hs.tld</a>');
});
afterEach(() => {
mocked(SdkConfig.get).mockRestore();
});
});
});
describe("with plaintext", function () {

View File

@ -25,6 +25,8 @@ import {
parsePermalink,
RoomPermalinkCreator,
} from "../../../src/utils/permalinks/Permalinks";
import { IConfigOptions } from "../../../src/IConfigOptions";
import SdkConfig from "../../../src/SdkConfig";
import { getMockClientWithEventEmitter } from "../../test-utils";
describe("Permalinks", function () {
@ -391,6 +393,17 @@ describe("Permalinks", function () {
expect(result).toBe("https://matrix.to/#/@someone:example.org");
});
it("should use permalink_prefix for permalinks", function () {
const sdkConfigGet = SdkConfig.get;
jest.spyOn(SdkConfig, "get").mockImplementation((key: keyof IConfigOptions, altCaseName?: string) => {
if (key === "permalink_prefix") {
return "https://element.fs.tld";
} else return sdkConfigGet(key, altCaseName);
});
const result = makeUserPermalink("@someone:example.org");
expect(result).toBe("https://element.fs.tld/#/user/@someone:example.org");
});
describe("parsePermalink", () => {
it("should correctly parse room permalinks with a via argument", () => {
const result = parsePermalink("https://matrix.to/#/!room_id:server?via=some.org");