Ensure we disambiguate display names which look like MXIDs (#4540)

* Ensure we disambiguate display names which look like MXIDs

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Make tests clearer

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

spec/unit/room-member.spec.ts

@@ -486,7 +486,43 @@ describe("RoomMember", function () {
             } as unknown as RoomState;
             expect(member.name).toEqual(userA); // default = user_id
             member.setMembershipEvent(joinEvent, roomState);
-            expect(member.name).not.toEqual("Alíce"); // it should disambig.
+            expect(member.name).toEqual("Alíce​ (@alice:bar)"); // it should disambig.
+            // user_id should be there somewhere
+            expect(member.name.indexOf(userA)).not.toEqual(-1);
+        });
+
+        it("should disambiguate a user when their displayname looks like an MXID which isn't theirs", function () {
+            const joinEvent = utils.mkMembership({
+                event: true,
+                mship: KnownMembership.Join,
+                user: userA,
+                room: roomId,
+                name: "@clarissa\u0a83bar",
+            });
+
+            const roomState = {
+                getStateEvents: function (type: string) {
+                    if (type !== "m.room.member") {
+                        return [];
+                    }
+                    return [
+                        utils.mkMembership({
+                            event: true,
+                            mship: KnownMembership.Join,
+                            room: roomId,
+                            user: userC,
+                            name: "Alice",
+                        }),
+                        joinEvent,
+                    ];
+                },
+                getUserIdsWithDisplayName: function (displayName: string) {
+                    return [userA, userC];
+                },
+            } as unknown as RoomState;
+            expect(member.name).toEqual(userA); // default = user_id
+            member.setMembershipEvent(joinEvent, roomState);
+            expect(member.name).toEqual("@clarissaઃbar (@alice:bar)"); // it should disambig.
             // user_id should be there somewhere
             expect(member.name.indexOf(userA)).not.toEqual(-1);
         });

spec/unit/utils.spec.ts

@@ -32,6 +32,7 @@ import {
     MapWithDefault,
     globToRegexp,
     escapeRegExp,
+    removeHiddenChars,
 } from "../../src/utils";
 import { logger } from "../../src/logger";
 import { mkMessage } from "../test-utils/test-utils";
@@ -740,4 +741,19 @@ describe("utils", function () {
             expect(result).toMatchInlineSnapshot(`"\\[FIT-Connect Zustelldienst \\\\\\(Testumgebung\\\\\\)\\]"`);
         });
     });
+
+    describe("removeHiddenChars", () => {
+        it.each([
+            ["various width spaces U+2000 - U+200D", "\u2000\u200D", ""],
+            ["LTR and RTL marks U+200E and U+200F", "\u200E\u200F", ""],
+            ["LTR/RTL and other directional formatting marks U+202A - U+202F", "\u202A\u202F", ""],
+            ["Arabic Letter RTL mark U+061C", "\u061C", ""],
+            ["Combining characters U+0300 - U+036F", "\u3000\u036F", ""],
+            ["Zero width no-break space (BOM) U+FEFF", "\uFEFF", ""],
+            ["Blank/invisible characters (U2800, U2062-U2063)", "\u2800\u2062\u2063", ""],
+            ["Zero Width Non Joiner", "‌", ""],
+        ])("should strip invisible characters: %s", (_, input, expected) => {
+            expect(removeHiddenChars(input)).toBe(expected);
+        });
+    });
 });

src/models/room-member.ts

@@ -408,17 +408,18 @@ const LTR_RTL_PATTERN = /[\u200E\u200F\u202A-\u202F]/;
 
 function shouldDisambiguate(selfUserId: string, displayName?: string, roomState?: RoomState): boolean {
     if (!displayName || displayName === selfUserId) return false;
+    if (!roomState) return false;
+
+    const strippedDisplayName = removeHiddenChars(displayName);
 
     // First check if the displayname is something we consider truthy
     // after stripping it of zero width characters and padding spaces
-    if (!removeHiddenChars(displayName)) return false;
+    if (!strippedDisplayName) return false;
-
-    if (!roomState) return false;
 
     // Next check if the name contains something that look like a mxid
     // If it does, it may be someone trying to impersonate someone else
     // Show full mxid in this case
-    if (MXID_PATTERN.test(displayName)) return true;
+    if (MXID_PATTERN.test(strippedDisplayName)) return true;
 
     // Also show mxid if the display name contains any LTR/RTL characters as these
     // make it very difficult for us to find similar *looking* display names