Skip to content

Commit

Permalink
Fix: handle baseUrl with trailing slash in fetch.getUrl (#3455)
Browse files Browse the repository at this point in the history
* tests

* tidy trailing slash in fetch.getUrl before forming url

* make sonar happy about Polynomial regular expression used on uncontrolled data

(cherry picked from commit ef1f5bf)
  • Loading branch information
Kerry authored and github-actions[bot] committed Jun 9, 2023
1 parent f41fa84 commit 13dccb3
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 2 deletions.
55 changes: 55 additions & 0 deletions spec/unit/http-api/fetch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
});
});
});
8 changes: 6 additions & 2 deletions src/http-api/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 13dccb3

Please sign in to comment.