Merge pull request #1479 from vector-im/dbkr/refactor_urlparams

Small refactor of URLParams stuff

src/UrlParams.ts

@@ -21,10 +21,21 @@ import { Config } from "./config/Config";
 
 export const PASSWORD_STRING = "password=";
 
-interface UrlParams {
+interface RoomIdentifier {
   roomAlias: string | null;
   roomId: string | null;
   viaServers: string[];
+}
+
+interface UrlParams {
+  /**
+   * Anything about what room we're pointed to should be from useRoomIdentifier which
+   * parses the path and resolves alias with respect to the default server name, however
+   * roomId is an exception as we need the room ID in embedded (matroyska) mode, and not
+   * the room alias (or even the via params because we are not trying to join it). This
+   * is also not validated, where it is in useRoomIdentifier().
+   */
+  roomId: string | null;
   /**
    * Whether the app is running in embedded mode, and should keep the user
    * confined to the current room.
@@ -106,25 +117,92 @@ export function editFragmentQuery(
   )}?${fragmentParams.toString()}`;
 }
 
+class ParamParser {
+  private fragmentParams: URLSearchParams;
+  private queryParams: URLSearchParams;
+
+  constructor(search: string, hash: string) {
+    this.queryParams = new URLSearchParams(search);
+
+    const fragmentQueryStart = hash.indexOf("?");
+    this.fragmentParams = new URLSearchParams(
+      fragmentQueryStart === -1 ? "" : hash.substring(fragmentQueryStart)
+    );
+  }
+
+  // Normally, URL params should be encoded in the fragment so as to avoid
+  // leaking them to the server. However, we also check the normal query
+  // string for backwards compatibility with versions that only used that.
+  hasParam(name: string): boolean {
+    return this.fragmentParams.has(name) || this.queryParams.has(name);
+  }
+
+  getParam(name: string): string | null {
+    return this.fragmentParams.get(name) ?? this.queryParams.get(name);
+  }
+
+  getAllParams(name: string): string[] {
+    return [
+      ...this.fragmentParams.getAll(name),
+      ...this.queryParams.getAll(name),
+    ];
+  }
+}
+
 /**
  * Gets the app parameters for the current URL.
- * @param ignoreRoomAlias If true, does not try to parse a room alias from the URL
  * @param search The URL search string
- * @param pathname The URL path name
  * @param hash The URL hash
  * @returns The app parameters encoded in the URL
  */
 export const getUrlParams = (
-  ignoreRoomAlias?: boolean,
   search = window.location.search,
-  pathname = window.location.pathname,
   hash = window.location.hash
 ): UrlParams => {
-  // This is legacy code - we're moving away from using aliases
+  const parser = new ParamParser(search, hash);
+
+  const fontScale = parseFloat(parser.getParam("fontScale") ?? "");
+
+  return {
+    // NB. we don't validate roomId here as we do in getRoomIdentifierFromUrl:
+    // what would we do if it were invalid? If the widget API says that's what
+    // the room ID is, then that's what it is.
+    roomId: parser.getParam("roomId"),
+    password: parser.getParam("password"),
+    isEmbedded: parser.hasParam("embed"),
+    preload: parser.hasParam("preload"),
+    hideHeader: parser.hasParam("hideHeader"),
+    hideScreensharing: parser.hasParam("hideScreensharing"),
+    e2eEnabled: parser.getParam("enableE2e") !== "false", // Defaults to true
+    userId: parser.getParam("userId"),
+    displayName: parser.getParam("displayName"),
+    deviceId: parser.getParam("deviceId"),
+    baseUrl: parser.getParam("baseUrl"),
+    lang: parser.getParam("lang"),
+    fonts: parser.getAllParams("font"),
+    fontScale: Number.isNaN(fontScale) ? null : fontScale,
+    analyticsID: parser.getParam("analyticsID"),
+    allowIceFallback: parser.hasParam("allowIceFallback"),
+  };
+};
+
+/**
+ * Hook to simplify use of getUrlParams.
+ * @returns The app parameters for the current URL
+ */
+export const useUrlParams = (): UrlParams => {
+  const { search, hash } = useLocation();
+  return useMemo(() => getUrlParams(search, hash), [search, hash]);
+};
+
+export function getRoomIdentifierFromUrl(
+  pathname: string,
+  search: string,
+  hash: string
+): RoomIdentifier {
   let roomAlias: string | null = null;
-  if (!ignoreRoomAlias) {
+
-    // Here we handle the beginning of the alias and make sure it starts with a
+  // Here we handle the beginning of the alias and make sure it starts with a "#"
-    // "#"
   if (hash === "" || hash.startsWith("#?")) {
     roomAlias = pathname.substring(1); // Strip the "/"
 
@@ -152,30 +230,11 @@ export const getUrlParams = (
       roomAlias = `${roomAlias}:${Config.defaultServerName()}`;
     }
   }
-  }
 
-  const fragmentQueryStart = hash.indexOf("?");
+  const parser = new ParamParser(search, hash);
-  const fragmentParams = new URLSearchParams(
-    fragmentQueryStart === -1 ? "" : hash.substring(fragmentQueryStart)
-  );
-  const queryParams = new URLSearchParams(search);
-
-  // Normally, URL params should be encoded in the fragment so as to avoid
-  // leaking them to the server. However, we also check the normal query
-  // string for backwards compatibility with versions that only used that.
-  const hasParam = (name: string): boolean =>
-    fragmentParams.has(name) || queryParams.has(name);
-  const getParam = (name: string): string | null =>
-    fragmentParams.get(name) ?? queryParams.get(name);
-  const getAllParams = (name: string): string[] => [
-    ...fragmentParams.getAll(name),
-    ...queryParams.getAll(name),
-  ];
-
-  const fontScale = parseFloat(getParam("fontScale") ?? "");
 
   // Make sure roomId is valid
-  let roomId: string | null = getParam("roomId");
+  let roomId: string | null = parser.getParam("roomId");
   if (!roomId?.startsWith("!")) {
     roomId = null;
   } else if (!roomId.includes("")) {
@@ -185,33 +244,14 @@ export const getUrlParams = (
   return {
     roomAlias,
     roomId,
-    password: getParam("password"),
+    viaServers: parser.getAllParams("viaServers"),
-    viaServers: getAllParams("via"),
-    isEmbedded: hasParam("embed"),
-    preload: hasParam("preload"),
-    hideHeader: hasParam("hideHeader"),
-    hideScreensharing: hasParam("hideScreensharing"),
-    e2eEnabled: getParam("enableE2e") !== "false", // Defaults to true
-    userId: getParam("userId"),
-    displayName: getParam("displayName"),
-    deviceId: getParam("deviceId"),
-    baseUrl: getParam("baseUrl"),
-    lang: getParam("lang"),
-    fonts: getAllParams("font"),
-    fontScale: Number.isNaN(fontScale) ? null : fontScale,
-    analyticsID: getParam("analyticsID"),
-    allowIceFallback: hasParam("allowIceFallback"),
   };
-};
+}
 
-/**
+export const useRoomIdentifier = (): RoomIdentifier => {
- * Hook to simplify use of getUrlParams.
+  const { pathname, search, hash } = useLocation();
- * @returns The app parameters for the current URL
- */
-export const useUrlParams = (): UrlParams => {
-  const { search, pathname, hash } = useLocation();
   return useMemo(
-    () => getUrlParams(false, search, pathname, hash),
+    () => getRoomIdentifierFromUrl(pathname, search, hash),
-    [search, pathname, hash]
+    [pathname, search, hash]
   );
 };

src/initializer.tsx

@@ -62,7 +62,7 @@ export class Initializer {
     languageDetector.addDetector({
       name: "urlFragment",
       // Look for a language code in the URL's fragment
-      lookup: () => getUrlParams(true).lang ?? undefined,
+      lookup: () => getUrlParams().lang ?? undefined,
     });
 
     i18n
@@ -95,7 +95,7 @@ export class Initializer {
     }
 
     // Custom fonts
-    const { fonts, fontScale } = getUrlParams(true);
+    const { fonts, fontScale } = getUrlParams();
     if (fontScale !== null) {
       document.documentElement.style.setProperty(
         "--font-scale",

src/room/RoomPage.tsx

@@ -22,7 +22,7 @@ import { ErrorView, LoadingView } from "../FullScreenView";
 import { RoomAuthView } from "./RoomAuthView";
 import { GroupCallLoader } from "./GroupCallLoader";
 import { GroupCallView } from "./GroupCallView";
-import { useUrlParams } from "../UrlParams";
+import { useRoomIdentifier, useUrlParams } from "../UrlParams";
 import { useRegisterPasswordlessUser } from "../auth/useRegisterPasswordlessUser";
 import { useOptInAnalytics } from "../settings/useSetting";
 import { HomePage } from "../home/HomePage";
@@ -30,15 +30,10 @@ import { platform } from "../Platform";
 import { AppSelectionModal } from "./AppSelectionModal";
 
 export const RoomPage: FC = () => {
-  const {
+  const { isEmbedded, preload, hideHeader, displayName } = useUrlParams();
-    roomAlias,
+
-    roomId,
+  const { roomAlias, roomId, viaServers } = useRoomIdentifier();
-    viaServers,
+
-    isEmbedded,
-    preload,
-    hideHeader,
-    displayName,
-  } = useUrlParams();
   const roomIdOrAlias = roomId ?? roomAlias;
   if (!roomIdOrAlias) {
     console.error("No room specified");

src/widget.ts

@@ -109,7 +109,7 @@ export const widget: WidgetHelpers | null = (() => {
         baseUrl,
         e2eEnabled,
         allowIceFallback,
-      } = getUrlParams(true);
+      } = getUrlParams();
       if (!roomId) throw new Error("Room ID must be supplied");
       if (!userId) throw new Error("User ID must be supplied");
       if (!deviceId) throw new Error("Device ID must be supplied");

test/UrlParams-test.ts

@@ -15,7 +15,8 @@ limitations under the License.
 */
 
 import { mocked } from "jest-mock";
-import { getUrlParams } from "../src/UrlParams";
+
+import { getRoomIdentifierFromUrl } from "../src/UrlParams";
 import { Config } from "../src/config/Config";
 
 const ROOM_NAME = "roomNameHere";
@@ -32,27 +33,28 @@ describe("UrlParams", () => {
 
   describe("handles URL with /room/", () => {
     it("and nothing else", () => {
-      expect(getUrlParams(false, "", `/room/${ROOM_NAME}`, "").roomAlias).toBe(
+      expect(
-        `#${ROOM_NAME}:${HOMESERVER}`
+        getRoomIdentifierFromUrl(`/room/${ROOM_NAME}`, "", "").roomAlias
-      );
+      ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
 
     it("and #", () => {
       expect(
-        getUrlParams(false, "", `${ORIGIN}/room/`, `#${ROOM_NAME}`).roomAlias
+        getRoomIdentifierFromUrl("", `${ORIGIN}/room/`, `#${ROOM_NAME}`)
+          .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
 
     it("and # and server part", () => {
       expect(
-        getUrlParams(false, "", `/room/`, `#${ROOM_NAME}:${HOMESERVER}`)
+        getRoomIdentifierFromUrl("", `/room/`, `#${ROOM_NAME}:${HOMESERVER}`)
           .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
 
     it("and server part", () => {
       expect(
-        getUrlParams(false, "", `/room/${ROOM_NAME}:${HOMESERVER}`, "")
+        getRoomIdentifierFromUrl(`/room/${ROOM_NAME}:${HOMESERVER}`, "", "")
           .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
@@ -60,39 +62,44 @@ describe("UrlParams", () => {
 
   describe("handles URL without /room/", () => {
     it("and nothing else", () => {
-      expect(getUrlParams(false, "", `/${ROOM_NAME}`, "").roomAlias).toBe(
+      expect(getRoomIdentifierFromUrl(`/${ROOM_NAME}`, "", "").roomAlias).toBe(
         `#${ROOM_NAME}:${HOMESERVER}`
       );
     });
 
     it("and with #", () => {
-      expect(getUrlParams(false, "", "", `#${ROOM_NAME}`).roomAlias).toBe(
+      expect(getRoomIdentifierFromUrl("", "", `#${ROOM_NAME}`).roomAlias).toBe(
         `#${ROOM_NAME}:${HOMESERVER}`
       );
     });
 
     it("and with # and server part", () => {
       expect(
-        getUrlParams(false, "", "", `#${ROOM_NAME}:${HOMESERVER}`).roomAlias
+        getRoomIdentifierFromUrl("", "", `#${ROOM_NAME}:${HOMESERVER}`)
+          .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
 
     it("and with server part", () => {
       expect(
-        getUrlParams(false, "", `/${ROOM_NAME}:${HOMESERVER}`, "").roomAlias
+        getRoomIdentifierFromUrl(`/${ROOM_NAME}:${HOMESERVER}`, "", "")
+          .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
   });
 
   describe("handles search params", () => {
     it("(roomId)", () => {
-      expect(getUrlParams(true, `?roomId=${ROOM_ID}`).roomId).toBe(ROOM_ID);
+      expect(
+        getRoomIdentifierFromUrl("", `?roomId=${ROOM_ID}`, "").roomId
+      ).toBe(ROOM_ID);
     });
   });
 
   it("ignores room alias", () => {
     expect(
-      getUrlParams(true, "", `/room/${ROOM_NAME}:${HOMESERVER}`).roomAlias
+      getRoomIdentifierFromUrl("", `/room/${ROOM_NAME}:${HOMESERVER}`, "")
+        .roomAlias
     ).toBeFalsy();
   });
 });