Handle nick collisions somewhat instead of blindly overwriting the nicks

in memory, which does weird things.

For fun different ircds implement this differently (Unreal compares
timestamps, TS6 compares timestamps and user username/host), and whether
or not we get a kill for our user also varies, so just kill everyone.

This can't really happen anyway with properly set qlines, only if
services haven't yet set the qlines, or possibly in a bot add/nick
user introduce race, or with enforcers, which not many ircds require.

4 files changed, 86 insertions, 10 deletions

diff --git a/src/bots.cpp b/src/bots.cpp
index feedacc63..77b7d1c35 100644
--- a/src/bots.cpp
+++ b/src/bots.cpp
@@ -108,14 +108,31 @@ Serializable* BotInfo::Unserialize(Serializable *obj, Serialize::Data &data)
 
 void BotInfo::GenerateUID()
 {
+	if (this->introduced)
+		throw CoreException("Changing bot UID when it is introduced?");
+
 	if (!this->uid.empty())
-		throw CoreException("Bot already has a uid?");
+	{
+		BotListByUID->erase(this->uid);
+		UserListByUID.erase(this->uid);
+	}
 
 	this->uid = Servers::TS6_UID_Retrieve();
 	(*BotListByUID)[this->uid] = this;
 	UserListByUID[this->uid] = this;
 }
 
+void BotInfo::OnKill()
+{
+	this->introduced = false;
+	this->GenerateUID();
+	IRCD->SendClientIntroduction(this);
+	this->introduced = true;
+
+	for (User::ChanUserList::const_iterator cit = this->chans.begin(), cit_end = this->chans.end(); cit != cit_end; ++cit)
+		IRCD->SendJoin(this, cit->second->chan, &cit->second->status);
+}
+
 void BotInfo::SetNewNick(const Anope::string &newnick)
 {
 	UserListByNick.erase(this->nick);
diff --git a/src/messages.cpp b/src/messages.cpp
index 2bca495a7..bb699a18e 100644
--- a/src/messages.cpp
+++ b/src/messages.cpp
@@ -199,12 +199,7 @@ void Kill::Run(MessageSource &source, const std::vector<Anope::string> &params)
 		}
 		last_time = Anope::CurTime;
 
-		bi->introduced = false;
-		IRCD->SendClientIntroduction(bi);
-		bi->introduced = true;
-
-		for (User::ChanUserList::const_iterator cit = bi->chans.begin(), cit_end = bi->chans.end(); cit != cit_end; ++cit)
-			IRCD->SendJoin(bi, cit->second->chan, &cit->second->status);
+		bi->OnKill();
 	}
 	else
 		u->KillInternal(source.GetSource(), params[1]);
diff --git a/src/protocol.cpp b/src/protocol.cpp
index 01c8d31c8..b90c44400 100644
--- a/src/protocol.cpp
+++ b/src/protocol.cpp
@@ -45,6 +45,11 @@ const Anope::string &IRCDProto::GetProtocolName()
 	return this->proto_name;
 }
 
+void IRCDProto::SendKill(const MessageSource &source, const Anope::string &target, const Anope::string &reason)
+{
+	UplinkSocket::Message(source) << "KILL " << target << " :" << reason;
+}
+
 void IRCDProto::SendSVSKillInternal(const MessageSource &source, User *user, const Anope::string &buf)
 {
 	UplinkSocket::Message(source) << "KILL " << user->GetUID() << " :" << buf;
diff --git a/src/users.cpp b/src/users.cpp
index b9564a2d5..97c850111 100644
--- a/src/users.cpp
+++ b/src/users.cpp
@@ -33,7 +33,7 @@ time_t MaxUserTime = 0;
 
 std::list<User *> User::quitting_users;
 
-User::User(const Anope::string &snick, const Anope::string &sident, const Anope::string &shost, const Anope::string &svhost, const Anope::string &sip, Server *sserver, const Anope::string &srealname, time_t ssignon, const Anope::string &smodes, const Anope::string &suid, NickCore *account)
+User::User(const Anope::string &snick, const Anope::string &sident, const Anope::string &shost, const Anope::string &svhost, const Anope::string &sip, Server *sserver, const Anope::string &srealname, time_t ts, const Anope::string &smodes, const Anope::string &suid, NickCore *account)
 {
 	if (snick.empty() || sident.empty() || shost.empty())
 		throw CoreException("Bad args passed to User::User");
@@ -52,7 +52,7 @@ User::User(const Anope::string &snick, const Anope::string &sident, const Anope:
 	this->ip = sip;
 	this->server = sserver;
 	this->realname = srealname;
-	this->timestamp = this->signon = ssignon;
+	this->timestamp = this->signon = ts;
 	this->SetModesInternal(sserver, "%s", smodes.c_str());
 	this->uid = suid;
 	this->super_admin = false;
@@ -88,6 +88,56 @@ User::User(const Anope::string &snick, const Anope::string &sident, const Anope:
 	FOREACH_MOD(OnUserConnect, (this, exempt));
 }
 
+static void CollideKill(User *target, const Anope::string &reason)
+{
+	if (target->server != Me)
+		target->Kill(Me, reason);
+	else
+	{
+		// Be sure my user is really dead
+		IRCD->SendQuit(target, "%s", reason.c_str());
+
+		// Reintroduce my client
+		if (BotInfo *bi = dynamic_cast<BotInfo *>(target))
+			bi->OnKill();
+		else
+			target->Quit(reason);
+	}
+}
+
+static void Collide(User *u, const Anope::string &id, const Anope::string &type)
+{
+	// Kill incoming user
+	IRCD->SendKill(Me, id, type);
+	// Quit colliding user
+	CollideKill(u, type);
+}
+
+User* User::OnIntroduce(const Anope::string &snick, const Anope::string &sident, const Anope::string &shost, const Anope::string &svhost, const Anope::string &sip, Server *sserver, const Anope::string &srealname, time_t ts, const Anope::string &smodes, const Anope::string &suid, NickCore *nc)
+{
+	// How IRCds handle collisions varies a lot, for safety well just always kill both sides
+	// With properly set qlines, this can almost never happen anyway
+
+	User *u = User::Find(snick);
+	if (u)
+	{
+		Collide(u, !suid.empty() ? suid : snick, "Nick collision");
+		return NULL;
+	}
+
+	if (!suid.empty())
+	{
+		u = User::Find(suid);
+		if (u)
+		{
+			Collide(u, suid, "ID collision");
+			return NULL;
+		}
+	}
+
+	return new User(snick, sident, shost, svhost, sip, sserver, srealname, ts, smodes, suid, nc);
+}
+
 void User::ChangeNick(const Anope::string &newnick, time_t ts)
 {
 	/* Sanity check to make sure we don't segfault */
@@ -109,8 +159,17 @@ void User::ChangeNick(const Anope::string &newnick, time_t ts)
 			old_na->last_seen = Anope::CurTime;
 		
 		UserListByNick.erase(this->nick);
+
 		this->nick = newnick;
-		UserListByNick[this->nick] = this;
+
+		User* &other = UserListByNick[this->nick];
+		if (other)
+		{
+			CollideKill(this, "Nick collision");
+			CollideKill(other, "Nick collision");
+			return;
+		}
+		other = this;
 
 		on_access = false;
 		NickAlias *na = NickAlias::Find(this->nick);