From ef1f5bf232c0b71c29e71cfa49f5e37683c4071f Mon Sep 17 00:00:00 2001
From: Kerry <kerrya@element.io>
Date: Fri, 9 Jun 2023 09:36:34 +1200
Subject: [PATCH] Fix: handle `baseUrl` with trailing slash in `fetch.getUrl`
 (#3455)

* tests

* tidy trailing slash in fetch.getUrl before forming url

* make sonar happy about Polynomial regular expression used on uncontrolled data
---
 spec/unit/http-api/fetch.spec.ts | 55 ++++++++++++++++++++++++++++++++
 src/http-api/fetch.ts            |  8 +++--
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/spec/unit/http-api/fetch.spec.ts b/spec/unit/http-api/fetch.spec.ts
index 9fc0642c5..77ac2e3a4 100644
--- a/spec/unit/http-api/fetch.spec.ts
+++ b/spec/unit/http-api/fetch.spec.ts
@@ -18,6 +18,7 @@ import { FetchHttpApi } from "../../../src/http-api/fetch";
 import { TypedEventEmitter } from "../../../src/models/typed-event-emitter";
 import { ClientPrefix, HttpApiEvent, HttpApiEventHandlerMap, IdentityPrefix, IHttpOpts, Method } from "../../../src";
 import { emitPromise } from "../../test-utils/test-utils";
+import { QueryDict } from "../../../src/utils";
 
 describe("FetchHttpApi", () => {
     const baseUrl = "http://baseUrl";
@@ -235,4 +236,58 @@ describe("FetchHttpApi", () => {
             expect(fetchFn.mock.calls[0][1].headers.Authorization).toBeUndefined();
         });
     });
+
+    describe("getUrl()", () => {
+        const localBaseUrl = "http://baseurl";
+        const baseUrlWithTrailingSlash = "http://baseurl/";
+        const makeApi = (thisBaseUrl = baseUrl): FetchHttpApi<any> => {
+            const fetchFn = jest.fn();
+            const emitter = new TypedEventEmitter<HttpApiEvent, HttpApiEventHandlerMap>();
+            return new FetchHttpApi(emitter, { baseUrl: thisBaseUrl, prefix, fetchFn });
+        };
+
+        type TestParams = {
+            path: string;
+            queryParams?: QueryDict;
+            prefix?: string;
+            baseUrl?: string;
+        };
+        type TestCase = [TestParams, string];
+        const queryParams: QueryDict = {
+            test1: 99,
+            test2: ["a", "b"],
+        };
+        const testPrefix = "/just/testing";
+        const testUrl = "http://justtesting.com";
+        const testUrlWithTrailingSlash = "http://justtesting.com/";
+
+        const testCases: TestCase[] = [
+            [{ path: "/terms" }, `${localBaseUrl}${prefix}/terms`],
+            [{ path: "/terms", queryParams }, `${localBaseUrl}${prefix}/terms?test1=99&test2=a&test2=b`],
+            [{ path: "/terms", prefix: testPrefix }, `${localBaseUrl}${testPrefix}/terms`],
+            [{ path: "/terms", baseUrl: testUrl }, `${testUrl}${prefix}/terms`],
+            [{ path: "/terms", baseUrl: testUrlWithTrailingSlash }, `${testUrl}${prefix}/terms`],
+            [
+                { path: "/terms", queryParams, prefix: testPrefix, baseUrl: testUrl },
+                `${testUrl}${testPrefix}/terms?test1=99&test2=a&test2=b`,
+            ],
+        ];
+        const runTests = (fetchBaseUrl: string) => {
+            it.each<TestCase>(testCases)(
+                "creates url with params %s",
+                ({ path, queryParams, prefix, baseUrl }, result) => {
+                    const api = makeApi(fetchBaseUrl);
+
+                    expect(api.getUrl(path, queryParams, prefix, baseUrl)).toEqual(new URL(result));
+                },
+            );
+        };
+
+        describe("when fetch.opts.baseUrl does not have a trailing slash", () => {
+            runTests(localBaseUrl);
+        });
+        describe("when fetch.opts.baseUrl does have a trailing slash", () => {
+            runTests(baseUrlWithTrailingSlash);
+        });
+    });
 });
diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts
index 5b0f0a1ba..4599b4b61 100644
--- a/src/http-api/fetch.ts
+++ b/src/http-api/fetch.ts
@@ -298,11 +298,15 @@ export class FetchHttpApi<O extends IHttpOpts> {
      * @param path - The HTTP path <b>after</b> the supplied prefix e.g. "/createRoom".
      * @param queryParams - A dict of query params (these will NOT be urlencoded).
      * @param prefix - The full prefix to use e.g. "/_matrix/client/v2_alpha", defaulting to this.opts.prefix.
-     * @param baseUrl - The baseUrl to use e.g. "https://matrix.org/", defaulting to this.opts.baseUrl.
+     * @param baseUrl - The baseUrl to use e.g. "https://matrix.org", defaulting to this.opts.baseUrl.
      * @returns URL
      */
     public getUrl(path: string, queryParams?: QueryDict, prefix?: string, baseUrl?: string): URL {
-        const url = new URL((baseUrl ?? this.opts.baseUrl) + (prefix ?? this.opts.prefix) + path);
+        const baseUrlWithFallback = baseUrl ?? this.opts.baseUrl;
+        const baseUrlWithoutTrailingSlash = baseUrlWithFallback.endsWith("/")
+            ? baseUrlWithFallback.slice(0, -1)
+            : baseUrlWithFallback;
+        const url = new URL(baseUrlWithoutTrailingSlash + (prefix ?? this.opts.prefix) + path);
         if (queryParams) {
             encodeParams(queryParams, url.searchParams);
         }