From d0ded906d4ac8f91c453f6ee544cc7638365a851 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Tue, 1 Jan 2019 21:16:29 -0500 Subject: [PATCH] fix a fairly bad bug where nicks could get out of sync during nick change, removeInternal(client) was being called even before checking whether the new nick was in use or reserved. Reproduction steps: 1. Log in a client 'alice' 2. Log in a client 'bob' 3. bob issues /nick alice, which fails (correctly) with: :oragono.test 433 bob alice :Nickname is already in use 4. alice issues /msg bob hi, which fails (incorrectly) with: :oragono.test 401 alice bob :No such nick --- irc/client_lookup_set.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/irc/client_lookup_set.go b/irc/client_lookup_set.go index e54e29b6..6a3a749c 100644 --- a/irc/client_lookup_set.go +++ b/irc/client_lookup_set.go @@ -129,7 +129,6 @@ func (clients *ClientManager) SetNick(client *Client, newNick string) error { clients.Lock() defer clients.Unlock() - clients.removeInternal(client) currentNewEntry := clients.byNick[newcfnick] // the client may just be changing case if currentNewEntry != nil && currentNewEntry != client { @@ -138,6 +137,7 @@ func (clients *ClientManager) SetNick(client *Client, newNick string) error { if method == NickReservationStrict && reservedAccount != client.Account() { return errNicknameReserved } + clients.removeInternal(client) clients.byNick[newcfnick] = client client.updateNickMask(newNick) return nil