Fix soft crash around inviting invalid MXIDs in start DM on first message flow (#9281)

t3chguy · web-flow · commit 4a23630e0695 · 2022-09-16T09:03:17.000+01:00
* Fix soft crash around inviting invalid MXIDs

* Make ts --strict happier

* Prevent suggesting invalid MXIDs

* Add tests

* Fix coverage

* Fix coverage

* Make tsc --strict happier

* Fix test

* Add tests
diff --git a/cypress/e2e/crypto/crypto.spec.ts b/cypress/e2e/crypto/crypto.spec.ts
@@ -50,7 +50,7 @@ const checkDMRoom = () => {
 
 const startDMWithBob = function(this: CryptoTestContext) {
     cy.get('.mx_RoomList [aria-label="Start chat"]').click();
-    cy.get('[data-test-id="invite-dialog-input"]').type(this.bob.getUserId());
+    cy.get('[data-testid="invite-dialog-input"]').type(this.bob.getUserId());
     cy.contains(".mx_InviteDialog_tile_nameStack_name", "Bob").click();
     cy.contains(".mx_InviteDialog_userTile_pill .mx_InviteDialog_userTile_name", "Bob").should("exist");
     cy.get(".mx_InviteDialog_goButton").click();
diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx
@@ -25,7 +25,12 @@ import { Icon as InfoIcon } from "../../../../res/img/element-icons/info.svg";
 import { Icon as EmailPillAvatarIcon } from "../../../../res/img/icon-email-pill-avatar.svg";
 import { _t, _td } from "../../../languageHandler";
 import { MatrixClientPeg } from "../../../MatrixClientPeg";
-import { makeRoomPermalink, makeUserPermalink } from "../../../utils/permalinks/Permalinks";
+import {
+    getHostnameFromMatrixServerName,
+    getServerName,
+    makeRoomPermalink,
+    makeUserPermalink,
+} from "../../../utils/permalinks/Permalinks";
 import DMRoomMap from "../../../utils/DMRoomMap";
 import SdkConfig from "../../../SdkConfig";
 import * as Email from "../../../email";
@@ -69,7 +74,7 @@ import {
     startDmOnFirstMessage,
     ThreepidMember,
 } from "../../../utils/direct-messages";
-import { AnyInviteKind, KIND_CALL_TRANSFER, KIND_DM, KIND_INVITE } from './InviteDialogTypes';
+import { KIND_CALL_TRANSFER, KIND_DM, KIND_INVITE } from './InviteDialogTypes';
 import Modal from '../../../Modal';
 import dis from "../../../dispatcher/dispatcher";
 
@@ -243,26 +248,37 @@ class DMRoomTile extends React.PureComponent<IDMRoomTileProps> {
     }
 }
 
-interface IInviteDialogProps {
+interface BaseProps {
     // Takes a boolean which is true if a user / users were invited /
     // a call transfer was initiated or false if the dialog was cancelled
     // with no action taken.
     onFinished: (success: boolean) => void;
 
-    // The kind of invite being performed. Assumed to be KIND_DM if
-    // not provided.
-    kind: AnyInviteKind;
+    // Initial value to populate the filter with
+    initialText?: string;
+}
+
+interface InviteDMProps extends BaseProps {
+    // The kind of invite being performed. Assumed to be KIND_DM if not provided.
+    kind?: typeof KIND_DM;
+}
+
+interface InviteRoomProps extends BaseProps {
+    kind: typeof KIND_INVITE;
 
     // The room ID this dialog is for. Only required for KIND_INVITE.
     roomId: string;
+}
+
+interface InviteCallProps extends BaseProps {
+    kind: typeof KIND_CALL_TRANSFER;
 
     // The call to transfer. Only required for KIND_CALL_TRANSFER.
     call: MatrixCall;
-
-    // Initial value to populate the filter with
-    initialText: string;
 }
 
+type Props = InviteDMProps | InviteRoomProps | InviteCallProps;
+
 interface IInviteDialogState {
     targets: Member[]; // array of Member objects (see interface above)
     filterText: string;
@@ -283,14 +299,13 @@ interface IInviteDialogState {
     errorText: string;
 }
 
-export default class InviteDialog extends React.PureComponent<IInviteDialogProps, IInviteDialogState> {
+export default class InviteDialog extends React.PureComponent<Props, IInviteDialogState> {
     static defaultProps = {
         kind: KIND_DM,
         initialText: "",
     };
 
-    private closeCopiedTooltip: () => void;
-    private debounceTimer: number = null; // actually number because we're in the browser
+    private debounceTimer: number | null = null; // actually number because we're in the browser
     private editorRef = createRef<HTMLInputElement>();
     private numberEntryFieldRef: React.RefObject<Field> = createRef();
     private unmounted = false;
@@ -316,7 +331,7 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
 
         this.state = {
             targets: [], // array of Member objects (see interface above)
-            filterText: this.props.initialText,
+            filterText: this.props.initialText || "",
             recents: InviteDialog.buildRecents(alreadyInvited),
             numRecentsShown: INITIAL_ROOMS_SHOWN,
             suggestions: this.buildSuggestions(alreadyInvited),
@@ -343,9 +358,6 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
 
     componentWillUnmount() {
         this.unmounted = true;
-        // if the Copied tooltip is open then get rid of it, there are ways to close the modal which wouldn't close
-        // the tooltip otherwise, such as pressing Escape or clicking X really quickly
-        if (this.closeCopiedTooltip) this.closeCopiedTooltip();
     }
 
     private onConsultFirstChange = (ev) => {
@@ -466,6 +478,7 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
     };
 
     private inviteUsers = async () => {
+        if (this.props.kind !== KIND_INVITE) return;
         this.setState({ busy: true });
         this.convertFilter();
         const targets = this.convertFilter();
@@ -499,6 +512,7 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
     };
 
     private transferCall = async () => {
+        if (this.props.kind !== KIND_CALL_TRANSFER) return;
         if (this.state.currentTabId == TabId.UserDirectory) {
             this.convertFilter();
             const targets = this.convertFilter();
@@ -565,7 +579,7 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
         this.props.onFinished(false);
     };
 
-    private updateSuggestions = async (term) => {
+    private updateSuggestions = async (term: string) => {
         MatrixClientPeg.get().searchUserDirectory({ term }).then(async r => {
             if (term !== this.state.filterText) {
                 // Discard the results - we were probably too slow on the server-side to make
@@ -595,13 +609,17 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
                     logger.warn("Non-fatal error trying to make an invite for a user ID");
                     logger.warn(e);
 
-                    // Add a result anyways, just without a profile. We stick it at the
-                    // top so it is most obviously presented to the user.
-                    r.results.splice(0, 0, {
-                        user_id: term,
-                        display_name: term,
-                        avatar_url: null,
-                    });
+                    // Reuse logic from Permalinks as a basic MXID validity check
+                    const serverName = getServerName(term);
+                    const domain = getHostnameFromMatrixServerName(serverName);
+                    if (domain) {
+                        // Add a result anyways, just without a profile. We stick it at the
+                        // top so it is most obviously presented to the user.
+                        r.results.splice(0, 0, {
+                            user_id: term,
+                            display_name: term,
+                        });
+                    }
                 }
             }
 
@@ -940,7 +958,7 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
                 disabled={this.state.busy || (this.props.kind == KIND_CALL_TRANSFER && this.state.targets.length > 0)}
                 autoComplete="off"
                 placeholder={hasPlaceholder ? _t("Search") : null}
-                data-test-id="invite-dialog-input"
+                data-testid="invite-dialog-input"
             />
         );
         return (
@@ -1107,14 +1125,15 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
                 </CopyableText>
             </div>;
         } else if (this.props.kind === KIND_INVITE) {
-            const room = MatrixClientPeg.get()?.getRoom(this.props.roomId);
+            const roomId = this.props.roomId;
+            const room = MatrixClientPeg.get()?.getRoom(roomId);
             const isSpace = room?.isSpaceRoom();
             title = isSpace
                 ? _t("Invite to %(spaceName)s", {
-                    spaceName: room.name || _t("Unnamed Space"),
+                    spaceName: room?.name || _t("Unnamed Space"),
                 })
                 : _t("Invite to %(roomName)s", {
-                    roomName: room.name || _t("Unnamed Room"),
+                    roomName: room?.name || _t("Unnamed Room"),
                 });
 
             let helpTextUntranslated;
@@ -1140,14 +1159,14 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
                 userId: () =>
                     <a href={makeUserPermalink(userId)} rel="noreferrer noopener" target="_blank">{ userId }</a>,
                 a: (sub) =>
-                    <a href={makeRoomPermalink(this.props.roomId)} rel="noreferrer noopener" target="_blank">{ sub }</a>,
+                    <a href={makeRoomPermalink(roomId)} rel="noreferrer noopener" target="_blank">{ sub }</a>,
             });
 
             buttonText = _t("Invite");
             goButtonFn = this.inviteUsers;
 
             if (cli.isRoomEncrypted(this.props.roomId)) {
-                const room = cli.getRoom(this.props.roomId);
+                const room = cli.getRoom(this.props.roomId)!;
                 const visibilityEvent = room.currentState.getStateEvents(
                     "m.room.history_visibility", "",
                 );
@@ -1185,8 +1204,6 @@ export default class InviteDialog extends React.PureComponent<IInviteDialogProps
                     { _t("Transfer") }
                 </AccessibleButton>
             </div>;
-        } else {
-            logger.error("Unknown kind of InviteDialog: " + this.props.kind);
         }
 
         const goButton = this.props.kind == KIND_CALL_TRANSFER ? null : <AccessibleButton
diff --git a/src/components/views/dialogs/ShareDialog.tsx b/src/components/views/dialogs/ShareDialog.tsx
@@ -71,8 +71,6 @@ interface IState {
 }
 
 export default class ShareDialog extends React.PureComponent<IProps, IState> {
-    protected closeCopiedTooltip: () => void;
-
     constructor(props) {
         super(props);
 
@@ -100,12 +98,6 @@ export default class ShareDialog extends React.PureComponent<IProps, IState> {
         });
     };
 
-    componentWillUnmount() {
-        // if the Copied tooltip is open then get rid of it, there are ways to close the modal which wouldn't close
-        // the tooltip otherwise, such as pressing Escape or clicking X really quickly
-        if (this.closeCopiedTooltip) this.closeCopiedTooltip();
-    }
-
     private getUrl() {
         let matrixToUrl;
 
diff --git a/src/utils/permalinks/Permalinks.ts b/src/utils/permalinks/Permalinks.ts
@@ -180,7 +180,7 @@ export class RoomPermalinkCreator {
         if (plEvent) {
             const content = plEvent.getContent();
             if (content) {
-                const users = content.users;
+                const users: Record<string, number> = content.users;
                 if (users) {
                     const entries = Object.entries(users);
                     const allowedEntries = entries.filter(([userId]) => {
@@ -189,9 +189,11 @@ export class RoomPermalinkCreator {
                             return false;
                         }
                         const serverName = getServerName(userId);
-                        return !isHostnameIpAddress(serverName) &&
-                            !isHostInRegex(serverName, this.bannedHostsRegexps) &&
-                            isHostInRegex(serverName, this.allowedHostsRegexps);
+
+                        const domain = getHostnameFromMatrixServerName(serverName) ?? serverName;
+                        return !isHostnameIpAddress(domain) &&
+                            !isHostInRegex(domain, this.bannedHostsRegexps) &&
+                            isHostInRegex(domain, this.allowedHostsRegexps);
                     });
                     const maxEntry = allowedEntries.reduce((max, entry) => {
                         return (entry[1] > max[1]) ? entry : max;
@@ -250,14 +252,15 @@ export class RoomPermalinkCreator {
             .sort((a, b) => this.populationMap[b] - this.populationMap[a]);
 
         for (let i = 0; i < serversByPopulation.length && candidates.size < MAX_SERVER_CANDIDATES; i++) {
-            const server = serversByPopulation[i];
+            const serverName = serversByPopulation[i];
+            const domain = getHostnameFromMatrixServerName(serverName) ?? "";
             if (
-                !candidates.has(server) &&
-                !isHostnameIpAddress(server) &&
-                !isHostInRegex(server, this.bannedHostsRegexps) &&
-                isHostInRegex(server, this.allowedHostsRegexps)
+                !candidates.has(serverName) &&
+                !isHostnameIpAddress(domain) &&
+                !isHostInRegex(domain, this.bannedHostsRegexps) &&
+                isHostInRegex(domain, this.allowedHostsRegexps)
             ) {
-                candidates.add(server);
+                candidates.add(serverName);
             }
         }
 
@@ -441,25 +444,28 @@ export function parsePermalink(fullUrl: string): PermalinkParts {
     return null; // not a permalink we can handle
 }
 
-function getServerName(userId: string): string {
+export function getServerName(userId: string): string {
     return userId.split(":").splice(1).join(":");
 }
 
-function getHostnameFromMatrixDomain(domain: string): string {
-    if (!domain) return null;
-    return new URL(`https://${domain}`).hostname;
+export function getHostnameFromMatrixServerName(serverName: string): string | null {
+    if (!serverName) return null;
+    try {
+        return new URL(`https://${serverName}`).hostname;
+    } catch (e) {
+        console.error("Error encountered while extracting hostname from server name", e);
+        return null;
+    }
 }
 
 function isHostInRegex(hostname: string, regexps: RegExp[]): boolean {
-    hostname = getHostnameFromMatrixDomain(hostname);
     if (!hostname) return true; // assumed
     if (regexps.length > 0 && !regexps[0].test) throw new Error(regexps[0].toString());
 
     return regexps.some(h => h.test(hostname));
 }
 
 function isHostnameIpAddress(hostname: string): boolean {
-    hostname = getHostnameFromMatrixDomain(hostname);
     if (!hostname) return false;
 
     // is-ip doesn't want IPv6 addresses surrounded by brackets, so
diff --git a/test/components/views/dialogs/InviteDialog-test.tsx b/test/components/views/dialogs/InviteDialog-test.tsx
diff --git a/test/utils/permalinks/Permalinks-test.ts b/test/utils/permalinks/Permalinks-test.ts
