Send SVSNICKs during nick collision to prevent servers that do not fully implement collisions from killing users

git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@11179 e03df62e-2008-0410-955e-edbf42e46eb7

3 files changed, 35 insertions, 24 deletions

diff --git a/src/modules/m_spanningtree/nickcollide.cpp b/src/modules/m_spanningtree/nickcollide.cpp
index 6c4a837a5..9656f9af3 100644
--- a/src/modules/m_spanningtree/nickcollide.cpp
+++ b/src/modules/m_spanningtree/nickcollide.cpp
@@ -30,8 +30,8 @@
 int TreeSocket::DoCollision(User *u, time_t remotets, const std::string &remoteident, const std::string &remoteip, const std::string &remoteuid)
 {
 	/*
-	 *  Under old protocol rules, we would have had to kill both clients.
-	 *  Really, this sucks.
+	 * Under old protocol rules, we would have had to kill both clients.
+	 * Really, this sucks.
 	 * These days, we have UID. And, so what we do is, force nick change client(s)
 	 * involved according to timestamp rules.
 	 *
@@ -84,9 +84,30 @@ int TreeSocket::DoCollision(User *u, time_t remotets, const std::string &remotei
 		}
 	}
 
+	/*
+	 * Cheat a little here. Instead of a dedicated command to change UID,
+	 * use SVSNICK and accept the losing client with its UID (as we know the SVSNICK will
+	 * not fail under any circumstances -- UIDs are netwide exclusive).
+	 *
+	 * This means that each side of a collide will generate one extra NICK back to where
+	 * they have just linked (and where it got the SVSNICK from), however, it will
+	 * be dropped harmlessly as it will come in as :928AAAB NICK 928AAAB, and we already
+	 * have 928AAAB's nick set to that.
+	 *   -- w00t
+	 */
 
 	if (bChangeLocal)
 	{
+		/*
+		 * Local-side nick needs to change. Just in case we are hub, and
+		 * this "local" nick is actually behind us, send an SVSNICK out.
+		 */
+		std::deque<std::string> params;
+		params.push_back(u->uuid);
+		params.push_back(u->uuid);
+		params.push_back(ConvToStr(u->age));
+		Utils->DoOneToMany(ServerInstance->Config->GetSID(),"SVSNICK",params);
+
 		u->ForceNickChange(u->uuid.c_str());
 
 		if (!bChangeRemote)
@@ -94,28 +115,18 @@ int TreeSocket::DoCollision(User *u, time_t remotets, const std::string &remotei
 	}
 	if (bChangeRemote)
 	{
+		User *remote = this->ServerInstance->FindUUID(remoteuid);
 		/*
-		 * Cheat a little here. Instead of a dedicated command to change UID,
-		 * use SVSNICK and accept their client with it's UID (as we know the SVSNICK will
-		 * not fail under any circumstances -- UIDs are netwide exclusive).
-		 *
-		 * This means that each side of a collide will generate one extra NICK back to where
-		 * they have just linked (and where it got the SVSNICK from), however, it will
-		 * be dropped harmlessly as it will come in as :928AAAB NICK 928AAAB, and we already
-		 * have 928AAAB's nick set to that.
-		 *   -- w00t
+		 * remote side needs to change. If this happens, we will modify
+		 * the UID or halt the propagation of the nick change command,
+		 * so other servers don't need to see the SVSNICK
 		 */
-		User *remote = this->ServerInstance->FindUUID(remoteuid);
+		WriteLine(std::string(":")+ServerInstance->Config->GetSID()+" SVSNICK "+remoteuid+" " + remoteuid + " " + ConvToStr(remotets));
 
 		if (remote)
 		{
-			/* buh.. nick change collide. force change their nick. */
-			remote->ForceNickChange(remote->uuid.c_str());
-		}
-		else
-		{
-			/* user has not been introduced yet, just inform their server */
-			this->WriteLine(std::string(":")+this->ServerInstance->Config->GetSID()+" SVSNICK "+remoteuid+" " + remoteuid + " " + ConvToStr(remotets));
+			/* nick change collide. Force change their nick. */
+			remote->ForceNickChange(remoteuid.c_str());
 		}
 
 		if (!bChangeLocal)
diff --git a/src/modules/m_spanningtree/treesocket2.cpp b/src/modules/m_spanningtree/treesocket2.cpp
index 0fa8dd31a..22dcf293d 100644
--- a/src/modules/m_spanningtree/treesocket2.cpp
+++ b/src/modules/m_spanningtree/treesocket2.cpp
@@ -596,9 +596,9 @@ bool TreeSocket::ProcessLine(std::string &line)
 						if (collideret != 1)
 						{
 							/*
-							 * Remote client lost, or both lost, parsing this nickchange would be
-							 * pointless, as the incoming client's server will soon recieve SVSNICK to
-							 * change its nick to its UID. :) -- w00t
+							 * Remote client lost, or both lost, parsing or passing on this
+							 * nickchange would be pointless, as the incoming client's server will
+							 * soon recieve SVSNICK to change its nick to its UID. :) -- w00t
 							 */
 							return true;
 						}
diff --git a/src/modules/m_spanningtree/uid.cpp b/src/modules/m_spanningtree/uid.cpp
index 749922c62..58a4a0e9d 100644
--- a/src/modules/m_spanningtree/uid.cpp
+++ b/src/modules/m_spanningtree/uid.cpp
@@ -71,10 +71,10 @@ bool TreeSocket::ParseUID(const std::string &source, std::deque<std::string> &pa
 		/*
 		 * Nick collision.
 		 */
-		ServerInstance->Logs->Log("m_spanningtree",DEBUG,"*** Collision on %s", params[2].c_str());
 		int collide = this->DoCollision(iter->second, age_t, params[5], params[8], params[0]);
+		ServerInstance->Logs->Log("m_spanningtree",DEBUG,"*** Collision on %s, collide=%d", params[2].c_str(), collide);
 
-		if (collide == 2)
+		if (collide != 1)
 		{
 			/* remote client changed, make sure we change their nick for the hash too */
 			params[2] = params[0];