From 5973c667266e2235e9c5901c76573b8fb0e02982 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 19 May 2023 16:33:19 +0100 Subject: [PATCH] Make sonar happier about our code & tests (#3388) --- spec/integ/crypto.spec.ts | 2 +- spec/olm-loader.ts | 2 +- spec/unit/crypto.spec.ts | 35 ++----------- spec/unit/crypto/secrets.spec.ts | 17 ++---- spec/unit/crypto/verification/sas.spec.ts | 8 +-- spec/unit/room.spec.ts | 8 +-- spec/unit/webrtc/call.spec.ts | 9 +--- spec/unit/webrtc/groupCall.spec.ts | 5 +- src/autodiscovery.ts | 6 +-- src/client.ts | 52 +++++++++---------- src/crypto/CrossSigning.ts | 4 +- src/crypto/DeviceList.ts | 2 +- src/crypto/store/memory-crypto-store.ts | 3 +- .../verification/request/InRoomChannel.ts | 4 +- src/embedded.ts | 3 +- 15 files changed, 52 insertions(+), 108 deletions(-) diff --git a/spec/integ/crypto.spec.ts b/spec/integ/crypto.spec.ts index 4e70bdd6e..951e853f9 100644 --- a/spec/integ/crypto.spec.ts +++ b/spec/integ/crypto.spec.ts @@ -1111,7 +1111,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string, } catch (e) { expect((e as any).name).toEqual("UnknownDeviceError"); expect([...(e as any).devices.keys()]).toEqual([aliceClient.getUserId()!]); - expect((e as any).devices.get(aliceClient.getUserId()!).has("DEVICE_ID")); + expect((e as any).devices.get(aliceClient.getUserId()!).has("DEVICE_ID")).toBeTruthy(); } // mark the device as known, and resend. diff --git a/spec/olm-loader.ts b/spec/olm-loader.ts index a139a4867..29817176b 100644 --- a/spec/olm-loader.ts +++ b/spec/olm-loader.ts @@ -23,5 +23,5 @@ try { global.Olm = require("@matrix-org/olm"); logger.log("loaded libolm"); } catch (e) { - logger.warn("unable to run crypto tests: libolm not available"); + logger.warn("unable to run crypto tests: libolm not available", e); } diff --git a/spec/unit/crypto.spec.ts b/spec/unit/crypto.spec.ts index e62bb2eac..b96c06686 100644 --- a/spec/unit/crypto.spec.ts +++ b/spec/unit/crypto.spec.ts @@ -381,12 +381,7 @@ describe("Crypto", function () { event.senderCurve25519Key = null; // @ts-ignore private properties event.claimedEd25519Key = null; - try { - await bobClient.crypto!.decryptEvent(event); - } catch (e) { - // we expect this to fail because we don't have the - // decryption keys yet - } + await expect(bobClient.crypto!.decryptEvent(event)).rejects.toBeTruthy(); }), ); @@ -617,12 +612,7 @@ describe("Crypto", function () { event.senderCurve25519Key = null; // @ts-ignore private properties event.claimedEd25519Key = null; - try { - await secondAliceClient.crypto!.decryptEvent(event); - } catch (e) { - // we expect this to fail because we don't have the - // decryption keys yet - } + await expect(secondAliceClient.crypto!.decryptEvent(event)).rejects.toBeTruthy(); }), ); @@ -725,12 +715,7 @@ describe("Crypto", function () { event.senderCurve25519Key = null; // @ts-ignore private properties event.claimedEd25519Key = null; - try { - await bobClient.crypto!.decryptEvent(event); - } catch (e) { - // we expect this to fail because we don't have the - // decryption keys yet - } + await expect(bobClient.crypto!.decryptEvent(event)).rejects.toBeTruthy(); }), ); @@ -805,12 +790,7 @@ describe("Crypto", function () { event.senderCurve25519Key = null; // @ts-ignore private properties event.claimedEd25519Key = null; - try { - await bobClient.crypto!.decryptEvent(event); - } catch (e) { - // we expect this to fail because we don't have the - // decryption keys yet - } + await expect(bobClient.crypto!.decryptEvent(event)).rejects.toBeTruthy(); }), ); @@ -897,12 +877,7 @@ describe("Crypto", function () { event.senderCurve25519Key = null; // @ts-ignore private properties event.claimedEd25519Key = null; - try { - await bobClient.crypto!.decryptEvent(event); - } catch (e) { - // we expect this to fail because we don't have the - // decryption keys yet - } + await expect(bobClient.crypto!.decryptEvent(event)).rejects.toBeTruthy(); }), ); diff --git a/spec/unit/crypto/secrets.spec.ts b/spec/unit/crypto/secrets.spec.ts index 91b8deb1c..3e95ed05e 100644 --- a/spec/unit/crypto/secrets.spec.ts +++ b/spec/unit/crypto/secrets.spec.ts @@ -148,22 +148,14 @@ describe("Secrets", function () { it("should throw if given a key that doesn't exist", async function () { const alice = await makeTestClient({ userId: "@alice:example.com", deviceId: "Osborne2" }); - try { - await alice.storeSecret("foo", "bar", ["this secret does not exist"]); - // should be able to use expect(...).toThrow() but mocha still fails - // the test even when it throws for reasons I have no inclination to debug - expect(true).toBeFalsy(); - } catch (e) {} + await expect(alice.storeSecret("foo", "bar", ["this secret does not exist"])).rejects.toBeTruthy(); alice.stopClient(); }); it("should refuse to encrypt with zero keys", async function () { const alice = await makeTestClient({ userId: "@alice:example.com", deviceId: "Osborne2" }); - try { - await alice.storeSecret("foo", "bar", []); - expect(true).toBeFalsy(); - } catch (e) {} + await expect(alice.storeSecret("foo", "bar", [])).rejects.toBeTruthy(); alice.stopClient(); }); @@ -214,10 +206,7 @@ describe("Secrets", function () { it("should refuse to encrypt if no keys given and no default key", async function () { const alice = await makeTestClient({ userId: "@alice:example.com", deviceId: "Osborne2" }); - try { - await alice.storeSecret("foo", "bar"); - expect(true).toBeFalsy(); - } catch (e) {} + await expect(alice.storeSecret("foo", "bar")).rejects.toBeTruthy(); alice.stopClient(); }); diff --git a/spec/unit/crypto/verification/sas.spec.ts b/spec/unit/crypto/verification/sas.spec.ts index 1ea2c3222..ba73ced6a 100644 --- a/spec/unit/crypto/verification/sas.spec.ts +++ b/spec/unit/crypto/verification/sas.spec.ts @@ -144,7 +144,7 @@ describe("SAS verification", function () { expect(e.sas).toEqual(aliceSasEvent.sas); e.confirm(); aliceSasEvent.confirm(); - } catch (error) { + } catch { e.mismatch(); aliceSasEvent.mismatch(); } @@ -169,7 +169,7 @@ describe("SAS verification", function () { expect(e.sas).toEqual(bobSasEvent.sas); e.confirm(); bobSasEvent.confirm(); - } catch (error) { + } catch { e.mismatch(); bobSasEvent.mismatch(); } @@ -519,7 +519,7 @@ describe("SAS verification", function () { expect(e.sas).toEqual(aliceSasEvent.sas); e.confirm(); aliceSasEvent.confirm(); - } catch (error) { + } catch { e.mismatch(); aliceSasEvent.mismatch(); } @@ -543,7 +543,7 @@ describe("SAS verification", function () { expect(e.sas).toEqual(bobSasEvent.sas); e.confirm(); bobSasEvent.confirm(); - } catch (error) { + } catch { e.mismatch(); bobSasEvent.mismatch(); } diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 8caca90be..2445ee4fd 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -1932,13 +1932,7 @@ describe("Room", function () { it("should allow retry on error", async function () { const client = createClientMock(new Error("server says no")); const room = new Room(roomId, client as any, null!, { lazyLoadMembers: true }); - let hasThrown = false; - try { - await room.loadMembersIfNeeded(); - } catch (err) { - hasThrown = true; - } - expect(hasThrown).toEqual(true); + await expect(room.loadMembersIfNeeded()).rejects.toBeTruthy(); client.members.mockReturnValue({ chunk: [memberEvent] }); await room.loadMembersIfNeeded(); diff --git a/spec/unit/webrtc/call.spec.ts b/spec/unit/webrtc/call.spec.ts index 965f41eae..5a3b38453 100644 --- a/spec/unit/webrtc/call.spec.ts +++ b/spec/unit/webrtc/call.spec.ts @@ -1054,14 +1054,7 @@ describe("Call", function () { mockSendEvent.mockReset(); - let caught = false; - try { - call.reject(); - } catch (e) { - caught = true; - } - - expect(caught).toEqual(true); + expect(() => call.reject()).toThrow(); expect(client.client.sendEvent).not.toHaveBeenCalled(); call.hangup(CallErrorCode.UserHangup, true); diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts index d20232367..876b658e2 100644 --- a/spec/unit/webrtc/groupCall.spec.ts +++ b/spec/unit/webrtc/groupCall.spec.ts @@ -186,10 +186,7 @@ describe("Group Call", function () { it("sets state to local call feed uninitialized when getUserMedia() fails", async () => { jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockRejectedValue("Error"); - try { - await groupCall.initLocalCallFeed(); - } catch (e) {} - + await expect(groupCall.initLocalCallFeed()).rejects.toBeTruthy(); expect(groupCall.state).toBe(GroupCallState.LocalCallFeedUninitialized); }); diff --git a/src/autodiscovery.ts b/src/autodiscovery.ts index f4a341596..5a7294b29 100644 --- a/src/autodiscovery.ts +++ b/src/autodiscovery.ts @@ -142,7 +142,7 @@ export class AutoDiscovery { }, }; - if (!wellknown || !wellknown["m.homeserver"]) { + if (!wellknown?.["m.homeserver"]) { logger.error("No m.homeserver key in config"); clientConfig["m.homeserver"].state = AutoDiscovery.FAIL_PROMPT; @@ -171,7 +171,7 @@ export class AutoDiscovery { // Step 3: Make sure the homeserver URL points to a homeserver. const hsVersions = await this.fetchWellKnownObject(`${hsUrl}/_matrix/client/versions`); - if (!hsVersions || !hsVersions.raw?.["versions"]) { + if (!hsVersions?.raw?.["versions"]) { logger.error("Invalid /versions response"); clientConfig["m.homeserver"].error = AutoDiscovery.ERROR_INVALID_HOMESERVER; @@ -345,7 +345,7 @@ export class AutoDiscovery { const response = await this.fetchWellKnownObject(`https://${domain}/.well-known/matrix/client`); if (!response) return {}; - return response.raw || {}; + return response.raw ?? {}; } /** diff --git a/src/client.ts b/src/client.ts index 820b36386..f34996686 100644 --- a/src/client.ts +++ b/src/client.ts @@ -36,7 +36,11 @@ import { StubStore } from "./store/stub"; import { CallEvent, CallEventHandlerMap, createNewMatrixCall, MatrixCall, supportsMatrixCall } from "./webrtc/call"; import { Filter, IFilterDefinition, IRoomEventFilter } from "./filter"; import { CallEventHandlerEvent, CallEventHandler, CallEventHandlerEventHandlerMap } from "./webrtc/callEventHandler"; -import { GroupCallEventHandlerEvent, GroupCallEventHandlerEventHandlerMap } from "./webrtc/groupCallEventHandler"; +import { + GroupCallEventHandler, + GroupCallEventHandlerEvent, + GroupCallEventHandlerEventHandlerMap, +} from "./webrtc/groupCallEventHandler"; import * as utils from "./utils"; import { replaceParam, QueryDict, sleep, noUnsafeEventProps, safeSet } from "./utils"; import { Direction, EventTimeline } from "./models/event-timeline"; @@ -180,7 +184,6 @@ import { IThreepid } from "./@types/threepids"; import { CryptoStore, OutgoingRoomKeyRequest } from "./crypto/store/base"; import { GroupCall, IGroupCallDataChannelOptions, GroupCallIntent, GroupCallType } from "./webrtc/groupCall"; import { MediaHandler } from "./webrtc/mediaHandler"; -import { GroupCallEventHandler } from "./webrtc/groupCallEventHandler"; import { LoginTokenPostResponse, ILoginFlowsResponse, IRefreshTokenResponse, SSOAction } from "./@types/auth"; import { TypedEventEmitter } from "./models/typed-event-emitter"; import { MAIN_ROOM_TIMELINE, ReceiptType } from "./@types/read_receipts"; @@ -4087,27 +4090,23 @@ export class MatrixClient extends TypedEventEmitter(Method.Post, path, queryString, data); - - const roomId = res.room_id; - const syncApi = new SyncApi(this, this.clientOpts, this.buildSyncApiOptions()); - const room = syncApi.createRoom(roomId); - if (opts.syncRoom) { - // v2 will do this for us - // return syncApi.syncRoom(room); - } - return room; - } catch (e) { - throw e; // rethrow for reject + const data: IJoinRequestBody = {}; + const signedInviteObj = await signPromise; + if (signedInviteObj) { + data.third_party_signed = signedInviteObj; } + + const path = utils.encodeUri("/join/$roomid", { $roomid: roomIdOrAlias }); + const res = await this.http.authedRequest<{ room_id: string }>(Method.Post, path, queryString, data); + + const roomId = res.room_id; + const syncApi = new SyncApi(this, this.clientOpts, this.buildSyncApiOptions()); + const syncRoom = syncApi.createRoom(roomId); + if (opts.syncRoom) { + // v2 will do this for us + // return syncApi.syncRoom(room); + } + return syncRoom; } /** @@ -4689,7 +4688,7 @@ export class MatrixClient extends TypedEventEmitter { const room = this.getRoom(roomId); - if (room && room.hasPendingEvent(rmEventId)) { + if (room?.hasPendingEvent(rmEventId)) { throw new Error(`Cannot set read marker to a pending event (${rmEventId})`); } @@ -5058,9 +5057,8 @@ export class MatrixClient extends TypedEventEmitter( diff --git a/src/crypto/CrossSigning.ts b/src/crypto/CrossSigning.ts index a77682e4c..e71967cb2 100644 --- a/src/crypto/CrossSigning.ts +++ b/src/crypto/CrossSigning.ts @@ -688,7 +688,7 @@ export function createCryptoStoreCacheCallbacks(store: CryptoStore, olmDevice: O _expectedPublicKey: string, ): Promise { const key = await new Promise((resolve) => { - return store.doTxn("readonly", [IndexedDBCryptoStore.STORE_ACCOUNT], (txn) => { + store.doTxn("readonly", [IndexedDBCryptoStore.STORE_ACCOUNT], (txn) => { store.getSecretStorePrivateKey(txn, resolve, type); }); }); @@ -790,7 +790,7 @@ export async function requestKeysDuringVerification( })(); // We call getCrossSigningKey() for its side-effects - return Promise.race([ + Promise.race([ Promise.all([ crossSigning.getCrossSigningKey("master"), crossSigning.getCrossSigningKey("self_signing"), diff --git a/src/crypto/DeviceList.ts b/src/crypto/DeviceList.ts index a1ff0ebf1..8ad383189 100644 --- a/src/crypto/DeviceList.ts +++ b/src/crypto/DeviceList.ts @@ -116,7 +116,7 @@ export class DeviceList extends TypedEventEmitter { await this.cryptoStore.doTxn("readonly", [IndexedDBCryptoStore.STORE_DEVICE_DATA], (txn) => { this.cryptoStore.getEndToEndDeviceData(txn, (deviceData) => { - this.hasFetched = Boolean(deviceData && deviceData.devices); + this.hasFetched = Boolean(deviceData?.devices); this.devices = deviceData ? deviceData.devices : {}; this.crossSigningInfo = deviceData ? deviceData.crossSigningInfo || {} : {}; this.deviceTrackingStatus = deviceData ? deviceData.trackingStatus : {}; diff --git a/src/crypto/store/memory-crypto-store.ts b/src/crypto/store/memory-crypto-store.ts index a78574850..3e264a429 100644 --- a/src/crypto/store/memory-crypto-store.ts +++ b/src/crypto/store/memory-crypto-store.ts @@ -15,7 +15,7 @@ limitations under the License. */ import { logger } from "../../logger"; -import { deepCompare, promiseTry } from "../../utils"; +import { safeSet, deepCompare, promiseTry } from "../../utils"; import { CryptoStore, IDeviceData, @@ -33,7 +33,6 @@ import { ICrossSigningKey } from "../../client"; import { IOlmDevice } from "../algorithms/megolm"; import { IRoomEncryption } from "../RoomList"; import { InboundGroupSessionData } from "../OlmDevice"; -import { safeSet } from "../../utils"; /** * Internal module. in-memory storage for e2e. diff --git a/src/crypto/verification/request/InRoomChannel.ts b/src/crypto/verification/request/InRoomChannel.ts index ae8ced5ff..d66032bf8 100644 --- a/src/crypto/verification/request/InRoomChannel.ts +++ b/src/crypto/verification/request/InRoomChannel.ts @@ -125,7 +125,7 @@ export class InRoomChannel implements IVerificationChannel { // part of a verification request, so be noisy when rejecting something if (type === REQUEST_TYPE) { if (!content || typeof content.to !== "string" || !content.to.length) { - logger.log("InRoomChannel: validateEvent: " + "no valid to " + (content && content.to)); + logger.log("InRoomChannel: validateEvent: " + "no valid to " + content.to); return false; } @@ -134,7 +134,7 @@ export class InRoomChannel implements IVerificationChannel { logger.log( "InRoomChannel: validateEvent: " + `not directed to or sent by me: ${event.getSender()}` + - `, ${content && content.to}`, + `, ${content.to}`, ); return false; } diff --git a/src/embedded.ts b/src/embedded.ts index a08b79a2f..c2c30105a 100644 --- a/src/embedded.ts +++ b/src/embedded.ts @@ -25,14 +25,13 @@ import { ISendEventFromWidgetResponseData, } from "matrix-widget-api"; -import { IEvent, IContent, EventStatus } from "./models/event"; +import { MatrixEvent, IEvent, IContent, EventStatus } from "./models/event"; import { ISendEventResponse } from "./@types/requests"; import { EventType } from "./@types/event"; import { logger } from "./logger"; import { MatrixClient, ClientEvent, IMatrixClientCreateOpts, IStartClientOpts, SendToDeviceContentMap } from "./client"; import { SyncApi, SyncState } from "./sync"; import { SlidingSyncSdk } from "./sliding-sync-sdk"; -import { MatrixEvent } from "./models/event"; import { User } from "./models/user"; import { Room } from "./models/room"; import { ToDeviceBatch, ToDevicePayload } from "./models/ToDeviceMessage";