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.

15 files changed, 105 insertions, 21 deletions

diff --git a/include/bots.h b/include/bots.h
index 349cab617..bab3931c5 100644
--- a/include/bots.h
+++ b/include/bots.h
@@ -60,6 +60,8 @@ class CoreExport BotInfo : public User, public Serializable
 
 	void GenerateUID();
 
+	void OnKill();
+
 	/** Change the nickname for the bot.
 	 * @param newnick The nick to change to
 	 */
diff --git a/include/protocol.h b/include/protocol.h
index 0f8482b4e..8f4f2ea58 100644
--- a/include/protocol.h
+++ b/include/protocol.h
@@ -111,6 +111,8 @@ class CoreExport IRCDProto : public Service
 	virtual void SendSQLine(User *, const XLine *x) { }
 	virtual void SendSQLineDel(const XLine *x) { }
 
+	virtual void SendKill(const MessageSource &source, const Anope::string &target, const Anope::string &reason);
+
 	/** Kills a user
 	 * @param source Who is doing the kill
 	 * @param user The user to be killed
diff --git a/include/users.h b/include/users.h
index 4b07b2f33..73c373938 100644
--- a/include/users.h
+++ b/include/users.h
@@ -92,6 +92,7 @@ class CoreExport User : public virtual Base, public Extensible, public CommandRe
 	/* Last time this user sent an email */
 	time_t lastmail;
 
+ protected:
 	/** Create a new user object, initialising necessary fields and
 	 * adds it to the hash
 	 *
@@ -102,19 +103,20 @@ class CoreExport User : public virtual Base, public Extensible, public CommandRe
 	 * @param sip The ip of the user
 	 * @param sserver The server of the user
 	 * @param srealname The realname/gecos of teh user
-	 * @param ssignon User's timestamp
+	 * @param ts User's timestamp
 	 * @param smodes User's modes
 	 * @param suid The unique identifier of the user.
 	 * @param nc The account the user is identified as, if any
 	 */
-	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 *nc);
+	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 *nc);
 
- protected:
 	/** Destroy a user.
 	 */
 	virtual ~User();
 
  public:
+	static 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);
+
 	/** Update the nickname of a user record accordingly, should be
 	 * called from ircd protocol.
 	 * @param newnick The new username
diff --git a/modules/protocol/bahamut.cpp b/modules/protocol/bahamut.cpp
index 80f4c4827..a114db890 100644
--- a/modules/protocol/bahamut.cpp
+++ b/modules/protocol/bahamut.cpp
@@ -379,7 +379,7 @@ struct IRCDMessageNick : IRCDMessage
 			if (signon && signon == stamp)
 				na = NickAlias::Find(params[0]);
 
-			new User(params[0], params[4], params[5], "", params[8], s, params[9], signon, params[3], "", na ? *na->nc : NULL);
+			User::OnIntroduce(params[0], params[4], params[5], "", params[8], s, params[9], signon, params[3], "", na ? *na->nc : NULL);
 		}
 		else
 			source.GetUser()->ChangeNick(params[0]);
diff --git a/modules/protocol/charybdis.cpp b/modules/protocol/charybdis.cpp
index a0c10be83..0801b58f4 100644
--- a/modules/protocol/charybdis.cpp
+++ b/modules/protocol/charybdis.cpp
@@ -225,7 +225,7 @@ struct IRCDMessageEUID : IRCDMessage
 		if (params[9] != "*")
 			na = NickAlias::Find(params[9]);
 
-		new User(params[0], params[4], params[8], params[5], params[6], source.GetServer(), params[10], params[2].is_pos_number_only() ? convertTo<time_t>(params[2]) : Anope::CurTime, params[3], params[7], na ? *na->nc : NULL);
+		User::OnIntroduce(params[0], params[4], params[8], params[5], params[6], source.GetServer(), params[10], params[2].is_pos_number_only() ? convertTo<time_t>(params[2]) : Anope::CurTime, params[3], params[7], na ? *na->nc : NULL);
 	}
 };
 
diff --git a/modules/protocol/hybrid.cpp b/modules/protocol/hybrid.cpp
index 965624264..f2528e304 100644
--- a/modules/protocol/hybrid.cpp
+++ b/modules/protocol/hybrid.cpp
@@ -527,7 +527,7 @@ struct IRCDMessageUID : IRCDMessage
 			na = NickAlias::Find(params[8]);
 
 		/* Source is always the server */
-		new User(params[0], params[4], params[5], "",
+		User::OnIntroduce(params[0], params[4], params[5], "",
 				ip, source.GetServer(),
 				params[9], params[2].is_pos_number_only() ? convertTo<time_t>(params[2]) : 0,
 				params[3], params[7], na ? *na->nc : NULL);
diff --git a/modules/protocol/inspircd12.cpp b/modules/protocol/inspircd12.cpp
index eded20306..0cf3e991d 100644
--- a/modules/protocol/inspircd12.cpp
+++ b/modules/protocol/inspircd12.cpp
@@ -1299,7 +1299,9 @@ struct IRCDMessageUID : IRCDMessage
 					++it;
 			}
 
-		new User(params[2], params[5], params[3], params[4], params[6], source.GetServer(), params[params.size() - 1], ts, modes, params[0], na ? *na->nc : NULL);
+		User *u = User::OnIntroduce(params[2], params[5], params[3], params[4], params[6], source.GetServer(), params[params.size() - 1], ts, modes, params[0], na ? *na->nc : NULL);
+		if (u)
+			u->signon = convertTo<time_t>(params[7]);
 	}
 };
 
diff --git a/modules/protocol/ngircd.cpp b/modules/protocol/ngircd.cpp
index 0d7a0b3bc..cd07866f2 100644
--- a/modules/protocol/ngircd.cpp
+++ b/modules/protocol/ngircd.cpp
@@ -437,7 +437,7 @@ struct IRCDMessageNick : IRCDMessage
 		else if (params.size() == 7)
 		{
 			// a new user is connecting to the network
-			new User(params[0], params[2], params[3], "", "", source.GetServer(), params[6], Anope::CurTime, params[5], "", NULL);
+			User::OnIntroduce(params[0], params[2], params[3], "", "", source.GetServer(), params[6], Anope::CurTime, params[5], "", NULL);
 		}
 		else
 		{
diff --git a/modules/protocol/plexus.cpp b/modules/protocol/plexus.cpp
index 05da5892e..653ec083b 100644
--- a/modules/protocol/plexus.cpp
+++ b/modules/protocol/plexus.cpp
@@ -284,7 +284,7 @@ struct IRCDMessageUID : IRCDMessage
 		if (params[8] != "0" && !na)
 			na = NickAlias::Find(params[8]);
 
-		new User(params[0], params[4], params[9], params[5], ip, source.GetServer(), params[10], ts, params[3], params[7], na ? *na->nc : NULL);
+		User::OnIntroduce(params[0], params[4], params[9], params[5], ip, source.GetServer(), params[10], ts, params[3], params[7], na ? *na->nc : NULL);
 	}
 };
 
diff --git a/modules/protocol/ratbox.cpp b/modules/protocol/ratbox.cpp
index 3c3079ae2..72cdc9573 100644
--- a/modules/protocol/ratbox.cpp
+++ b/modules/protocol/ratbox.cpp
@@ -197,7 +197,7 @@ struct IRCDMessageUID : IRCDMessage
 	void Run(MessageSource &source, const std::vector<Anope::string> &params) anope_override
 	{
 		/* Source is always the server */
-		new User(params[0], params[4], params[5], "", params[6], source.GetServer(), params[8], params[2].is_pos_number_only() ? convertTo<time_t>(params[2]) : 0, params[3], params[7], NULL);
+		User::OnIntroduce(params[0], params[4], params[5], "", params[6], source.GetServer(), params[8], params[2].is_pos_number_only() ? convertTo<time_t>(params[2]) : 0, params[3], params[7], NULL);
 	}
 };
 
diff --git a/modules/protocol/unreal.cpp b/modules/protocol/unreal.cpp
index f0d01ce9d..d7bc8fa14 100644
--- a/modules/protocol/unreal.cpp
+++ b/modules/protocol/unreal.cpp
@@ -855,7 +855,7 @@ struct IRCDMessageNick : IRCDMessage
 				na = NickAlias::Find(params[6]);
 			}
 
-			new User(params[0], params[3], params[4], vhost, ip, s, params[10], user_ts, params[7], "", na ? *na->nc : NULL);
+			User::OnIntroduce(params[0], params[3], params[4], vhost, ip, s, params[10], user_ts, params[7], "", na ? *na->nc : NULL);
 		}
 		else
 			source.GetUser()->ChangeNick(params[0]);
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);