From 12a9875c4670f6fa5460dd5b981c8cc47d87bfe8 Mon Sep 17 00:00:00 2001 From: rsb-tbg <69879226+rsb-tbg@users.noreply.github.com> Date: Fri, 30 May 2025 04:09:21 -0500 Subject: [PATCH] Include extraParams in all HTTP requests (#4860) * attaching queryParams from client config in getUrl Signed-off-by: rsb-tbg <69879226+rsb-tbg@users.noreply.github.com> * changed client queryParams to QueryDict for consistency and now merging both sets of params in getUrl if one or both exist Signed-off-by: rsb-tbg <69879226+rsb-tbg@users.noreply.github.com> * added tests Signed-off-by: rsb-tbg <69879226+rsb-tbg@users.noreply.github.com> --------- Signed-off-by: rsb-tbg <69879226+rsb-tbg@users.noreply.github.com> --- spec/integ/matrix-client-opts.spec.ts | 105 ++++++++++++++++++++++++++ spec/unit/http-api/fetch.spec.ts | 77 +++++++++++++++++++ src/client.ts | 2 +- src/http-api/fetch.ts | 7 +- src/http-api/interface.ts | 3 +- 5 files changed, 190 insertions(+), 4 deletions(-) diff --git a/spec/integ/matrix-client-opts.spec.ts b/spec/integ/matrix-client-opts.spec.ts index 6d8d19811..de45304f8 100644 --- a/spec/integ/matrix-client-opts.spec.ts +++ b/spec/integ/matrix-client-opts.spec.ts @@ -205,4 +205,109 @@ describe("MatrixClient opts", function () { expect(res.event_id).toEqual("foo"); }); }); + + describe("with opts.queryParams", function () { + let client: MatrixClient; + let httpBackend: HttpBackend; + const userId = "@rsb-tbg:localhost"; + + beforeEach(function () { + httpBackend = new HttpBackend(); + client = new MatrixClient({ + fetchFn: httpBackend.fetchFn as typeof globalThis.fetch, + store: new MemoryStore() as IStore, + baseUrl: baseUrl, + userId: userId, + accessToken: accessToken, + queryParams: { user_id: userId }, + }); + }); + + afterEach(function () { + client.stopClient(); + httpBackend.verifyNoOutstandingExpectation(); + return httpBackend.stop(); + }); + + it("should include queryParams in matrix server requests", async () => { + const eventId = "$test:event"; + httpBackend + .when("PUT", "/txn1") + .check((req) => { + expect(req.path).toContain(`user_id=${encodeURIComponent(userId)}`); + return true; + }) + .respond(200, { + event_id: eventId, + }); + + const [res] = await Promise.all([ + client.sendTextMessage("!foo:bar", "test message", "txn1"), + httpBackend.flush("/txn1", 1), + ]); + + expect(res.event_id).toEqual(eventId); + }); + + it("should include queryParams in sync requests", async () => { + httpBackend + .when("GET", "/versions") + .check((req) => { + expect(req.path).toContain(`user_id=${encodeURIComponent(userId)}`); + return true; + }) + .respond(200, {}); + + httpBackend + .when("GET", "/pushrules") + .check((req) => { + expect(req.path).toContain(`user_id=${encodeURIComponent(userId)}`); + return true; + }) + .respond(200, {}); + + httpBackend + .when("POST", "/filter") + .check((req) => { + expect(req.path).toContain(`user_id=${encodeURIComponent(userId)}`); + return true; + }) + .respond(200, { filter_id: "foo" }); + + httpBackend + .when("GET", "/sync") + .check((req) => { + expect(req.path).toContain(`user_id=${encodeURIComponent(userId)}`); + return true; + }) + .respond(200, syncData); + + client.startClient(); + await httpBackend.flush("/versions", 1); + await httpBackend.flush("/pushrules", 1); + await httpBackend.flush("/filter", 1); + await Promise.all([httpBackend.flush("/sync", 1), utils.syncPromise(client)]); + }); + + it("should merge queryParams with request-specific params", async () => { + const eventId = "$test:event"; + httpBackend + .when("PUT", "/txn1") + .check((req) => { + // Should contain both global queryParams and request-specific params + expect(req.path).toContain(`user_id=${encodeURIComponent(userId)}`); + return true; + }) + .respond(200, { + event_id: eventId, + }); + + const [res] = await Promise.all([ + client.sendTextMessage("!foo:bar", "test message", "txn1"), + httpBackend.flush("/txn1", 1), + ]); + + expect(res.event_id).toEqual(eventId); + }); + }); }); diff --git a/spec/unit/http-api/fetch.spec.ts b/spec/unit/http-api/fetch.spec.ts index 1f6355792..d7e0e19a9 100644 --- a/spec/unit/http-api/fetch.spec.ts +++ b/spec/unit/http-api/fetch.spec.ts @@ -521,6 +521,83 @@ describe("FetchHttpApi", () => { describe("when fetch.opts.baseUrl does have a trailing slash", () => { runTests(baseUrlWithTrailingSlash); }); + + describe("extraParams handling", () => { + const makeApiWithExtraParams = (extraParams: QueryDict): FetchHttpApi => { + const fetchFn = jest.fn(); + const emitter = new TypedEventEmitter(); + return new FetchHttpApi(emitter, { baseUrl: localBaseUrl, prefix, fetchFn, extraParams }); + }; + + const userId = "@rsb-tbg:localhost"; + const encodedUserId = encodeURIComponent(userId); + + it("should include extraParams in URL when no queryParams provided", () => { + const extraParams = { user_id: userId, version: "1.0" }; + const api = makeApiWithExtraParams(extraParams); + + const result = api.getUrl("/test"); + expect(result.toString()).toBe(`${localBaseUrl}${prefix}/test?user_id=${encodedUserId}&version=1.0`); + }); + + it("should merge extraParams with queryParams", () => { + const extraParams = { user_id: userId, version: "1.0" }; + const api = makeApiWithExtraParams(extraParams); + + const queryParams = { userId: "123", filter: "active" }; + const result = api.getUrl("/test", queryParams); + + expect(result.searchParams.get("user_id")!).toBe(userId); + expect(result.searchParams.get("version")!).toBe("1.0"); + expect(result.searchParams.get("userId")!).toBe("123"); + expect(result.searchParams.get("filter")!).toBe("active"); + }); + + it("should allow queryParams to override extraParams", () => { + const extraParams = { user_id: "@default:localhost", version: "1.0" }; + const api = makeApiWithExtraParams(extraParams); + + const queryParams = { user_id: "@override:localhost", userId: "123" }; + const result = api.getUrl("/test", queryParams); + + expect(result.searchParams.get("user_id")).toBe("@override:localhost"); + expect(result.searchParams.get("version")!).toBe("1.0"); + expect(result.searchParams.get("userId")!).toBe("123"); + }); + + it("should handle empty extraParams", () => { + const extraParams = {}; + const api = makeApiWithExtraParams(extraParams); + + const queryParams = { userId: "123" }; + const result = api.getUrl("/test", queryParams); + + expect(result.searchParams.get("userId")!).toBe("123"); + expect(result.searchParams.has("user_id")).toBe(false); + }); + + it("should work when extraParams is undefined", () => { + const fetchFn = jest.fn(); + const emitter = new TypedEventEmitter(); + const api = new FetchHttpApi(emitter, { baseUrl: localBaseUrl, prefix, fetchFn }); + + const queryParams = { userId: "123" }; + const result = api.getUrl("/test", queryParams); + + expect(result.searchParams.get("userId")!).toBe("123"); + expect(result.toString()).toBe(`${localBaseUrl}${prefix}/test?userId=123`); + }); + + it("should work when queryParams is undefined", () => { + const extraParams = { user_id: userId, version: "1.0" }; + const api = makeApiWithExtraParams(extraParams); + + const result = api.getUrl("/test"); + + expect(result.searchParams.get("user_id")!).toBe(userId); + expect(result.toString()).toBe(`${localBaseUrl}${prefix}/test?user_id=${encodedUserId}&version=1.0`); + }); + }); }); it("should not log query parameters", async () => { diff --git a/src/client.ts b/src/client.ts index 74e86ecab..4686fcd42 100644 --- a/src/client.ts +++ b/src/client.ts @@ -360,7 +360,7 @@ export interface ICreateClientOpts { * to all requests with this client. Useful for application services which require * `?user_id=`. */ - queryParams?: Record; + queryParams?: QueryDict; /** * Encryption key used for encrypting sensitive data (such as e2ee keys) in {@link ICreateClientOpts#cryptoStore}. diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index 6dc5c78de..e6728fbd9 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -379,9 +379,12 @@ export class FetchHttpApi { ? baseUrlWithFallback.slice(0, -1) : baseUrlWithFallback; const url = new URL(baseUrlWithoutTrailingSlash + (prefix ?? this.opts.prefix) + path); - if (queryParams) { - encodeParams(queryParams, url.searchParams); + // If there are any params, encode and append them to the URL. + if (this.opts.extraParams || queryParams) { + const mergedParams = { ...this.opts.extraParams, ...queryParams }; + encodeParams(mergedParams, url.searchParams); } + return url; } } diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index d5a01deb2..1b0df88b5 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -16,6 +16,7 @@ limitations under the License. import { type MatrixError } from "./errors.ts"; import { type Logger } from "../logger.ts"; +import { type QueryDict } from "../utils.ts"; export type Body = Record | BodyInit; @@ -52,7 +53,7 @@ export interface IHttpOpts { baseUrl: string; idBaseUrl?: string; prefix: string; - extraParams?: Record; + extraParams?: QueryDict; accessToken?: string; /**