1
0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-07-31 15:24:23 +03:00

Log query parameters on HTTP requests (#3591)

* Log query parameters on HTTP requests

Follow-up to https://github.com/matrix-org/matrix-js-sdk/pull/3485

* Only stringify once

See https://github.com/matrix-org/matrix-js-sdk/pull/3591#discussion_r1261300323
This commit is contained in:
Eric Eastwood
2023-07-13 08:07:01 -05:00
committed by GitHub
parent d92936fba5
commit 8ef2e848b9
2 changed files with 14 additions and 8 deletions

View File

@ -300,7 +300,7 @@ describe("FetchHttpApi", () => {
const fetchFn = jest.fn().mockReturnValue(deferred.promise); const fetchFn = jest.fn().mockReturnValue(deferred.promise);
jest.spyOn(logger, "debug").mockImplementation(() => {}); jest.spyOn(logger, "debug").mockImplementation(() => {});
const api = new FetchHttpApi(new TypedEventEmitter<any, any>(), { baseUrl, prefix, fetchFn }); const api = new FetchHttpApi(new TypedEventEmitter<any, any>(), { baseUrl, prefix, fetchFn });
const prom = api.requestOtherUrl(Method.Get, "https://server:8448/some/path#fragment?query=param"); const prom = api.requestOtherUrl(Method.Get, "https://server:8448/some/path?query=param#fragment");
jest.advanceTimersByTime(1234); jest.advanceTimersByTime(1234);
deferred.resolve({ ok: true, status: 200, text: () => Promise.resolve("RESPONSE") } as Response); deferred.resolve({ ok: true, status: 200, text: () => Promise.resolve("RESPONSE") } as Response);
await prom; await prom;
@ -310,12 +310,12 @@ describe("FetchHttpApi", () => {
expect(logger.debug).toHaveBeenCalledTimes(2); expect(logger.debug).toHaveBeenCalledTimes(2);
expect(mocked(logger.debug).mock.calls[0]).toMatchInlineSnapshot(` expect(mocked(logger.debug).mock.calls[0]).toMatchInlineSnapshot(`
[ [
"FetchHttpApi: --> GET https://server:8448/some/path", "FetchHttpApi: --> GET https://server:8448/some/path?query=xxx",
] ]
`); `);
expect(mocked(logger.debug).mock.calls[1]).toMatchInlineSnapshot(` expect(mocked(logger.debug).mock.calls[1]).toMatchInlineSnapshot(`
[ [
"FetchHttpApi: <-- GET https://server:8448/some/path [1234ms 200]", "FetchHttpApi: <-- GET https://server:8448/some/path?query=xxx [1234ms 200]",
] ]
`); `);
}); });

View File

@ -224,7 +224,7 @@ export class FetchHttpApi<O extends IHttpOpts> {
body?: Body, body?: Body,
opts: Pick<IRequestOpts, "headers" | "json" | "localTimeoutMs" | "keepAlive" | "abortSignal"> = {}, opts: Pick<IRequestOpts, "headers" | "json" | "localTimeoutMs" | "keepAlive" | "abortSignal"> = {},
): Promise<ResponseType<T, O>> { ): Promise<ResponseType<T, O>> {
const urlForLogs = this.clearUrlParamsForLogs(url); const urlForLogs = this.sanitizeUrlForLogs(url);
logger.debug(`FetchHttpApi: --> ${method} ${urlForLogs}`); logger.debug(`FetchHttpApi: --> ${method} ${urlForLogs}`);
const headers = Object.assign({}, opts.headers || {}); const headers = Object.assign({}, opts.headers || {});
@ -299,7 +299,7 @@ export class FetchHttpApi<O extends IHttpOpts> {
return res as ResponseType<T, O>; return res as ResponseType<T, O>;
} }
private clearUrlParamsForLogs(url: URL | string): string { private sanitizeUrlForLogs(url: URL | string): string {
try { try {
let asUrl: URL; let asUrl: URL;
if (typeof url === "string") { if (typeof url === "string") {
@ -307,9 +307,15 @@ export class FetchHttpApi<O extends IHttpOpts> {
} else { } else {
asUrl = url; asUrl = url;
} }
// get just the path to remove any potential url param that could have // Remove the values of any URL params that could contain potential secrets
// some potential secrets const sanitizedQs = new URLSearchParams();
return asUrl.origin + asUrl.pathname; for (const key of asUrl.searchParams.keys()) {
sanitizedQs.append(key, "xxx");
}
const sanitizedQsString = sanitizedQs.toString();
const sanitizedQsUrlPiece = sanitizedQsString ? `?${sanitizedQsString}` : "";
return asUrl.origin + asUrl.pathname + sanitizedQsUrlPiece;
} catch (error) { } catch (error) {
// defensive coding for malformed url // defensive coding for malformed url
return "??"; return "??";