From 0086a34bc439f2113be1b359b3e9a46ef8c89dd8 Mon Sep 17 00:00:00 2001 From: Tilo Spannagel Date: Fri, 6 Mar 2020 16:16:35 +0100 Subject: [PATCH 01/11] Add BridgedClientStatus enum Signed-off-by: Tilo Spannagel --- changelog.d/976.bugfix | 1 + src/DebugApi.ts | 12 +-- src/bridge/AdminRoomHandler.ts | 6 +- src/bridge/IrcBridge.ts | 4 +- src/bridge/MatrixHandler.ts | 6 +- src/bridge/PublicitySyncer.ts | 5 +- src/bridge/RoomAccessSyncer.ts | 5 +- src/irc/BridgedClient.ts | 140 ++++++++++++++++++++------------ src/irc/ClientPool.ts | 6 +- src/irc/IrcEventBroker.ts | 8 +- src/provisioning/Provisioner.ts | 6 +- 11 files changed, 121 insertions(+), 78 deletions(-) create mode 100644 changelog.d/976.bugfix diff --git a/changelog.d/976.bugfix b/changelog.d/976.bugfix new file mode 100644 index 000000000..19dfa064a --- /dev/null +++ b/changelog.d/976.bugfix @@ -0,0 +1 @@ +Make sure a killed BridgedClient is dead, even if connect was never called diff --git a/src/DebugApi.ts b/src/DebugApi.ts index 0d04104c4..4e15f4058 100644 --- a/src/DebugApi.ts +++ b/src/DebugApi.ts @@ -24,7 +24,7 @@ import { inspect } from "util"; import { DataStore } from "./datastore/DataStore"; import { ClientPool } from "./irc/ClientPool"; import { getLogger } from "./logging"; -import { BridgedClient } from "./irc/BridgedClient"; +import { BridgedClient, BridgedClientStatus } from "./irc/BridgedClient"; import { IrcBridge } from "./bridge/IrcBridge"; import { ProvisionRequest } from "./provisioning/ProvisionRequest"; import { getBridgeVersion } from "./util/PackageInfo"; @@ -249,12 +249,12 @@ export class DebugApi { "User " + user + " does not have a client on " + server.domain + "\n" ); } - const connection = client.unsafeClient && client.unsafeClient.conn; - if (!client.unsafeClient || !connection) { + if (client.state.status != BridgedClientStatus.CONNECTED || !client.state.client.conn) { return Bluebird.resolve( "There is no underlying client instance.\n" ); } + const connection = client.state.client.conn; // store all received response strings const buffer: string[] = []; @@ -263,7 +263,7 @@ export class DebugApi { buffer.push(JSON.stringify(msg)); } - client.unsafeClient.on("raw", listener); + client.state.client.on("raw", listener); // turn rn to n so if there are any new lines they are all n. body = body.replace("\r\n", "\n"); body.split("\n").forEach((c: string) => { @@ -275,8 +275,8 @@ export class DebugApi { // wait 3s to pool responses return Bluebird.delay(3000).then(function() { // unhook listener to avoid leaking - if (client.unsafeClient) { - client.unsafeClient.removeListener("raw", listener); + if (client.state.status == BridgedClientStatus.CONNECTED) { + client.state.client.removeListener("raw", listener); } return buffer.join("\n") + "\n"; }); diff --git a/src/bridge/AdminRoomHandler.ts b/src/bridge/AdminRoomHandler.ts index cde275eff..9480e1df6 100644 --- a/src/bridge/AdminRoomHandler.ts +++ b/src/bridge/AdminRoomHandler.ts @@ -19,7 +19,7 @@ import { MatrixRoom, MatrixUser } from "matrix-appservice-bridge"; import { IrcBridge } from "./IrcBridge"; import { MatrixAction } from "../models/MatrixAction"; import { IrcServer } from "../irc/IrcServer"; -import { BridgedClient } from "../irc/BridgedClient"; +import { BridgedClient, BridgedClientStatus } from "../irc/BridgedClient"; import { IrcClientConfig } from "../models/IrcClientConfig"; import { MatrixHandler } from "./MatrixHandler"; import logging from "../logging"; @@ -335,11 +335,11 @@ export class AdminRoomHandler { server, sender ); - if (!bridgedClient.unsafeClient) { + if (bridgedClient.state.status != BridgedClientStatus.CONNECTED) { throw new Error('Possibly disconnected'); } - bridgedClient.unsafeClient.send(...sendArgs); + bridgedClient.state.client.send(...sendArgs); } catch (err) { const notice = new MatrixAction("notice", `${err}\n` ); diff --git a/src/bridge/IrcBridge.ts b/src/bridge/IrcBridge.ts index bc0bb412c..9aa086eb3 100644 --- a/src/bridge/IrcBridge.ts +++ b/src/bridge/IrcBridge.ts @@ -6,7 +6,7 @@ import { MatrixHandler } from "./MatrixHandler"; import { MemberListSyncer } from "./MemberListSyncer"; import { IrcServer } from "../irc/IrcServer"; import { ClientPool } from "../irc/ClientPool"; -import { BridgedClient } from "../irc/BridgedClient"; +import { BridgedClient, BridgedClientStatus } from "../irc/BridgedClient"; import { IrcUser } from "../models/IrcUser"; import { IrcRoom } from "../models/IrcRoom"; import { BridgeRequest, BridgeRequestErr } from "../models/BridgeRequest"; @@ -958,7 +958,7 @@ export class IrcBridge { public sendIrcAction(ircRoom: IrcRoom, bridgedClient: BridgedClient, action: IrcAction) { log.info( "Sending IRC message in %s as %s (connected=%s)", - ircRoom.channel, bridgedClient.nick, Boolean(bridgedClient.unsafeClient) + ircRoom.channel, bridgedClient.nick, Boolean(bridgedClient.state.status == BridgedClientStatus.CONNECTED) ); return bridgedClient.sendAction(ircRoom, action); } diff --git a/src/bridge/MatrixHandler.ts b/src/bridge/MatrixHandler.ts index 6beee4111..30fbe72fc 100644 --- a/src/bridge/MatrixHandler.ts +++ b/src/bridge/MatrixHandler.ts @@ -5,7 +5,7 @@ import { IrcUser } from "../models/IrcUser"; import { MatrixAction, MatrixMessageEvent } from "../models/MatrixAction"; import { IrcRoom } from "../models/IrcRoom"; import logging from "../logging"; -import { BridgedClient } from "../irc/BridgedClient"; +import { BridgedClient, BridgedClientStatus } from "../irc/BridgedClient"; import { IrcServer } from "../irc/IrcServer"; import { IrcAction } from "../models/IrcAction"; import { toIrcLowerCase } from "../irc/formatting"; @@ -908,13 +908,13 @@ export class MatrixHandler { } // Check for the existance of the getSplitMessages method. - if (!(ircClient.unsafeClient && ircClient.unsafeClient.getSplitMessages)) { + if (!(ircClient.state.status == BridgedClientStatus.CONNECTED && ircClient.state.client.getSplitMessages)) { await this.ircBridge.sendIrcAction(ircRoom, ircClient, ircAction); return; } // Generate an array of individual messages that would be sent - const potentialMessages = ircClient.unsafeClient.getSplitMessages(ircRoom.channel, text); + const potentialMessages = ircClient.state.client.getSplitMessages(ircRoom.channel, text); const lineLimit = ircRoom.server.getLineLimit(); if (potentialMessages.length <= lineLimit) { diff --git a/src/bridge/PublicitySyncer.ts b/src/bridge/PublicitySyncer.ts index aaa592f03..ecb3e75ce 100644 --- a/src/bridge/PublicitySyncer.ts +++ b/src/bridge/PublicitySyncer.ts @@ -1,6 +1,7 @@ import logger from "../logging"; import { IrcBridge } from "./IrcBridge"; import { IrcServer } from "../irc/IrcServer"; +import { BridgedClientStatus } from "../irc/BridgedClient"; const log = logger("PublicitySyncer"); @@ -49,11 +50,11 @@ export class PublicitySyncer { public initModeForChannel(server: IrcServer, chan: string) { return this.ircBridge.getBotClient(server).then( (client) => { - if (!client.unsafeClient) { + if (client.state.status != BridgedClientStatus.CONNECTED) { throw Error("Can't request modes, bot client not connected") } log.info(`Bot requesting mode for ${chan} on ${server.domain}`); - client.unsafeClient.mode(chan); + client.state.client.mode(chan); }, (err) => { log.error(`Could not request mode of ${chan} (${err.message})`); diff --git a/src/bridge/RoomAccessSyncer.ts b/src/bridge/RoomAccessSyncer.ts index 3feb1f02d..70d95fbe7 100644 --- a/src/bridge/RoomAccessSyncer.ts +++ b/src/bridge/RoomAccessSyncer.ts @@ -3,6 +3,7 @@ import { IrcBridge } from "./IrcBridge"; import { BridgeRequest } from "../models/BridgeRequest"; import { IrcServer } from "../irc/IrcServer"; import { MatrixRoom } from "matrix-appservice-bridge"; +import { BridgedClientStatus } from "../irc/BridgedClient"; const log = getLogger("RoomAccessSyncer"); const MODES_TO_WATCH = [ @@ -114,11 +115,11 @@ export class RoomAccessSyncer { let userId = null; if (nick !== null && bridgedClient) { userId = bridgedClient.userId; - if (!bridgedClient.unsafeClient) { + if (bridgedClient.state.status != BridgedClientStatus.CONNECTED) { req.log.info(`Bridged client for ${nick} has no IRC client.`); return; } - const client = bridgedClient.unsafeClient; + const client = bridgedClient.state.client; const chanData = client.chanData(channel); if (!(chanData && chanData.users)) { req.log.error(`No channel data for ${channel}`); diff --git a/src/irc/BridgedClient.ts b/src/irc/BridgedClient.ts index 710454137..6d665aa6f 100644 --- a/src/irc/BridgedClient.ts +++ b/src/irc/BridgedClient.ts @@ -58,16 +58,34 @@ export interface BridgedClientLogger { export const illegalCharactersRegex = /[^A-Za-z0-9\]\[\^\\\{\}\-`_\|]/g; +export enum BridgedClientStatus { + CREATED, + CONNECTING, + CONNECTED, + DEAD, + KILLED, +} + +interface NotConnected { + status: BridgedClientStatus.CREATED | BridgedClientStatus.CONNECTING | + BridgedClientStatus.DEAD | BridgedClientStatus.KILLED; +} + +interface Connected { + status: BridgedClientStatus.CONNECTED; + client: Client; + inst: ConnectionInstance; +} + +type State = Connected | NotConnected + export class BridgedClient extends EventEmitter { public readonly userId: string|null; public readonly displayName: string|null; private _nick: string; public readonly id: string; private readonly password?: string; - private _unsafeClient: Client|null = null; private lastActionTs: number; - private inst: ConnectionInstance|null = null; - private instCreationFailed = false; private _explicitDisconnect = false; private _disconnectReason: string|null = null; private _chanList: Set = new Set(); @@ -76,6 +94,9 @@ export class BridgedClient extends EventEmitter { private cachedOperatorNicksInfo: {[channel: string]: GetNicksResponseOperators} = {}; private idleTimeout: NodeJS.Timer|null = null; private whoisPendingNicks: Set = new Set(); + private _state: State = { + status: BridgedClientStatus.CREATED + }; /** * Create a new bridged IRC client. * @constructor @@ -150,8 +171,8 @@ export class BridgedClient extends EventEmitter { return Array.from(this._chanList); } - public get unsafeClient() { - return this._unsafeClient; + public get state() { + return this._state; } public get nick(): string { @@ -164,21 +185,19 @@ export class BridgedClient extends EventEmitter { } public kill(reason?: string) { - // Nullify so that no further commands can be issued - // via unsafeClient, which should be null checked - // anyway as it is not instantiated until a connection - // has occurred. - this._unsafeClient = null; + const state = this.state; + // so that no further commands can be issued + this._state = { + status: BridgedClientStatus.KILLED + } + // kill connection instance log.info('Killing client ', this.nick); - return this.disconnect("killed", reason); + return this.disconnectWithState(state, "killed", reason); } public isDead() { - if (this.instCreationFailed || (this.inst && this.inst.dead)) { - return true; - } - return false; + return this.state.status == BridgedClientStatus.DEAD || this.state.status == BridgedClientStatus.KILLED; } public toString() { @@ -192,6 +211,10 @@ export class BridgedClient extends EventEmitter { public async connect(): Promise { let identResolver: (() => void) | undefined; + this._state = { + status: BridgedClientStatus.CONNECTING + } + try { const nameInfo = await this.identGenerator.getIrcNames( this.clientConfig, this.matrixUser @@ -228,8 +251,11 @@ export class BridgedClient extends EventEmitter { this.onConnectionCreated(inst, nameInfo, identResolver!); }); - this.inst = connInst; - this._unsafeClient = connInst.client; + this._state = { + status: BridgedClientStatus.CONNECTED, + inst: connInst, + client: connInst.client, + } this.emit("client-connected", this); // we may have been assigned a different nick, so update it from source this._nick = connInst.client.nick; @@ -280,7 +306,9 @@ export class BridgedClient extends EventEmitter { } catch (err) { this.log.debug("Failed to connect."); - this.instCreationFailed = true; + this._state = { + status: BridgedClientStatus.DEAD + } if (identResolver) { identResolver(); } @@ -303,11 +331,15 @@ export class BridgedClient extends EventEmitter { } public disconnect(reason: InstanceDisconnectReason, textReason?: string, explicit = true) { + return this.disconnectWithState(this.state, reason, textReason, explicit); + } + + private disconnectWithState(state: State, reason: InstanceDisconnectReason, textReason?: string, explicit = true) { this._explicitDisconnect = explicit; - if (!this.inst || this.inst.dead) { + if (state.status != BridgedClientStatus.CONNECTED) { return Promise.resolve(); } - return this.inst.disconnect(reason, textReason); + return state.inst.disconnect(reason, textReason); } /** @@ -348,10 +380,11 @@ export class BridgedClient extends EventEmitter { } private async sendNickCommand(nick: string): Promise { - if (!this.unsafeClient) { + if (this.state.status != BridgedClientStatus.CONNECTED) { throw Error("You are not connected to the network."); } - const client = this.unsafeClient; + const client = this.state.client; + return new Promise((resolve, reject) => { // These are nullified to prevent the linter from thinking these should be consts. let nickListener: ((old: string, n: string) => void) | null = null; @@ -401,7 +434,7 @@ export class BridgedClient extends EventEmitter { } public leaveChannel(channel: string, reason = "User left") { - if (!this.inst || this.inst.dead || !this.unsafeClient) { + if (this.state.status != BridgedClientStatus.CONNECTED) { return Promise.resolve(); // we were never connected to the network. } if (channel.indexOf("#") !== 0) { @@ -413,7 +446,7 @@ export class BridgedClient extends EventEmitter { const defer = promiseutil.defer(); this.removeChannel(channel); this.log.debug("Leaving channel %s", channel); - this.unsafeClient.part(channel, reason, () => { + this.state.client.part(channel, reason, () => { this.log.debug("Left channel %s", channel); defer.resolve(); }); @@ -427,10 +460,10 @@ export class BridgedClient extends EventEmitter { public kick(nick: string, channel: string, reason: string) { reason = reason || "User kicked"; - if (!this.inst || this.inst.dead || !this.unsafeClient) { + if (this.state.status != BridgedClientStatus.CONNECTED) { return Promise.resolve(); // we were never connected to the network. } - if (Object.keys(this.unsafeClient.chans).indexOf(channel) === -1) { + if (Object.keys(this.state.client.chans).indexOf(channel) === -1) { // we were never joined to it. We need to be joined to it to kick people. return Promise.resolve(); } @@ -438,7 +471,7 @@ export class BridgedClient extends EventEmitter { return Promise.resolve(); // PM room } - const c = this.unsafeClient; + const c = this.state.client; return new Promise((resolve) => { this.log.debug("Kicking %s from channel %s", nick, channel); @@ -476,10 +509,10 @@ export class BridgedClient extends EventEmitter { * @param {string} nick : The nick to call /whois on */ public async whois(nick: string): Promise<{ server: IrcServer; nick: string; msg: string}|null> { - if (!this.unsafeClient) { + if (this.state.status != BridgedClientStatus.CONNECTED) { throw Error("unsafeClient not ready yet"); } - const client = this.unsafeClient; + const client = this.state.client; let timeout: NodeJS.Timeout|null = null; let errorHandler!: (msg: IrcMessage) => void; try { @@ -585,11 +618,10 @@ export class BridgedClient extends EventEmitter { if (prefix === "@") { return true; } - const cli = this.unsafeClient; - if (!cli) { + if (this.state.status != BridgedClientStatus.CONNECTED) { throw new Error("Missing client"); } - if (cli.isUserPrefixMorePowerfulThan(prefix, "@")) { + if (this.state.client.isUserPrefixMorePowerfulThan(prefix, "@")) { return true; } } @@ -618,11 +650,11 @@ export class BridgedClient extends EventEmitter { */ public getNicks(channel: string): Bluebird { return new Bluebird((resolve, reject) => { - if (!this.unsafeClient) { + if (this.state.status != BridgedClientStatus.CONNECTED) { reject(Error("unsafeClient not ready yet")); return; } - this.unsafeClient.names(channel, (channelName: string, names: {[nick: string]: string}) => { + this.state.client.names(channel, (channelName: string, names: {[nick: string]: string}) => { // names maps nicks to chan op status, where '@' indicates chan op // names = {'nick1' : '', 'nick2' : '@', ...} resolve({ @@ -679,12 +711,12 @@ export class BridgedClient extends EventEmitter { n = "M" + n; } - if (this.unsafeClient) { + if (this.state.status == BridgedClientStatus.CONNECTED) { // nicks can't be too long let maxNickLen = 9; // RFC 1459 default - if (this.unsafeClient.supported && - typeof this.unsafeClient.supported.nicklength == "number") { - maxNickLen = this.unsafeClient.supported.nicklength; + if (this.state.client.supported && + typeof this.state.client.supported.nicklength == "number") { + maxNickLen = this.state.client.supported.nicklength; } if (n.length > maxNickLen) { if (throwOnInvalid) { @@ -762,6 +794,14 @@ export class BridgedClient extends EventEmitter { // If we've been banned, this is intentional. this._explicitDisconnect = true; } + + // make sure a killed connection stays killed + if (this.state.status != BridgedClientStatus.KILLED) { + this._state = { + status: BridgedClientStatus.DEAD, + } + } + this.emit("client-disconnected", this); this.eventBroker.sendMetadata(this, "Your connection to the IRC network '" + this.server.domain + @@ -777,13 +817,13 @@ export class BridgedClient extends EventEmitter { } private async setTopic(room: IrcRoom, topic: string): Promise { - if (!this.unsafeClient) { + if (this.state.status != BridgedClientStatus.CONNECTED) { throw Error("unsafeClient not ready yet"); } // join the room if we haven't already await this.joinChannel(room.channel) this.log.info("Setting topic to %s in channel %s", topic, room.channel); - return this.unsafeClient.send("TOPIC", room.channel, topic); + return this.state.client.send("TOPIC", room.channel, topic); } private async sendMessage(room: IrcRoom, msgType: string, text: string, expiryTs: number) { @@ -800,18 +840,18 @@ export class BridgedClient extends EventEmitter { return; } - if (!this.unsafeClient) { + if (this.state.status != BridgedClientStatus.CONNECTED) { return; } if (msgType == "action") { - await this.unsafeClient.action(room.channel, text); + await this.state.client.action(room.channel, text); } else if (msgType == "notice") { - await this.unsafeClient.notice(room.channel, text); + await this.state.client.notice(room.channel, text); } else if (msgType == "message") { - await this.unsafeClient.say(room.channel, text); + await this.state.client.say(room.channel, text); } defer.resolve(); } @@ -823,7 +863,7 @@ export class BridgedClient extends EventEmitter { } public joinChannel(channel: string, key?: string, attemptCount = 1): Bluebird { - if (!this.unsafeClient) { + if (this.state.status != BridgedClientStatus.CONNECTED) { // we may be trying to join before we've connected, so check and wait if (this.connectDefer && this.connectDefer.promise.isPending()) { return this.connectDefer.promise.then(() => { @@ -832,7 +872,7 @@ export class BridgedClient extends EventEmitter { } return Bluebird.reject(new Error("No client")); } - if (Object.keys(this.unsafeClient.chans).indexOf(channel) !== -1) { + if (Object.keys(this.state.client.chans).indexOf(channel) !== -1) { return Bluebird.resolve(new IrcRoom(this.server, channel)); } if (channel.indexOf("#") !== 0) { @@ -845,7 +885,7 @@ export class BridgedClient extends EventEmitter { const defer = promiseutil.defer() as promiseutil.Defer; this.log.debug("Joining channel %s", channel); this.addChannel(channel); - const client = this.unsafeClient; + const client = this.state.client; // listen for failures to join a channel (e.g. +i, +k) const failFn = (err: IrcMessage) => { if (!err || !err.args) { return; } @@ -869,7 +909,7 @@ export class BridgedClient extends EventEmitter { // add a timeout to try joining again setTimeout(() => { - if (!this.unsafeClient) { + if (this.state.status != BridgedClientStatus.CONNECTED) { log.error( `Could not try to join: no client for ${this.nick}, channel = ${channel}` ); @@ -878,7 +918,7 @@ export class BridgedClient extends EventEmitter { // promise isn't resolved yet and we still want to join this channel if (defer.promise.isPending() && this._chanList.has(channel)) { // we may have joined but didn't get the callback so check the client - if (Object.keys(this.unsafeClient.chans).indexOf(channel) !== -1) { + if (Object.keys(this.state.client.chans).indexOf(channel) !== -1) { // we're joined this.log.debug("Timed out joining %s - didn't get callback but " + "are now joined. Resolving.", channel); @@ -908,7 +948,7 @@ export class BridgedClient extends EventEmitter { } // send the JOIN with a key if it was specified. - this.unsafeClient.join(channel + (key ? " " + key : ""), () => { + this.state.client.join(channel + (key ? " " + key : ""), () => { this.log.debug("Joined channel %s", channel); client.removeListener("error", failFn); const room = new IrcRoom(this.server, channel); diff --git a/src/irc/ClientPool.ts b/src/irc/ClientPool.ts index 33f55deda..60197dced 100644 --- a/src/irc/ClientPool.ts +++ b/src/irc/ClientPool.ts @@ -21,7 +21,7 @@ import { BridgeRequest } from "../models/BridgeRequest"; import { IrcClientConfig } from "../models/IrcClientConfig"; import { IrcServer } from "../irc/IrcServer"; import { PrometheusMetrics, MatrixUser, MatrixRoom } from "matrix-appservice-bridge"; -import { BridgedClient } from "./BridgedClient"; +import { BridgedClient, BridgedClientStatus } from "./BridgedClient"; import { IrcBridge } from "../bridge/IrcBridge"; import { IdentGenerator } from "./IdentGenerator"; import { Ipv6Generator } from "./Ipv6Generator"; @@ -514,10 +514,10 @@ export class ClientPool { private onClientConnected(bridgedClient: BridgedClient): void { const server = bridgedClient.server; const oldNick = bridgedClient.nick; - if (!bridgedClient.unsafeClient) { + if (bridgedClient.state.status != BridgedClientStatus.CONNECTED) { return; } - const actualNick = bridgedClient.unsafeClient.nick; + const actualNick = bridgedClient.state.client.nick; // remove the pending nick we had set for this user this.virtualClients[server.domain].pending.delete(oldNick); diff --git a/src/irc/IrcEventBroker.ts b/src/irc/IrcEventBroker.ts index 288c0eb26..59df64f45 100644 --- a/src/irc/IrcEventBroker.ts +++ b/src/irc/IrcEventBroker.ts @@ -88,7 +88,7 @@ import { ProcessedDict } from "../util/ProcessedDict"; import { getLogger } from "../logging"; import { Bridge } from "matrix-appservice-bridge"; import { ClientPool } from "./ClientPool"; -import { BridgedClient } from "./BridgedClient"; +import { BridgedClient, BridgedClientStatus } from "./BridgedClient"; import { IrcMessage, ConnectionInstance } from "./ConnectionInstance"; import { IrcHandler } from "../bridge/IrcHandler"; @@ -354,7 +354,7 @@ export class IrcEventBroker { } // chain off an onMode after the onJoin has been processed. return promise.then(() => { - if (!client.unsafeClient) { + if (client.state.status != BridgedClientStatus.CONNECTED) { req.log.error("No client exists to set onMode for " + name.nick); return null; } @@ -370,14 +370,14 @@ export class IrcEventBroker { prefixLetter = prefix; continue; } - if (client.unsafeClient.isUserPrefixMorePowerfulThan(prefixLetter, prefix)) { + if (client.state.client.isUserPrefixMorePowerfulThan(prefixLetter, prefix)) { prefixLetter = prefix; } } if (!prefixLetter) { return null; } - const modeLetter = client.unsafeClient.modeForPrefix[prefixLetter]; + const modeLetter = client.state.client.modeForPrefix[prefixLetter]; if (!modeLetter) { return null; } diff --git a/src/provisioning/Provisioner.ts b/src/provisioning/Provisioner.ts index 5d6576c16..bcdd78f81 100644 --- a/src/provisioning/Provisioner.ts +++ b/src/provisioning/Provisioner.ts @@ -12,7 +12,7 @@ import * as promiseutil from "../promiseutil"; import * as express from "express"; import { IrcServer } from "../irc/IrcServer"; import { IrcUser } from "../models/IrcUser"; -import { BridgedClient, GetNicksResponseOperators } from "../irc/BridgedClient"; +import { BridgedClient, GetNicksResponseOperators, BridgedClientStatus } from "../irc/BridgedClient"; import { BridgeStateSyncer } from "../bridge/BridgeStateSyncer"; const log = logging("Provisioner"); @@ -1101,10 +1101,10 @@ export class Provisioner { // Using ISUPPORT rules supported by MatrixBridge bot, case map ircChannel private static caseFold(cli: BridgedClient, channel: string) { - if (!cli.unsafeClient) { + if (cli.state.status != BridgedClientStatus.CONNECTED) { log.warn(`Could not case map ${channel} - BridgedClient has no IRC client`); return channel; } - return cli.unsafeClient._toLowerCase(channel); + return cli.state.client._toLowerCase(channel); } } From 0ee2c5bf972c9f0a3db5c124dd336589a0b2d60c Mon Sep 17 00:00:00 2001 From: Christian Paul Date: Wed, 5 Aug 2020 13:13:32 +0200 Subject: [PATCH 02/11] Fix eqeqeq linting errors --- spec/integ/provisioning.spec.js | 4 ++-- src/DebugApi.ts | 4 ++-- src/bridge/AdminRoomHandler.ts | 2 +- src/bridge/IrcBridge.ts | 2 +- src/bridge/MatrixHandler.ts | 2 +- src/bridge/PublicitySyncer.ts | 2 +- src/bridge/RoomAccessSyncer.ts | 2 +- src/irc/BridgedClient.ts | 28 ++++++++++++++-------------- src/irc/ClientPool.ts | 2 +- src/irc/IrcEventBroker.ts | 2 +- src/provisioning/Provisioner.ts | 2 +- 11 files changed, 26 insertions(+), 26 deletions(-) diff --git a/spec/integ/provisioning.spec.js b/spec/integ/provisioning.spec.js index 17bb031ea..a3ed26ff7 100644 --- a/spec/integ/provisioning.spec.js +++ b/spec/integ/provisioning.spec.js @@ -96,7 +96,7 @@ describe("Provisioning API", function() { if (content.status === "failure") { env.isFailed.resolve(); } - else if (content.status == "success") { + else if (content.status === "success") { env.isSuccess.resolve(); } } @@ -472,7 +472,7 @@ describe("Provisioning API", function() { if (content.status === "failure") { env.isFailed.resolve(); } - else if (content.status == "success") { + else if (content.status === "success") { env.isSuccess.resolve(); } } diff --git a/src/DebugApi.ts b/src/DebugApi.ts index 0a93eaba2..70788c349 100644 --- a/src/DebugApi.ts +++ b/src/DebugApi.ts @@ -251,7 +251,7 @@ export class DebugApi { "User " + user + " does not have a client on " + server.domain + "\n" ); } - if (client.state.status != BridgedClientStatus.CONNECTED || !client.state.client.conn) { + if (client.state.status !== BridgedClientStatus.CONNECTED || !client.state.client.conn) { return Bluebird.resolve( "There is no underlying client instance.\n" ); @@ -277,7 +277,7 @@ export class DebugApi { // wait 3s to pool responses return Bluebird.delay(3000).then(function() { // unhook listener to avoid leaking - if (client.state.status == BridgedClientStatus.CONNECTED) { + if (client.state.status === BridgedClientStatus.CONNECTED) { client.state.client.removeListener("raw", listener); } return buffer.join("\n") + "\n"; diff --git a/src/bridge/AdminRoomHandler.ts b/src/bridge/AdminRoomHandler.ts index 989a377ee..e0a5e201b 100644 --- a/src/bridge/AdminRoomHandler.ts +++ b/src/bridge/AdminRoomHandler.ts @@ -287,7 +287,7 @@ export class AdminRoomHandler { server, sender ); - if (bridgedClient.state.status != BridgedClientStatus.CONNECTED) { + if (bridgedClient.state.status !== BridgedClientStatus.CONNECTED) { throw new Error('Possibly disconnected'); } diff --git a/src/bridge/IrcBridge.ts b/src/bridge/IrcBridge.ts index 2d5e51c8e..a2ff3349f 100644 --- a/src/bridge/IrcBridge.ts +++ b/src/bridge/IrcBridge.ts @@ -984,7 +984,7 @@ export class IrcBridge { public sendIrcAction(ircRoom: IrcRoom, bridgedClient: BridgedClient, action: IrcAction) { log.info( "Sending IRC message in %s as %s (connected=%s)", - ircRoom.channel, bridgedClient.nick, Boolean(bridgedClient.state.status == BridgedClientStatus.CONNECTED) + ircRoom.channel, bridgedClient.nick, Boolean(bridgedClient.state.status === BridgedClientStatus.CONNECTED) ); return bridgedClient.sendAction(ircRoom, action); } diff --git a/src/bridge/MatrixHandler.ts b/src/bridge/MatrixHandler.ts index 17ff6052c..c7ed162b0 100644 --- a/src/bridge/MatrixHandler.ts +++ b/src/bridge/MatrixHandler.ts @@ -906,7 +906,7 @@ export class MatrixHandler { } // Check for the existance of the getSplitMessages method. - if (!(ircClient.state.status == BridgedClientStatus.CONNECTED && ircClient.state.client.getSplitMessages)) { + if (!(ircClient.state.status === BridgedClientStatus.CONNECTED && ircClient.state.client.getSplitMessages)) { await this.ircBridge.sendIrcAction(ircRoom, ircClient, ircAction); return; } diff --git a/src/bridge/PublicitySyncer.ts b/src/bridge/PublicitySyncer.ts index ecb3e75ce..8c090c93d 100644 --- a/src/bridge/PublicitySyncer.ts +++ b/src/bridge/PublicitySyncer.ts @@ -50,7 +50,7 @@ export class PublicitySyncer { public initModeForChannel(server: IrcServer, chan: string) { return this.ircBridge.getBotClient(server).then( (client) => { - if (client.state.status != BridgedClientStatus.CONNECTED) { + if (client.state.status !== BridgedClientStatus.CONNECTED) { throw Error("Can't request modes, bot client not connected") } log.info(`Bot requesting mode for ${chan} on ${server.domain}`); diff --git a/src/bridge/RoomAccessSyncer.ts b/src/bridge/RoomAccessSyncer.ts index 70d95fbe7..ce62dcefb 100644 --- a/src/bridge/RoomAccessSyncer.ts +++ b/src/bridge/RoomAccessSyncer.ts @@ -115,7 +115,7 @@ export class RoomAccessSyncer { let userId = null; if (nick !== null && bridgedClient) { userId = bridgedClient.userId; - if (bridgedClient.state.status != BridgedClientStatus.CONNECTED) { + if (bridgedClient.state.status !== BridgedClientStatus.CONNECTED) { req.log.info(`Bridged client for ${nick} has no IRC client.`); return; } diff --git a/src/irc/BridgedClient.ts b/src/irc/BridgedClient.ts index 26acb2933..1ee7e154d 100644 --- a/src/irc/BridgedClient.ts +++ b/src/irc/BridgedClient.ts @@ -197,7 +197,7 @@ export class BridgedClient extends EventEmitter { } public isDead() { - return this.state.status == BridgedClientStatus.DEAD || this.state.status == BridgedClientStatus.KILLED; + return this.state.status === BridgedClientStatus.DEAD || this.state.status === BridgedClientStatus.KILLED; } public toString() { @@ -336,7 +336,7 @@ export class BridgedClient extends EventEmitter { private disconnectWithState(state: State, reason: InstanceDisconnectReason, textReason?: string, explicit = true) { this._explicitDisconnect = explicit; - if (state.status != BridgedClientStatus.CONNECTED) { + if (state.status !== BridgedClientStatus.CONNECTED) { return Promise.resolve(); } return state.inst.disconnect(reason, textReason); @@ -380,7 +380,7 @@ export class BridgedClient extends EventEmitter { } private async sendNickCommand(nick: string): Promise { - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { throw Error("You are not connected to the network."); } const client = this.state.client; @@ -434,7 +434,7 @@ export class BridgedClient extends EventEmitter { } public leaveChannel(channel: string, reason = "User left") { - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { return Promise.resolve(); // we were never connected to the network. } if (channel.indexOf("#") !== 0) { @@ -460,7 +460,7 @@ export class BridgedClient extends EventEmitter { public kick(nick: string, channel: string, reason: string) { reason = reason || "User kicked"; - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { return Promise.resolve(); // we were never connected to the network. } if (Object.keys(this.state.client.chans).indexOf(channel) === -1) { @@ -509,7 +509,7 @@ export class BridgedClient extends EventEmitter { * @param {string} nick : The nick to call /whois on */ public async whois(nick: string): Promise<{ server: IrcServer; nick: string; msg: string}|null> { - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { throw Error("unsafeClient not ready yet"); } const client = this.state.client; @@ -618,7 +618,7 @@ export class BridgedClient extends EventEmitter { if (prefix === "@") { return true; } - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { throw new Error("Missing client"); } if (this.state.client.isUserPrefixMorePowerfulThan(prefix, "@")) { @@ -650,7 +650,7 @@ export class BridgedClient extends EventEmitter { */ public getNicks(channel: string): Bluebird { return new Bluebird((resolve, reject) => { - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { reject(Error("unsafeClient not ready yet")); return; } @@ -711,7 +711,7 @@ export class BridgedClient extends EventEmitter { n = "M" + n; } - if (this.state.status == BridgedClientStatus.CONNECTED) { + if (this.state.status === BridgedClientStatus.CONNECTED) { // nicks can't be too long let maxNickLen = 9; // RFC 1459 default if (this.state.client.supported && @@ -796,7 +796,7 @@ export class BridgedClient extends EventEmitter { } // make sure a killed connection stays killed - if (this.state.status != BridgedClientStatus.KILLED) { + if (this.state.status !== BridgedClientStatus.KILLED) { this._state = { status: BridgedClientStatus.DEAD, } @@ -817,7 +817,7 @@ export class BridgedClient extends EventEmitter { } private async setTopic(room: IrcRoom, topic: string): Promise { - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { throw Error("unsafeClient not ready yet"); } // join the room if we haven't already @@ -840,7 +840,7 @@ export class BridgedClient extends EventEmitter { return; } - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { return; } @@ -863,7 +863,7 @@ export class BridgedClient extends EventEmitter { } public joinChannel(channel: string, key?: string, attemptCount = 1): Bluebird { - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { // we may be trying to join before we've connected, so check and wait if (this.connectDefer && this.connectDefer.promise.isPending()) { return this.connectDefer.promise.then(() => { @@ -909,7 +909,7 @@ export class BridgedClient extends EventEmitter { // add a timeout to try joining again setTimeout(() => { - if (this.state.status != BridgedClientStatus.CONNECTED) { + if (this.state.status !== BridgedClientStatus.CONNECTED) { log.error( `Could not try to join: no client for ${this.nick}, channel = ${channel}` ); diff --git a/src/irc/ClientPool.ts b/src/irc/ClientPool.ts index cd70b014c..548b92929 100644 --- a/src/irc/ClientPool.ts +++ b/src/irc/ClientPool.ts @@ -513,7 +513,7 @@ export class ClientPool { private onClientConnected(bridgedClient: BridgedClient): void { const server = bridgedClient.server; const oldNick = bridgedClient.nick; - if (bridgedClient.state.status != BridgedClientStatus.CONNECTED) { + if (bridgedClient.state.status !== BridgedClientStatus.CONNECTED) { return; } const actualNick = bridgedClient.state.client.nick; diff --git a/src/irc/IrcEventBroker.ts b/src/irc/IrcEventBroker.ts index 59df64f45..b2282c6f1 100644 --- a/src/irc/IrcEventBroker.ts +++ b/src/irc/IrcEventBroker.ts @@ -354,7 +354,7 @@ export class IrcEventBroker { } // chain off an onMode after the onJoin has been processed. return promise.then(() => { - if (client.state.status != BridgedClientStatus.CONNECTED) { + if (client.state.status !== BridgedClientStatus.CONNECTED) { req.log.error("No client exists to set onMode for " + name.nick); return null; } diff --git a/src/provisioning/Provisioner.ts b/src/provisioning/Provisioner.ts index a4c3178f7..9aff380e9 100644 --- a/src/provisioning/Provisioner.ts +++ b/src/provisioning/Provisioner.ts @@ -1101,7 +1101,7 @@ export class Provisioner { // Using ISUPPORT rules supported by MatrixBridge bot, case map ircChannel private static caseFold(cli: BridgedClient, channel: string) { - if (cli.state.status != BridgedClientStatus.CONNECTED) { + if (cli.state.status !== BridgedClientStatus.CONNECTED) { log.warn(`Could not case map ${channel} - BridgedClient has no IRC client`); return channel; } From 7717d691d06f8462e6177dfcc00babf9611bb8e1 Mon Sep 17 00:00:00 2001 From: Christian Paul Date: Wed, 5 Aug 2020 13:14:30 +0200 Subject: [PATCH 03/11] Update newsfile --- changelog.d/{976.bugfix => 1098.bugfix} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{976.bugfix => 1098.bugfix} (100%) diff --git a/changelog.d/976.bugfix b/changelog.d/1098.bugfix similarity index 100% rename from changelog.d/976.bugfix rename to changelog.d/1098.bugfix From f0eb8ed9abfaa22dbb453aaca7fe5f177ea29c18 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 7 Aug 2020 15:02:14 +0100 Subject: [PATCH 04/11] Fake split messages --- spec/util/irc-client-mock.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/util/irc-client-mock.js b/spec/util/irc-client-mock.js index 1c0ae9ba9..c4d06e7aa 100644 --- a/spec/util/irc-client-mock.js +++ b/spec/util/irc-client-mock.js @@ -124,6 +124,10 @@ function Client(addr, nick, opts) { return channel.toLowerCase(); } + this.getSplitMessages = function(target, text) { + return []; + } + this.modeForPrefix = { "@" : "o", "+" : "v", From 6a14533f8c644689d8449ebf14f0ef015e1a037a Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 7 Aug 2020 15:03:15 +0100 Subject: [PATCH 05/11] Expose .status --- src/bridge/IrcBridge.ts | 2 +- src/bridge/RoomAccessSyncer.ts | 2 +- src/irc/BridgedClient.ts | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bridge/IrcBridge.ts b/src/bridge/IrcBridge.ts index a2ff3349f..4c0e2bfb1 100644 --- a/src/bridge/IrcBridge.ts +++ b/src/bridge/IrcBridge.ts @@ -984,7 +984,7 @@ export class IrcBridge { public sendIrcAction(ircRoom: IrcRoom, bridgedClient: BridgedClient, action: IrcAction) { log.info( "Sending IRC message in %s as %s (connected=%s)", - ircRoom.channel, bridgedClient.nick, Boolean(bridgedClient.state.status === BridgedClientStatus.CONNECTED) + ircRoom.channel, bridgedClient.nick, Boolean(bridgedClient.status === BridgedClientStatus.CONNECTED) ); return bridgedClient.sendAction(ircRoom, action); } diff --git a/src/bridge/RoomAccessSyncer.ts b/src/bridge/RoomAccessSyncer.ts index ce62dcefb..b7e86be07 100644 --- a/src/bridge/RoomAccessSyncer.ts +++ b/src/bridge/RoomAccessSyncer.ts @@ -115,7 +115,7 @@ export class RoomAccessSyncer { let userId = null; if (nick !== null && bridgedClient) { userId = bridgedClient.userId; - if (bridgedClient.state.status !== BridgedClientStatus.CONNECTED) { + if (bridgedClient.status !== BridgedClientStatus.CONNECTED) { req.log.info(`Bridged client for ${nick} has no IRC client.`); return; } diff --git a/src/irc/BridgedClient.ts b/src/irc/BridgedClient.ts index 1ee7e154d..cbc1cbd54 100644 --- a/src/irc/BridgedClient.ts +++ b/src/irc/BridgedClient.ts @@ -94,7 +94,7 @@ export class BridgedClient extends EventEmitter { private cachedOperatorNicksInfo: {[channel: string]: GetNicksResponseOperators} = {}; private idleTimeout: NodeJS.Timer|null = null; private whoisPendingNicks: Set = new Set(); - private _state: State = { + private state: State = { status: BridgedClientStatus.CREATED }; /** @@ -171,8 +171,8 @@ export class BridgedClient extends EventEmitter { return Array.from(this._chanList); } - public get state() { - return this._state; + public get status() { + return this.state.status; } public get nick(): string { From 6db409a8ac91c77a5c6dde66a10706049fd4aa61 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 7 Aug 2020 15:03:39 +0100 Subject: [PATCH 06/11] Expose functions which check the connection state --- src/irc/BridgedClient.ts | 95 ++++++++++++++++++++++++++++++++++++++++ src/irc/ClientPool.ts | 4 +- 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/src/irc/BridgedClient.ts b/src/irc/BridgedClient.ts index cbc1cbd54..28d7ebce9 100644 --- a/src/irc/BridgedClient.ts +++ b/src/irc/BridgedClient.ts @@ -957,4 +957,99 @@ export class BridgedClient extends EventEmitter { return defer.promise; } + + public getSplitMessages(target: string, text: string) { + if (this.state.status === BridgedClientStatus.CONNECTED) { + return this.state.client.getSplitMessages(target, text); + } + throw Error('Client is not connected'); + } + + public getClientInternalNick() { + if (this.state.status === BridgedClientStatus.CONNECTED) { + return this.state.client.nick; + } + throw Error('Client is not connected'); + } + + public async mode(channelOrNick: string) { + if (this.state.status === BridgedClientStatus.CONNECTED) { + return this.state.client.mode(channelOrNick); + } + throw Error('Client is not connected'); + } + + public sendCommands(...data: string[]) { + if (this.state.status === BridgedClientStatus.CONNECTED) { + this.state.client.send(...data); + return; + } + throw Error('Client is not connected'); + } + + public writeToConnection(buffer: string|Uint8Array) { + if (this.state.status === BridgedClientStatus.CONNECTED) { + this.state.client.conn?.write(buffer); + return; + } + throw Error('Client is not connected'); + } + + public addClientListener(type: string, listener: (msg: unknown) => void) { + if (this.state.status === BridgedClientStatus.CONNECTED) { + this.state.client.on(type, listener); + return; + } + throw Error('Client is not connected'); + } + + public removeClientListener(type: string, listener: (msg: unknown) => void) { + if (this.state.status === BridgedClientStatus.CONNECTED) { + this.state.client.removeListener(type, listener); + return; + } + // no-op + this.log.info("Tried to unbind listener from client but client was not connected"); + } + + public caseFold(channel: string) { + // Using ISUPPORT rules supported by MatrixBridge bot, case map ircChannel + if (this.state.status !== BridgedClientStatus.CONNECTED) { + log.warn(`Could not case map ${channel} - BridgedClient has no IRC client`); + return channel; + } + return this.state.client._toLowerCase(channel); + } + + public modeForPrefix(prefix: string) { + if (this.state.status === BridgedClientStatus.CONNECTED) { + return this.state.client.modeForPrefix[prefix]; + } + this.log.error("Could not get mode for prefix, client not connected"); + return null; + } + + public isUserPrefixMorePowerfulThan(prefix: string, testPrefix: string) { + if (this.state.status === BridgedClientStatus.CONNECTED) { + return this.state.client.isUserPrefixMorePowerfulThan(prefix, testPrefix); + } + this.log.error("Could not call isUserPrefixMorePowerfulThan, client not connected"); + return null; + } + + public chanData(channel: string) { + if (this.state.status === BridgedClientStatus.CONNECTED) { + return this.state.client.chanData(channel, false); + } + throw Error('Client is not connected'); + } + + public async waitForConnected() { + if (this.state.status === BridgedClientStatus.CONNECTED) { + return; + } else if (this.status !== BridgedClientStatus.CONNECTING) { + throw Error('Client is not connecting or connected'); + } + return this.connectDefer.promise; + } } diff --git a/src/irc/ClientPool.ts b/src/irc/ClientPool.ts index 548b92929..ae4dfb5de 100644 --- a/src/irc/ClientPool.ts +++ b/src/irc/ClientPool.ts @@ -513,10 +513,10 @@ export class ClientPool { private onClientConnected(bridgedClient: BridgedClient): void { const server = bridgedClient.server; const oldNick = bridgedClient.nick; - if (bridgedClient.state.status !== BridgedClientStatus.CONNECTED) { + if (bridgedClient.status !== BridgedClientStatus.CONNECTED) { return; } - const actualNick = bridgedClient.state.client.nick; + const actualNick = bridgedClient.getClientInternalNick(); // remove the pending nick we had set for this user this.virtualClients[server.domain].pending.delete(oldNick); From 20d648a6f1f1ae02b6499fd2baa848bf1e85af78 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 7 Aug 2020 15:04:23 +0100 Subject: [PATCH 07/11] Use exposed functions --- src/DebugApi.ts | 13 +++++-------- src/bridge/AdminRoomHandler.ts | 6 +----- src/bridge/PublicitySyncer.ts | 21 ++++++++------------- src/bridge/RoomAccessSyncer.ts | 7 +++---- src/irc/BridgedClient.ts | 25 ++++++++++--------------- src/irc/IrcEventBroker.ts | 6 +++--- src/provisioning/Provisioner.ts | 13 ++----------- 7 files changed, 32 insertions(+), 59 deletions(-) diff --git a/src/DebugApi.ts b/src/DebugApi.ts index 70788c349..6902b5ccd 100644 --- a/src/DebugApi.ts +++ b/src/DebugApi.ts @@ -251,35 +251,32 @@ export class DebugApi { "User " + user + " does not have a client on " + server.domain + "\n" ); } - if (client.state.status !== BridgedClientStatus.CONNECTED || !client.state.client.conn) { + if (client.status !== BridgedClientStatus.CONNECTED) { return Bluebird.resolve( "There is no underlying client instance.\n" ); } - const connection = client.state.client.conn; // store all received response strings const buffer: string[] = []; // "raw" can take many forms - const listener = (msg: object) => { + const listener = (msg: unknown) => { buffer.push(JSON.stringify(msg)); } - client.state.client.on("raw", listener); + client.addClientListener("raw", listener); // turn rn to n so if there are any new lines they are all n. body = body.replace("\r\n", "\n"); body.split("\n").forEach((c: string) => { // IRC protocol require rn - connection.write(c + "\r\n"); + client.writeToConnection(c + "\r\n"); buffer.push(c); }); // wait 3s to pool responses return Bluebird.delay(3000).then(function() { // unhook listener to avoid leaking - if (client.state.status === BridgedClientStatus.CONNECTED) { - client.state.client.removeListener("raw", listener); - } + client.removeClientListener("raw", listener); return buffer.join("\n") + "\n"; }); } diff --git a/src/bridge/AdminRoomHandler.ts b/src/bridge/AdminRoomHandler.ts index e0a5e201b..397954063 100644 --- a/src/bridge/AdminRoomHandler.ts +++ b/src/bridge/AdminRoomHandler.ts @@ -287,11 +287,7 @@ export class AdminRoomHandler { server, sender ); - if (bridgedClient.state.status !== BridgedClientStatus.CONNECTED) { - throw new Error('Possibly disconnected'); - } - - bridgedClient.state.client.send(...sendArgs); + bridgedClient.sendCommands(...sendArgs); } catch (err) { const notice = new MatrixAction("notice", `${err}\n` ); diff --git a/src/bridge/PublicitySyncer.ts b/src/bridge/PublicitySyncer.ts index 8c090c93d..501d80ebb 100644 --- a/src/bridge/PublicitySyncer.ts +++ b/src/bridge/PublicitySyncer.ts @@ -47,19 +47,14 @@ export class PublicitySyncer { }; constructor (private ircBridge: IrcBridge) { } - public initModeForChannel(server: IrcServer, chan: string) { - return this.ircBridge.getBotClient(server).then( - (client) => { - if (client.state.status !== BridgedClientStatus.CONNECTED) { - throw Error("Can't request modes, bot client not connected") - } - log.info(`Bot requesting mode for ${chan} on ${server.domain}`); - client.state.client.mode(chan); - }, - (err) => { - log.error(`Could not request mode of ${chan} (${err.message})`); - } - ); + public async initModeForChannel(server: IrcServer, chan: string) { + try { + const botClient = await this.ircBridge.getBotClient(server); + log.info(`Bot requesting mode for ${chan} on ${server.domain}`); + await botClient.mode(chan); + } catch (err) { + log.error(`Could not request mode of ${chan} (${err.message})`); + } } public async initModes (server: IrcServer) { diff --git a/src/bridge/RoomAccessSyncer.ts b/src/bridge/RoomAccessSyncer.ts index b7e86be07..2953676ef 100644 --- a/src/bridge/RoomAccessSyncer.ts +++ b/src/bridge/RoomAccessSyncer.ts @@ -119,8 +119,7 @@ export class RoomAccessSyncer { req.log.info(`Bridged client for ${nick} has no IRC client.`); return; } - const client = bridgedClient.state.client; - const chanData = client.chanData(channel); + const chanData = bridgedClient.chanData(channel); if (!(chanData && chanData.users)) { req.log.error(`No channel data for ${channel}`); return; @@ -128,8 +127,8 @@ export class RoomAccessSyncer { const userPrefixes = chanData.users[nick] as string; userPrefixes.split('').forEach( prefix => { - const m = client.modeForPrefix[prefix]; - if (modeToPower[m] !== undefined) { + const m = bridgedClient.modeForPrefix(prefix); + if (m && modeToPower[m] !== undefined) { userPowers.push(modeToPower[m]); } } diff --git a/src/irc/BridgedClient.ts b/src/irc/BridgedClient.ts index 28d7ebce9..d866ee404 100644 --- a/src/irc/BridgedClient.ts +++ b/src/irc/BridgedClient.ts @@ -179,20 +179,20 @@ export class BridgedClient extends EventEmitter { return this._nick; } - public getClientConfig() { return this.clientConfig; } public kill(reason?: string) { + log.info('Killing client ', this.nick); const state = this.state; // so that no further commands can be issued - this._state = { + log.debug("Client is now KILLED") + this.state = { status: BridgedClientStatus.KILLED } // kill connection instance - log.info('Killing client ', this.nick); return this.disconnectWithState(state, "killed", reason); } @@ -211,7 +211,8 @@ export class BridgedClient extends EventEmitter { public async connect(): Promise { let identResolver: (() => void) | undefined; - this._state = { + this.log.debug("Client is now CONNECTING"); + this.state = { status: BridgedClientStatus.CONNECTING } @@ -250,8 +251,8 @@ export class BridgedClient extends EventEmitter { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion this.onConnectionCreated(inst, nameInfo, identResolver!); }); - - this._state = { + this.log.info("Client is now CONNECTED"); + this.state = { status: BridgedClientStatus.CONNECTED, inst: connInst, client: connInst.client, @@ -306,7 +307,8 @@ export class BridgedClient extends EventEmitter { } catch (err) { this.log.debug("Failed to connect."); - this._state = { + this.log.info("Client is now DEAD") + this.state = { status: BridgedClientStatus.DEAD } if (identResolver) { @@ -793,14 +795,7 @@ export class BridgedClient extends EventEmitter { if (reason === "banned") { // If we've been banned, this is intentional. this._explicitDisconnect = true; - } - - // make sure a killed connection stays killed - if (this.state.status !== BridgedClientStatus.KILLED) { - this._state = { - status: BridgedClientStatus.DEAD, - } - } + } else if (reason) this.emit("client-disconnected", this); this.eventBroker.sendMetadata(this, diff --git a/src/irc/IrcEventBroker.ts b/src/irc/IrcEventBroker.ts index b2282c6f1..05ef9af20 100644 --- a/src/irc/IrcEventBroker.ts +++ b/src/irc/IrcEventBroker.ts @@ -354,7 +354,7 @@ export class IrcEventBroker { } // chain off an onMode after the onJoin has been processed. return promise.then(() => { - if (client.state.status !== BridgedClientStatus.CONNECTED) { + if (client.status !== BridgedClientStatus.CONNECTED) { req.log.error("No client exists to set onMode for " + name.nick); return null; } @@ -370,14 +370,14 @@ export class IrcEventBroker { prefixLetter = prefix; continue; } - if (client.state.client.isUserPrefixMorePowerfulThan(prefixLetter, prefix)) { + if (client.isUserPrefixMorePowerfulThan(prefixLetter, prefix)) { prefixLetter = prefix; } } if (!prefixLetter) { return null; } - const modeLetter = client.state.client.modeForPrefix[prefixLetter]; + const modeLetter = client.modeForPrefix(prefixLetter); if (!modeLetter) { return null; } diff --git a/src/provisioning/Provisioner.ts b/src/provisioning/Provisioner.ts index 9aff380e9..97a740b5e 100644 --- a/src/provisioning/Provisioner.ts +++ b/src/provisioning/Provisioner.ts @@ -691,7 +691,7 @@ export class Provisioner { const botClient = await this.ircBridge.getBotClient(server); - ircChannel = Provisioner.caseFold(botClient, ircChannel); + ircChannel = botClient.caseFold(ircChannel); if (server.isExcludedChannel(ircChannel)) { throw new Error(`Server is configured to exclude channel ${ircChannel}`); @@ -775,7 +775,7 @@ export class Provisioner { const botClient = await this.ircBridge.getBotClient(server); - ircChannel = Provisioner.caseFold(botClient, ircChannel); + ircChannel = botClient.caseFold(ircChannel); if (server.isExcludedChannel(ircChannel)) { throw new Error(`Server is configured to exclude given channel ('${ircChannel}')`); @@ -1098,13 +1098,4 @@ export class Provisioner { limit, }; } - - // Using ISUPPORT rules supported by MatrixBridge bot, case map ircChannel - private static caseFold(cli: BridgedClient, channel: string) { - if (cli.state.status !== BridgedClientStatus.CONNECTED) { - log.warn(`Could not case map ${channel} - BridgedClient has no IRC client`); - return channel; - } - return cli.state.client._toLowerCase(channel); - } } From d5a4ee016568f8e272b5a622aa1edcbb8740c0fb Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 7 Aug 2020 15:04:36 +0100 Subject: [PATCH 08/11] Put a safety wait statement to ensure we don't try to getSplitMessages too early --- src/bridge/MatrixHandler.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/bridge/MatrixHandler.ts b/src/bridge/MatrixHandler.ts index c7ed162b0..94e5d8ebc 100644 --- a/src/bridge/MatrixHandler.ts +++ b/src/bridge/MatrixHandler.ts @@ -905,14 +905,11 @@ export class MatrixHandler { this.eventCache.delete(delKey); } - // Check for the existance of the getSplitMessages method. - if (!(ircClient.state.status === BridgedClientStatus.CONNECTED && ircClient.state.client.getSplitMessages)) { - await this.ircBridge.sendIrcAction(ircRoom, ircClient, ircAction); - return; - } + // The client might still be connected, for abundance of safety let's wait. + await ircClient.waitForConnected(); // Generate an array of individual messages that would be sent - const potentialMessages = ircClient.state.client.getSplitMessages(ircRoom.channel, text); + const potentialMessages = ircClient.getSplitMessages(ircRoom.channel, text); const lineLimit = ircRoom.server.getLineLimit(); if (potentialMessages.length <= lineLimit) { From 0ad9e0956d0094045222b7e9c3d8544f4664d65d Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 7 Aug 2020 15:14:23 +0100 Subject: [PATCH 09/11] linting --- src/bridge/AdminRoomHandler.ts | 2 +- src/bridge/MatrixHandler.ts | 2 +- src/bridge/PublicitySyncer.ts | 4 ++-- src/irc/BridgedClient.ts | 13 +++++++------ src/provisioning/Provisioner.ts | 2 +- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/bridge/AdminRoomHandler.ts b/src/bridge/AdminRoomHandler.ts index 397954063..2d280fc8c 100644 --- a/src/bridge/AdminRoomHandler.ts +++ b/src/bridge/AdminRoomHandler.ts @@ -19,7 +19,7 @@ import { MatrixRoom, MatrixUser } from "matrix-appservice-bridge"; import { IrcBridge } from "./IrcBridge"; import { MatrixAction } from "../models/MatrixAction"; import { IrcServer } from "../irc/IrcServer"; -import { BridgedClient, BridgedClientStatus } from "../irc/BridgedClient"; +import { BridgedClient } from "../irc/BridgedClient"; import { IrcClientConfig } from "../models/IrcClientConfig"; import { MatrixHandler } from "./MatrixHandler"; import logging from "../logging"; diff --git a/src/bridge/MatrixHandler.ts b/src/bridge/MatrixHandler.ts index 94e5d8ebc..2ae8b8809 100644 --- a/src/bridge/MatrixHandler.ts +++ b/src/bridge/MatrixHandler.ts @@ -4,7 +4,7 @@ import { MatrixUser, MatrixRoom, StateLookup } from "matrix-appservice-bridge"; import { IrcUser } from "../models/IrcUser"; import { MatrixAction, MatrixMessageEvent } from "../models/MatrixAction"; import { IrcRoom } from "../models/IrcRoom"; -import { BridgedClient, BridgedClientStatus } from "../irc/BridgedClient"; +import { BridgedClient } from "../irc/BridgedClient"; import { IrcServer } from "../irc/IrcServer"; import { IrcAction } from "../models/IrcAction"; import { toIrcLowerCase } from "../irc/formatting"; diff --git a/src/bridge/PublicitySyncer.ts b/src/bridge/PublicitySyncer.ts index 501d80ebb..ebaaa5c64 100644 --- a/src/bridge/PublicitySyncer.ts +++ b/src/bridge/PublicitySyncer.ts @@ -1,7 +1,6 @@ import logger from "../logging"; import { IrcBridge } from "./IrcBridge"; import { IrcServer } from "../irc/IrcServer"; -import { BridgedClientStatus } from "../irc/BridgedClient"; const log = logger("PublicitySyncer"); @@ -52,7 +51,8 @@ export class PublicitySyncer { const botClient = await this.ircBridge.getBotClient(server); log.info(`Bot requesting mode for ${chan} on ${server.domain}`); await botClient.mode(chan); - } catch (err) { + } + catch (err) { log.error(`Could not request mode of ${chan} (${err.message})`); } } diff --git a/src/irc/BridgedClient.ts b/src/irc/BridgedClient.ts index d866ee404..abb5df7b3 100644 --- a/src/irc/BridgedClient.ts +++ b/src/irc/BridgedClient.ts @@ -795,7 +795,7 @@ export class BridgedClient extends EventEmitter { if (reason === "banned") { // If we've been banned, this is intentional. this._explicitDisconnect = true; - } else if (reason) + } this.emit("client-disconnected", this); this.eventBroker.sendMetadata(this, @@ -983,8 +983,8 @@ export class BridgedClient extends EventEmitter { } public writeToConnection(buffer: string|Uint8Array) { - if (this.state.status === BridgedClientStatus.CONNECTED) { - this.state.client.conn?.write(buffer); + if (this.state.status === BridgedClientStatus.CONNECTED && this.state.client.conn) { + this.state.client.conn.write(buffer); return; } throw Error('Client is not connected'); @@ -1039,10 +1039,11 @@ export class BridgedClient extends EventEmitter { throw Error('Client is not connected'); } - public async waitForConnected() { + public async waitForConnected(): Promise { if (this.state.status === BridgedClientStatus.CONNECTED) { - return; - } else if (this.status !== BridgedClientStatus.CONNECTING) { + return Promise.resolve(); + } + else if (this.status !== BridgedClientStatus.CONNECTING) { throw Error('Client is not connecting or connected'); } return this.connectDefer.promise; diff --git a/src/provisioning/Provisioner.ts b/src/provisioning/Provisioner.ts index 97a740b5e..51ce8e791 100644 --- a/src/provisioning/Provisioner.ts +++ b/src/provisioning/Provisioner.ts @@ -12,7 +12,7 @@ import * as promiseutil from "../promiseutil"; import * as express from "express"; import { IrcServer } from "../irc/IrcServer"; import { IrcUser } from "../models/IrcUser"; -import { BridgedClient, GetNicksResponseOperators, BridgedClientStatus } from "../irc/BridgedClient"; +import { GetNicksResponseOperators } from "../irc/BridgedClient"; import { BridgeStateSyncer } from "../bridge/BridgeStateSyncer"; const log = logging("Provisioner"); From 12bc0540cab3aead7ec2fdf519449479cd245ecf Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Fri, 7 Aug 2020 15:20:03 +0100 Subject: [PATCH 10/11] set state to DEAD on disconnect --- src/irc/BridgedClient.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/irc/BridgedClient.ts b/src/irc/BridgedClient.ts index abb5df7b3..f4e0652ad 100644 --- a/src/irc/BridgedClient.ts +++ b/src/irc/BridgedClient.ts @@ -797,6 +797,12 @@ export class BridgedClient extends EventEmitter { this._explicitDisconnect = true; } + if (this.status !== BridgedClientStatus.KILLED) { + this.state = { + status: BridgedClientStatus.DEAD + }; + } + this.emit("client-disconnected", this); this.eventBroker.sendMetadata(this, "Your connection to the IRC network '" + this.server.domain + From f59a5c0b92d667df0cff22a0d9cf4a6176364b22 Mon Sep 17 00:00:00 2001 From: Christian Paul Date: Fri, 7 Aug 2020 17:40:52 +0200 Subject: [PATCH 11/11] Fix condition with .includes() --- src/irc/BridgedClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/irc/BridgedClient.ts b/src/irc/BridgedClient.ts index ed3beaf65..7f66ed7ff 100644 --- a/src/irc/BridgedClient.ts +++ b/src/irc/BridgedClient.ts @@ -465,7 +465,7 @@ export class BridgedClient extends EventEmitter { if (this.state.status !== BridgedClientStatus.CONNECTED) { return Promise.resolve(); // we were never connected to the network. } - if (Object.keys(this.state.client.chans).includes(channel)) { + if (!Object.keys(this.state.client.chans).includes(channel)) { // we were never joined to it. We need to be joined to it to kick people. return Promise.resolve(); }