diff --git a/server/models/network.ts b/server/models/network.ts index ee5b7d92..167abd46 100644 --- a/server/models/network.ts +++ b/server/models/network.ts @@ -12,6 +12,7 @@ import Client from "../client"; import {MessageType} from "../../shared/types/msg"; import {ChanType} from "../../shared/types/chan"; import {SharedNetwork} from "../../shared/types/network"; +import NickKeeper, {NickKeeperOwner} from "./nickKeeper"; type NetworkIrcOptions = { host: string; @@ -121,6 +122,7 @@ class Network { chanCache!: Chan[]; ignoreList!: IgnoreList; keepNick!: string | null; + private nickKeeper?: NickKeeper; status!: NetworkStatus; @@ -196,6 +198,32 @@ class Network { ); } + getKeepNick(this: Network) { + return this.keepNick; + } + + setKeepNick(this: Network, nick: string) { + this.keepNick = nick; + } + + clearKeepNick(this: Network) { + this.keepNick = null; + } + + getNickKeeper(this: Network) { + if (!this.nickKeeper) { + const owner: NickKeeperOwner = { + getKeepNick: () => this.getKeepNick(), + setKeepNick: (nick) => this.setKeepNick(nick), + clearKeepNick: () => this.clearKeepNick(), + }; + + this.nickKeeper = new NickKeeper(owner); + } + + return this.nickKeeper; + } + validate(this: Network, client: Client) { // Remove !, :, @ and whitespace characters from nicknames and usernames const cleanNick = (str: string) => str.replace(/[\x00\s:!@]/g, "_").substring(0, 100); @@ -394,7 +422,7 @@ class Network { const oldNick = this.nick; const oldRealname = this.realname; - this.keepNick = null; + this.clearKeepNick(); this.nick = args.nick; this.host = String(args.host || ""); this.name = String(args.name || "") || this.host; @@ -493,7 +521,7 @@ class Network { ); if (this.keepNick === nick) { - this.keepNick = null; + this.clearKeepNick(); } if (this.irc?.options) { diff --git a/server/models/nickKeeper.ts b/server/models/nickKeeper.ts new file mode 100644 index 00000000..1f6e7712 --- /dev/null +++ b/server/models/nickKeeper.ts @@ -0,0 +1,60 @@ +export type NickInUseContext = { + registered: boolean; + isPublic: boolean; +}; + +export type RegisteredResult = { + shouldUpdatePreferred: boolean; +}; + +export type NickKeeperOwner = { + getKeepNick(): string | null; + setKeepNick(nick: string): void; + clearKeepNick(): void; +}; + +export default class NickKeeper { + private owner: NickKeeperOwner; + + constructor(owner: NickKeeperOwner) { + this.owner = owner; + } + + onNickInUse(preferredNick: string, context: NickInUseContext) { + if (!context.registered && !context.isPublic) { + this.owner.setKeepNick(preferredNick); + } + } + + onRegistered(registeredNick: string, preferredNick: string): RegisteredResult { + if (registeredNick === preferredNick) { + if (this.owner.getKeepNick() === registeredNick) { + this.owner.clearKeepNick(); + } + + return {shouldUpdatePreferred: true}; + } + + return {shouldUpdatePreferred: false}; + } + + onQuit(quitNick: string) { + if (this.owner.getKeepNick() === quitNick) { + this.owner.clearKeepNick(); + return true; + } + + return false; + } + + onSocketClose() { + const keepNick = this.owner.getKeepNick(); + + if (!keepNick) { + return null; + } + + this.owner.clearKeepNick(); + return keepNick; + } +} diff --git a/server/plugins/inputs/nick.ts b/server/plugins/inputs/nick.ts index 90b7bd42..5032e750 100644 --- a/server/plugins/inputs/nick.ts +++ b/server/plugins/inputs/nick.ts @@ -43,7 +43,7 @@ const input: PluginInputHandler = function (network, chan, cmd, args) { } // If we were trying to keep a nick and user changes nick, stop trying to keep the old one - network.keepNick = null; + network.clearKeepNick(); // If connected to IRC, send to server and wait for ACK // otherwise update the nick and UI straight away diff --git a/server/plugins/irc-events/connection.ts b/server/plugins/irc-events/connection.ts index ff28f545..16237865 100644 --- a/server/plugins/irc-events/connection.ts +++ b/server/plugins/irc-events/connection.ts @@ -124,12 +124,13 @@ export default function (irc, network) { ); } - if (network.keepNick) { - // We disconnected without getting our original nick back yet, just set it back locally - irc.options.nick = irc.user.nick = network.keepNick; + const keepNick = network.getNickKeeper().onSocketClose(); - network.setNick(network.keepNick); - network.keepNick = null; + if (keepNick) { + // We disconnected without getting our original nick back yet, just set it back locally + irc.options.nick = irc.user.nick = keepNick; + + network.setNick(keepNick); client.emit("nick", { network: network.uuid, diff --git a/server/plugins/irc-events/error.ts b/server/plugins/irc-events/error.ts index 7140bf65..074322a6 100644 --- a/server/plugins/irc-events/error.ts +++ b/server/plugins/irc-events/error.ts @@ -40,9 +40,10 @@ export default function (irc, network) { if (irc.connection.registered === false && !Config.values.public) { message += " An attempt to use it will be made when this nick quits."; - // Store the user's preferred nick in keepNick so the quit handler can reclaim it - // This is the user's actual preference from network.nick, not a fallback - network.keepNick = network.nick; + // Store the user's preferred nick so the quit handler can reclaim it + network + .getNickKeeper() + .onNickInUse(network.nick, {registered: false, isPublic: Config.values.public}); } const lobby = network.getLobby(); diff --git a/server/plugins/irc-events/quit.ts b/server/plugins/irc-events/quit.ts index 74393454..b1ed9986 100644 --- a/server/plugins/irc-events/quit.ts +++ b/server/plugins/irc-events/quit.ts @@ -27,9 +27,8 @@ export default function (irc, network) { }); // If user with the nick we are trying to keep has quit, try to get this nick - if (network.keepNick === data.nick) { - irc.changeNick(network.keepNick); - network.keepNick = null; + if (network.getNickKeeper().onQuit(data.nick)) { + irc.changeNick(data.nick); } }); }; diff --git a/server/plugins/irc-events/welcome.ts b/server/plugins/irc-events/welcome.ts index 7c9ea2ac..db143c0d 100644 --- a/server/plugins/irc-events/welcome.ts +++ b/server/plugins/irc-events/welcome.ts @@ -6,15 +6,14 @@ export default function (irc, network) { const client = this; irc.on("registered", function (data) { + const nickKeeper = network.getNickKeeper(); + const {shouldUpdatePreferred} = nickKeeper.onRegistered(data.nick, network.nick); + // Only update the user's preferred nick (network.nick) if we registered with it // If we registered with a fallback nick (e.g., nick123), don't overwrite the preference - // This allows the existing quit handler to reclaim the preferred nick when available - if (data.nick === network.nick) { - // We got our preferred nick, clear keepNick if it was set - if (network.keepNick === data.nick) { - network.keepNick = null; - } - } else if (data.nick !== network.nick) { + if (shouldUpdatePreferred) { + network.setNick(data.nick); + } else { // We registered with a fallback, don't call setNick which would overwrite the preference // Just update the IRC session nick irc.user.nick = data.nick; diff --git a/test/tests/nickKeeper.ts b/test/tests/nickKeeper.ts new file mode 100644 index 00000000..c0481d18 --- /dev/null +++ b/test/tests/nickKeeper.ts @@ -0,0 +1,112 @@ +import {expect} from "chai"; +import NickKeeper from "../../server/models/nickKeeper"; + +type Owner = { + keepNick: string | null; + getKeepNick(): string | null; + setKeepNick(nick: string): void; + clearKeepNick(): void; +}; + +function createOwner(initial: string | null = null): Owner { + return { + keepNick: initial, + getKeepNick() { + return this.keepNick; + }, + setKeepNick(nick: string) { + this.keepNick = nick; + }, + clearKeepNick() { + this.keepNick = null; + }, + }; +} + +describe("NickKeeper", function () { + it("stores preferred nick when nick in use on connect", function () { + const owner = createOwner(); + const keeper = new NickKeeper(owner); + + keeper.onNickInUse("preferred", {registered: false, isPublic: false}); + + expect(owner.keepNick).to.equal("preferred"); + }); + + it("does not store preferred nick when already registered", function () { + const owner = createOwner(); + const keeper = new NickKeeper(owner); + + keeper.onNickInUse("preferred", {registered: true, isPublic: false}); + + expect(owner.keepNick).to.equal(null); + }); + + it("does not store preferred nick in public mode", function () { + const owner = createOwner(); + const keeper = new NickKeeper(owner); + + keeper.onNickInUse("preferred", {registered: false, isPublic: true}); + + expect(owner.keepNick).to.equal(null); + }); + + it("clears keepNick when registered with preferred nick", function () { + const owner = createOwner("preferred"); + const keeper = new NickKeeper(owner); + + const result = keeper.onRegistered("preferred", "preferred"); + + expect(result.shouldUpdatePreferred).to.equal(true); + expect(owner.keepNick).to.equal(null); + }); + + it("does not clear keepNick when registered with fallback", function () { + const owner = createOwner("preferred"); + const keeper = new NickKeeper(owner); + + const result = keeper.onRegistered("fallback", "preferred"); + + expect(result.shouldUpdatePreferred).to.equal(false); + expect(owner.keepNick).to.equal("preferred"); + }); + + it("reclaims preferred nick on quit and clears keepNick", function () { + const owner = createOwner("preferred"); + const keeper = new NickKeeper(owner); + + const shouldReclaim = keeper.onQuit("preferred"); + + expect(shouldReclaim).to.equal(true); + expect(owner.keepNick).to.equal(null); + }); + + it("does not reclaim when quit nick does not match", function () { + const owner = createOwner("preferred"); + const keeper = new NickKeeper(owner); + + const shouldReclaim = keeper.onQuit("other"); + + expect(shouldReclaim).to.equal(false); + expect(owner.keepNick).to.equal("preferred"); + }); + + it("returns keepNick on socket close and clears it", function () { + const owner = createOwner("preferred"); + const keeper = new NickKeeper(owner); + + const keepNick = keeper.onSocketClose(); + + expect(keepNick).to.equal("preferred"); + expect(owner.keepNick).to.equal(null); + }); + + it("returns null on socket close when no keepNick", function () { + const owner = createOwner(null); + const keeper = new NickKeeper(owner); + + const keepNick = keeper.onSocketClose(); + + expect(keepNick).to.equal(null); + }); +});