Merge pull request #4411 from matrix-org/t3chguy/backslash

Fix pills being broken by unescaped characters
2020-04-15 19:23:28 +01:00 · 2020-04-15 19:23:28 +01:00 · 2929bcf998
parent 4154acc9dd 9c1939b756
commit 2929bcf998
4 changed files with 80 additions and 30 deletions
--- a/src/editor/deserialize.ts
+++ b/src/editor/deserialize.ts
@ -1,6 +1,6 @@
 /*
 Copyright 2019 New Vector Ltd
-Copyright 2019 The Matrix.org Foundation C.I.C.
+Copyright 2019, 2020 The Matrix.org Foundation C.I.C.

 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@ -15,11 +15,14 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */

+import { MatrixEvent } from "matrix-js-sdk/src/models/event";
+
 import { walkDOMDepthFirst } from "./dom";
 import { checkBlockNode } from "../HtmlUtils";
 import { getPrimaryPermalinkEntity } from "../utils/permalinks/Permalinks";
+import { PartCreator } from "./parts";

-function parseAtRoomMentions(text, partCreator) {
+function parseAtRoomMentions(text: string, partCreator: PartCreator) {
    const ATROOM = "@room";
    const parts = [];
    text.split(ATROOM).forEach((textPart, i, arr) => {
@ -37,7 +40,7 @@ function parseAtRoomMentions(text, partCreator) {
    return parts;
 }

-function parseLink(a, partCreator) {
+function parseLink(a: HTMLAnchorElement, partCreator: PartCreator) {
    const {href} = a;
    const resourceId = getPrimaryPermalinkEntity(href); // The room/user ID
    const prefix = resourceId ? resourceId[0] : undefined; // First character of ID
@ -50,17 +53,17 @@ function parseLink(a, partCreator) {
            if (href === a.textContent) {
                return partCreator.plain(a.textContent);
            } else {
-                return partCreator.plain(`[${a.textContent}](${href})`);
+                return partCreator.plain(`[${a.textContent.replace(/[[\\\]]/g, c => "\\" + c)}](${href})`);
            }
        }
    }
 }

-function parseCodeBlock(n, partCreator) {
+function parseCodeBlock(n: HTMLElement, partCreator: PartCreator) {
    const parts = [];
    let language = "";
    if (n.firstChild && n.firstChild.nodeName === "CODE") {
-        for (const className of n.firstChild.classList) {
+        for (const className of (<HTMLElement>n.firstChild).classList) {
            if (className.startsWith("language-")) {
                language = className.substr("language-".length);
                break;
@ -77,12 +80,17 @@ function parseCodeBlock(n, partCreator) {
    return parts;
 }

-function parseHeader(el, partCreator) {
+function parseHeader(el: HTMLElement, partCreator: PartCreator) {
    const depth = parseInt(el.nodeName.substr(1), 10);
    return partCreator.plain("#".repeat(depth) + " ");
 }

-function parseElement(n, partCreator, lastNode, state) {
+interface IState {
+    listIndex: number[];
+    listDepth?: number;
+}
+
+function parseElement(n: HTMLElement, partCreator: PartCreator, lastNode: HTMLElement | undefined, state: IState) {
    switch (n.nodeName) {
        case "H1":
        case "H2":
@ -92,7 +100,7 @@ function parseElement(n, partCreator, lastNode, state) {
        case "H6":
            return parseHeader(n, partCreator);
        case "A":
-            return parseLink(n, partCreator);
+            return parseLink(<HTMLAnchorElement>n, partCreator);
        case "BR":
            return partCreator.newline();
        case "EM":
@ -123,11 +131,11 @@ function parseElement(n, partCreator, lastNode, state) {
            break;
        }
        case "OL":
-            state.listIndex.push(n.start || 1);
-            // fallthrough
+            state.listIndex.push((<HTMLOListElement>n).start || 1);
+            /* falls through */
        case "UL":
            state.listDepth = (state.listDepth || 0) + 1;
-            // fallthrough
+            /* falls through */
        default:
            // don't textify block nodes we'll descend into
            if (!checkDescendInto(n)) {
@ -174,7 +182,7 @@ function prefixQuoteLines(isFirstNode, parts, partCreator) {
    }
 }

-function parseHtmlMessage(html, partCreator, isQuotedMessage) {
+function parseHtmlMessage(html: string, partCreator: PartCreator, isQuotedMessage: boolean) {
    // no nodes from parsing here should be inserted in the document,
    // as scripts in event handlers, etc would be executed then.
    // we're only taking text, so that is fine
@ -182,7 +190,7 @@ function parseHtmlMessage(html, partCreator, isQuotedMessage) {
    const parts = [];
    let lastNode;
    let inQuote = isQuotedMessage;
-    const state = {
+    const state: IState = {
        listIndex: [],
    };

@ -236,7 +244,7 @@ function parseHtmlMessage(html, partCreator, isQuotedMessage) {
                break;
            case "OL":
                state.listIndex.pop();
-                // fallthrough
+                /* falls through */
            case "UL":
                state.listDepth -= 1;
                break;
@ -249,9 +257,9 @@ function parseHtmlMessage(html, partCreator, isQuotedMessage) {
    return parts;
 }

-export function parsePlainTextMessage(body, partCreator, isQuotedMessage) {
+export function parsePlainTextMessage(body: string, partCreator: PartCreator, isQuotedMessage: boolean) {
    const lines = body.split(/\r\n|\r|\n/g); // split on any new-line combination not just \n, collapses \r\n
-    const parts = lines.reduce((parts, line, i) => {
+    return lines.reduce((parts, line, i) => {
        if (isQuotedMessage) {
            parts.push(partCreator.plain(QUOTE_LINE_PREFIX));
        }
@ -262,10 +270,9 @@ export function parsePlainTextMessage(body, partCreator, isQuotedMessage) {
        }
        return parts;
    }, []);
-    return parts;
 }

-export function parseEvent(event, partCreator, {isQuotedMessage = false} = {}) {
+export function parseEvent(event: MatrixEvent, partCreator: PartCreator, {isQuotedMessage = false} = {}) {
    const content = event.getContent();
    let parts;
    if (content.format === "org.matrix.custom.html") {
--- a/src/editor/serialize.ts
+++ b/src/editor/serialize.ts
@ -1,6 +1,6 @@
 /*
 Copyright 2019 New Vector Ltd
-Copyright 2019 The Matrix.org Foundation C.I.C.
+Copyright 2019, 2020 The Matrix.org Foundation C.I.C.

 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@ -17,8 +17,9 @@ limitations under the License.

 import Markdown from '../Markdown';
 import {makeGenericPermalink} from "../utils/permalinks/Permalinks";
+import EditorModel from "./model";

-export function mdSerialize(model) {
+export function mdSerialize(model: EditorModel) {
    return model.parts.reduce((html, part) => {
        switch (part.type) {
            case "newline":
@ -30,12 +31,12 @@ export function mdSerialize(model) {
                return html + part.text;
            case "room-pill":
            case "user-pill":
-                return html + `[${part.text}](${makeGenericPermalink(part.resourceId)})`;
+                return html + `[${part.text.replace(/[[\\\]]/g, c => "\\" + c)}](${makeGenericPermalink(part.resourceId)})`;
        }
    }, "");
 }

-export function htmlSerializeIfNeeded(model, {forceHTML = false} = {}) {
+export function htmlSerializeIfNeeded(model: EditorModel, {forceHTML = false} = {}) {
    const md = mdSerialize(model);
    const parser = new Markdown(md);
    if (!parser.isPlainText() || forceHTML) {
@ -43,7 +44,7 @@ export function htmlSerializeIfNeeded(model, {forceHTML = false} = {}) {
    }
 }

-export function textSerialize(model) {
+export function textSerialize(model: EditorModel) {
    return model.parts.reduce((text, part) => {
        switch (part.type) {
            case "newline":
@ -60,11 +61,11 @@ export function textSerialize(model) {
    }, "");
 }

-export function containsEmote(model) {
+export function containsEmote(model: EditorModel) {
    return startsWith(model, "/me ");
 }

-export function startsWith(model, prefix) {
+export function startsWith(model: EditorModel, prefix: string) {
    const firstPart = model.parts[0];
    // part type will be "plain" while editing,
    // and "command" while composing a message.
@ -73,18 +74,18 @@ export function startsWith(model, prefix) {
        firstPart.text.startsWith(prefix);
 }

-export function stripEmoteCommand(model) {
+export function stripEmoteCommand(model: EditorModel) {
    // trim "/me "
    return stripPrefix(model, "/me ");
 }

-export function stripPrefix(model, prefix) {
+export function stripPrefix(model: EditorModel, prefix: string) {
    model = model.clone();
    model.removeText({index: 0, offset: 0}, prefix.length);
    return model;
 }

-export function unescapeMessage(model) {
+export function unescapeMessage(model: EditorModel) {
    const {parts} = model;
    if (parts.length) {
        const firstPart = parts[0];
--- a/test/editor/deserialize-test.js
+++ b/test/editor/deserialize-test.js
@ -148,6 +148,30 @@ describe('editor/deserialize', function() {
            expect(parts[1]).toStrictEqual({type: "user-pill", text: "Alice", resourceId: "@alice:hs.tld"});
            expect(parts[2]).toStrictEqual({type: "plain", text: "!"});
        });
+        it('user pill with displayname containing backslash', function() {
+            const html = "Hi <a href=\"https://matrix.to/#/@alice:hs.tld\">Alice\\</a>!";
+            const parts = normalize(parseEvent(htmlMessage(html), createPartCreator()));
+            expect(parts.length).toBe(3);
+            expect(parts[0]).toStrictEqual({type: "plain", text: "Hi "});
+            expect(parts[1]).toStrictEqual({type: "user-pill", text: "Alice\\", resourceId: "@alice:hs.tld"});
+            expect(parts[2]).toStrictEqual({type: "plain", text: "!"});
+        });
+        it('user pill with displayname containing opening square bracket', function() {
+            const html = "Hi <a href=\"https://matrix.to/#/@alice:hs.tld\">Alice[[</a>!";
+            const parts = normalize(parseEvent(htmlMessage(html), createPartCreator()));
+            expect(parts.length).toBe(3);
+            expect(parts[0]).toStrictEqual({type: "plain", text: "Hi "});
+            expect(parts[1]).toStrictEqual({type: "user-pill", text: "Alice[[", resourceId: "@alice:hs.tld"});
+            expect(parts[2]).toStrictEqual({type: "plain", text: "!"});
+        });
+        it('user pill with displayname containing closing square bracket', function() {
+            const html = "Hi <a href=\"https://matrix.to/#/@alice:hs.tld\">Alice]</a>!";
+            const parts = normalize(parseEvent(htmlMessage(html), createPartCreator()));
+            expect(parts.length).toBe(3);
+            expect(parts[0]).toStrictEqual({type: "plain", text: "Hi "});
+            expect(parts[1]).toStrictEqual({type: "user-pill", text: "Alice]", resourceId: "@alice:hs.tld"});
+            expect(parts[2]).toStrictEqual({type: "plain", text: "!"});
+        });
        it('room pill', function() {
            const html = "Try <a href=\"https://matrix.to/#/#room:hs.tld\">#room:hs.tld</a>?";
            const parts = normalize(parseEvent(htmlMessage(html), createPartCreator()));
--- a/test/editor/serialize-test.js
+++ b/test/editor/serialize-test.js
@ -43,4 +43,22 @@ describe('editor/serialize', function() {
        const html = htmlSerializeIfNeeded(model, {});
        expect(html).toBe("<em>hello</em> world");
    });
+    it('displaynames ending in a backslash work', function() {
+        const pc = createPartCreator();
+        const model = new EditorModel([pc.userPill("Displayname\\", "@user:server")]);
+        const html = htmlSerializeIfNeeded(model, {});
+        expect(html).toBe("<a href=\"https://matrix.to/#/@user:server\">Displayname\\</a>");
+    });
+    it('displaynames containing an opening square bracket work', function() {
+        const pc = createPartCreator();
+        const model = new EditorModel([pc.userPill("Displayname[[", "@user:server")]);
+        const html = htmlSerializeIfNeeded(model, {});
+        expect(html).toBe("<a href=\"https://matrix.to/#/@user:server\">Displayname[[</a>");
+    });
+    it('displaynames containing a closing square bracket work', function() {
+        const pc = createPartCreator();
+        const model = new EditorModel([pc.userPill("Displayname]", "@user:server")]);
+        const html = htmlSerializeIfNeeded(model, {});
+        expect(html).toBe("<a href=\"https://matrix.to/#/@user:server\">Displayname]</a>");
+    });
 });