Fix for bug that took ages to track down and was very subtle.

During authentication the flow of commands is as follows:
> SERVER aaaaaa
< SERVER bbbbbb
> BURST
< BURST
... what can happen is that between the two server commands we can introduce SERVER aaaa to server bbbbb again, from a different socket. As server aaaaa doesnt exist yet and we're waiting for it to say 
yes or no to our own SERVER command over at bbbbbb, it cant be found with FindServer.
Therefore we need a second list of servers that arent yet authenticated, but are waiting TO authenticate, by the pointer their socket has and the name they want to become after auth.
If two servers introduce themselves at the same time, triggering what was the race condition, both servers are disconnected with "ERROR :Negotiation collision" and must try again until only one succeeds.


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

4 files changed, 62 insertions, 0 deletions

diff --git a/src/modules/m_spanningtree/treesocket1.cpp b/src/modules/m_spanningtree/treesocket1.cpp
index 1dede8b31..ddb8cf70e 100644
--- a/src/modules/m_spanningtree/treesocket1.cpp
+++ b/src/modules/m_spanningtree/treesocket1.cpp
@@ -93,6 +93,8 @@ TreeSocket::~TreeSocket()
 {
 	if (Hook)
 		InspSocketUnhookRequest(this, (Module*)Utils->Creator, Hook).Send();
+
+	Utils->DelBurstingServer(this);
 }
 
 const std::string& TreeSocket::GetOurChallenge()
diff --git a/src/modules/m_spanningtree/treesocket2.cpp b/src/modules/m_spanningtree/treesocket2.cpp
index fa1e8ef15..c87e5d406 100644
--- a/src/modules/m_spanningtree/treesocket2.cpp
+++ b/src/modules/m_spanningtree/treesocket2.cpp
@@ -903,6 +903,20 @@ bool TreeSocket::Inbound_Server(std::deque<std::string> &params)
 	{
 		if ((x->Name == servername) && ((ComparePass(this->MakePass(x->RecvPass,this->GetOurChallenge()),password) || x->RecvPass == password && (this->GetTheirChallenge().empty()))))
 		{
+			/* First check for instances of the server that are waiting between the inbound and outbound SERVER command */
+			TreeSocket* CheckDupeSocket = Utils->FindBurstingServer(sname);
+			if (CheckDupeSocket)
+			{
+				/* If we find one, we abort the link to prevent a race condition */
+				this->WriteLine("ERROR :Negotiation collision");
+				this->Instance->SNO->WriteToSnoMask('l',"Server connection from \2"+sname+"\2 denied, already exists in a negotiating state.");
+				CheckDupeSocket->WriteLine("ERROR :Negotiation collision");
+				Instance->SE->DelFd(CheckDupeSocket);
+				CheckDupeSocket->Close();
+				delete CheckDupeSocket;
+				return false;
+			}
+			/* Now check for fully initialized instances of the server */
 			TreeServer* CheckDupe = Utils->FindServer(sname);
 			if (CheckDupe)
 			{
@@ -917,6 +931,8 @@ bool TreeSocket::Inbound_Server(std::deque<std::string> &params)
 				this->Instance->SNO->WriteToSnoMask('l',"Connection from \2"+sname+"\2["+(x->HiddenFromStats ? "<hidden>" : this->GetIP())+"] using transport \2"+name+"\2");
 			}
 
+			Utils->AddBurstingServer(sname,this);
+
 			this->InboundServerName = sname;
 			this->InboundDescription = description;
 			// this is good. Send our details: Our server name and description and hopcount of 0,
@@ -1050,6 +1066,7 @@ bool TreeSocket::ProcessLine(std::string &line)
 				this->LinkState = CONNECTED;
 				Link* lnk = Utils->FindLink(InboundServerName);
 				Node = new TreeServer(this->Utils,this->Instance, InboundServerName, InboundDescription, Utils->TreeRoot, this, lnk ? lnk->Hidden : false);
+				Utils->DelBurstingServer(this);
 				Utils->TreeRoot->AddChild(Node);
 				params.clear();
 				params.push_back(InboundServerName);
diff --git a/src/modules/m_spanningtree/utils.cpp b/src/modules/m_spanningtree/utils.cpp
index 671fad7c3..923308de0 100644
--- a/src/modules/m_spanningtree/utils.cpp
+++ b/src/modules/m_spanningtree/utils.cpp
@@ -53,6 +53,40 @@ TreeServer* SpanningTreeUtilities::FindServer(const std::string &ServerName)
 	}
 }
 
+TreeSocket* SpanningTreeUtilities::FindBurstingServer(const std::string &ServerName)
+{
+	std::map<irc::string,TreeSocket*>::iterator iter;
+	iter = burstingserverlist.find(ServerName.c_str());
+	if (iter != burstingserverlist.end())
+	{
+		return iter->second;
+	}
+	else
+	{
+		return NULL;
+	}
+}
+
+void SpanningTreeUtilities::AddBurstingServer(const std::string &ServerName, TreeSocket* s)
+{
+	std::map<irc::string,TreeSocket*>::iterator iter;
+	iter = burstingserverlist.find(ServerName.c_str());
+	if (iter == burstingserverlist.end())
+		burstingserverlist[ServerName.c_str()] = s;
+}
+
+void SpanningTreeUtilities::DelBurstingServer(TreeSocket* s)
+{
+	 for (std::map<irc::string,TreeSocket*>::iterator iter = burstingserverlist.begin(); iter != burstingserverlist.end(); iter++)
+	 {
+		 if (iter->second == s)
+		 {
+			 burstingserverlist.erase(iter);
+			 return;
+		 }
+	 }
+}
+
 /** Returns the locally connected server we must route a
  * message through to reach server 'ServerName'. This
  * only applies to one-to-one and not one-to-many routing.
diff --git a/src/modules/m_spanningtree/utils.h b/src/modules/m_spanningtree/utils.h
index 70ce1eb13..2fd53bb43 100644
--- a/src/modules/m_spanningtree/utils.h
+++ b/src/modules/m_spanningtree/utils.h
@@ -75,6 +75,9 @@ class SpanningTreeUtilities
 	/** Hash of currently connected servers by name
 	 */
 	server_hash serverlist;
+	/** Hash of servers currently bursting but not initialized as connected
+	 */
+	std::map<irc::string,TreeSocket*> burstingserverlist;
 	/** Holds the data from the <link> tags in the conf
 	 */
 	std::vector<Link> LinkBlocks;
@@ -160,6 +163,12 @@ class SpanningTreeUtilities
 	/** Refresh the IP cache used for allowing inbound connections
 	 */
 	void RefreshIPCache();
+
+	TreeSocket* FindBurstingServer(const std::string &ServerName);
+
+	void AddBurstingServer(const std::string &ServerName, TreeSocket* s);
+
+	void DelBurstingServer(TreeSocket* s);
 };
 
 #endif