m_spanningtree Propagate topic changes via FTOPIC in order to prevent desync when two TOPIC messages cross

TOPIC is no longer accepted from servers using the new protocol

7 files changed, 80 insertions, 42 deletions

diff --git a/src/commands/cmd_topic.cpp b/src/commands/cmd_topic.cpp
index 997fb6a91..8f5979865 100644
--- a/src/commands/cmd_topic.cpp
+++ b/src/commands/cmd_topic.cpp
@@ -27,7 +27,7 @@
  * the same way, however, they can be fully unloaded, where these
  * may not.
  */
-class CommandTopic : public Command
+class CommandTopic : public SplitCommand
 {
 	ChanModeReference secretmode;
 	ChanModeReference topiclockmode;
@@ -36,7 +36,7 @@ class CommandTopic : public Command
 	/** Constructor for topic.
 	 */
 	CommandTopic(Module* parent)
-		: Command(parent, "TOPIC", 1, 2)
+		: SplitCommand(parent, "TOPIC", 1, 2)
 		, secretmode(parent, "secret")
 		, topiclockmode(parent, "topiclock")
 	{
@@ -50,14 +50,10 @@ class CommandTopic : public Command
 	 * @param user The user issuing the command
 	 * @return A value from CmdResult to indicate command success or failure.
 	 */
-	CmdResult Handle(const std::vector<std::string>& parameters, User *user);
-	RouteDescriptor GetRouting(User* user, const std::vector<std::string>& parameters)
-	{
-		return (IS_LOCAL(user) ? ROUTE_LOCALONLY : ROUTE_BROADCAST);
-	}
+	CmdResult HandleLocal(const std::vector<std::string>& parameters, LocalUser* user);
 };
 
-CmdResult CommandTopic::Handle (const std::vector<std::string>& parameters, User *user)
+CmdResult CommandTopic::HandleLocal(const std::vector<std::string>& parameters, LocalUser* user)
 {
 	Channel* c = ServerInstance->FindChan(parameters[0]);
 	if (!c)
@@ -89,13 +85,6 @@ CmdResult CommandTopic::Handle (const std::vector<std::string>& parameters, User
 		return CMD_SUCCESS;
 	}
 
-	// Access checks are skipped for non-local users
-	if (!IS_LOCAL(user))
-	{
-		c->SetTopic(user, parameters[1]);
-		return CMD_SUCCESS;
-	}
-
 	std::string t = parameters[1]; // needed, in case a module wants to change it
 	ModResult res;
 	FIRST_MOD_RESULT(OnPreTopicChange, res, (user,c,t));
diff --git a/src/modules/m_spanningtree/commands.h b/src/modules/m_spanningtree/commands.h
index 007c0f2b0..f15708a9a 100644
--- a/src/modules/m_spanningtree/commands.h
+++ b/src/modules/m_spanningtree/commands.h
@@ -145,8 +145,15 @@ class CommandFMode : public ServerCommand
 class CommandFTopic : public ServerCommand
 {
  public:
-	CommandFTopic(Module* Creator) : ServerCommand(Creator, "FTOPIC", 5) { }
+	CommandFTopic(Module* Creator) : ServerCommand(Creator, "FTOPIC", 4, 5) { }
 	CmdResult Handle(User* user, std::vector<std::string>& params);
+
+	class Builder : public CmdBuilder
+	{
+	 public:
+		Builder(Channel* chan);
+		Builder(User* user, Channel* chan);
+	};
 };
 
 class CommandFHost : public UserOnlyServerCommand<CommandFHost>
diff --git a/src/modules/m_spanningtree/compat.cpp b/src/modules/m_spanningtree/compat.cpp
index a9e48a10c..02e11c849 100644
--- a/src/modules/m_spanningtree/compat.cpp
+++ b/src/modules/m_spanningtree/compat.cpp
@@ -123,8 +123,10 @@ void TreeSocket::WriteLine(const std::string& original_line)
 				else if (command == "FTOPIC")
 				{
 					// Drop channel TS for FTOPIC
-					// :sid FTOPIC #target TS TopicTS ...
-					//     A      B       C  D
+					// :sid FTOPIC #target TS TopicTS setter :newtopic
+					//     A      B       C  D       E      F
+					// :uid FTOPIC #target TS TopicTS :newtopic
+					//     A      B       C  D       E
 					if (b == std::string::npos)
 						return;
 
@@ -136,7 +138,14 @@ void TreeSocket::WriteLine(const std::string& original_line)
 					if (d == std::string::npos)
 						return;
 
-					line.erase(c, d-c);
+					std::string::size_type e = line.find(' ', d + 1);
+					if (line[e+1] == ':')
+					{
+						line.erase(c, e-c);
+						line.erase(a+1, 1);
+					}
+					else
+						line.erase(c, d-c);
 				}
 				else if ((command == "PING") || (command == "PONG"))
 				{
@@ -268,6 +277,15 @@ bool TreeSocket::PreProcessOldProtocolMessage(User*& who, std::string& cmd, std:
 		params.insert(params.begin()+1, "operquit");
 		who = MyRoot->ServerUser;
 	}
+	else if ((cmd == "TOPIC") && (params.size() >= 2))
+	{
+		// :20DAAAAAC TOPIC #chan :new topic
+		cmd = "FTOPIC";
+		if (!InsertCurrentChannelTS(params))
+			return false;
+
+		params.insert(params.begin()+2, ConvToStr(ServerInstance->Time()));
+	}
 
 	return true; // Passthru
 }
diff --git a/src/modules/m_spanningtree/ftopic.cpp b/src/modules/m_spanningtree/ftopic.cpp
index 0a4a95f9e..bd15489a2 100644
--- a/src/modules/m_spanningtree/ftopic.cpp
+++ b/src/modules/m_spanningtree/ftopic.cpp
@@ -44,26 +44,65 @@ CmdResult CommandFTopic::Handle(User* user, std::vector<std::string>& params)
 	if (ts < c->topicset)
 		return CMD_FAILURE;
 
+	// The topic text is always the last parameter
+	const std::string& newtopic = params.back();
+
+	// If there is a setter in the message use that, otherwise use the message source
+	const std::string& setter = ((params.size() > 4) ? params[3] : (ServerInstance->Config->FullHostInTopic ? user->GetFullHost() : user->nick));
+
 	/*
 	 * If the topics were updated at the exact same second, accept
 	 * the remote only when it's "bigger" than ours as defined by
 	 * string comparision, so non-empty topics always overridde
 	 * empty topics if their timestamps are equal
+	 *
+	 * Similarly, if the topic texts are equal too, keep one topic
+	 * setter and discard the other
 	 */
-	if ((ts == c->topicset) && (c->topic > params[4]))
-		return CMD_FAILURE; // Topics were set at the exact same time, keep our topic and setter
+	if (ts == c->topicset)
+	{
+		// Discard if their topic text is "smaller"
+		if (c->topic > newtopic)
+			return CMD_FAILURE;
 
-	if (c->topic != params[4])
+		// If the texts are equal in addition to the timestamps, decide which setter to keep
+		if ((c->topic == newtopic) && (c->setby >= setter))
+			return CMD_FAILURE;
+	}
+
+	if (c->topic != newtopic)
 	{
 		// Update topic only when it differs from current topic
-		c->topic.assign(params[4], 0, ServerInstance->Config->Limits.MaxTopic);
+		c->topic.assign(newtopic, 0, ServerInstance->Config->Limits.MaxTopic);
 		c->WriteChannel(user, "TOPIC %s :%s", c->name.c_str(), c->topic.c_str());
 	}
 
 	// Update setter and settime
-	c->setby.assign(params[3], 0, 127);
+	c->setby.assign(setter, 0, 128);
 	c->topicset = ts;
 
+	FOREACH_MOD(OnPostTopicChange, (user, c, c->topic));
+
 	return CMD_SUCCESS;
 }
 
+// Used when bursting and in reply to RESYNC, contains topic setter as the 4th parameter
+CommandFTopic::Builder::Builder(Channel* chan)
+	: CmdBuilder("FTOPIC")
+{
+	push(chan->name);
+	push_int(chan->age);
+	push_int(chan->topicset);
+	push(chan->setby);
+	push_last(chan->topic);
+}
+
+// Used when changing the topic, the setter is the message source
+CommandFTopic::Builder::Builder(User* user, Channel* chan)
+	: CmdBuilder(user, "FTOPIC")
+{
+	push(chan->name);
+	push_int(chan->age);
+	push_int(chan->topicset);
+	push_last(chan->topic);
+}
diff --git a/src/modules/m_spanningtree/main.cpp b/src/modules/m_spanningtree/main.cpp
index f458c2d2f..98f9a304b 100644
--- a/src/modules/m_spanningtree/main.cpp
+++ b/src/modules/m_spanningtree/main.cpp
@@ -422,10 +422,7 @@ void ModuleSpanningTree::OnPostTopicChange(User* user, Channel* chan, const std:
 	if (!IS_LOCAL(user))
 		return;
 
-	CmdBuilder params(user->uuid, "TOPIC");
-	params.push_back(chan->name);
-	params.push_last(topic);
-	params.Broadcast();
+	CommandFTopic::Builder(user, chan).Broadcast();
 }
 
 void ModuleSpanningTree::OnUserMessage(User* user, void* dest, int target_type, const std::string& text, char status, const CUList& exempt_list, MessageType msgtype)
diff --git a/src/modules/m_spanningtree/netburst.cpp b/src/modules/m_spanningtree/netburst.cpp
index 2cd107972..d4669442a 100644
--- a/src/modules/m_spanningtree/netburst.cpp
+++ b/src/modules/m_spanningtree/netburst.cpp
@@ -147,11 +147,7 @@ void TreeSocket::SyncChannel(Channel* chan)
 	// If the topic was ever set, send it, even if it's empty now
 	// because a new empty topic should override an old non-empty topic
 	if (chan->topicset != 0)
-	{
-		this->WriteLine(InspIRCd::Format(":%s FTOPIC %s %lu %lu %s :%s", ServerInstance->Config->GetSID().c_str(),
-			chan->name.c_str(), (unsigned long)chan->age, (unsigned long)chan->topicset,
-			chan->setby.c_str(), chan->topic.c_str()));
-	}
+		this->WriteLine(CommandFTopic::Builder(chan));
 
 	for (Extensible::ExtensibleStore::const_iterator i = chan->GetExtList().begin(); i != chan->GetExtList().end(); i++)
 	{
diff --git a/src/modules/m_spanningtree/protocolinterface.cpp b/src/modules/m_spanningtree/protocolinterface.cpp
index afd463360..013dfac1b 100644
--- a/src/modules/m_spanningtree/protocolinterface.cpp
+++ b/src/modules/m_spanningtree/protocolinterface.cpp
@@ -70,15 +70,7 @@ void SpanningTreeProtocolInterface::SendMetaData(Extensible* target, const std::
 
 void SpanningTreeProtocolInterface::SendTopic(Channel* channel, std::string &topic)
 {
-	CmdBuilder params("FTOPIC");
-
-	params.push_back(channel->name);
-	params.push_back(ConvToStr(channel->age));
-	params.push_back(ConvToStr(ServerInstance->Time()));
-	params.push_back(ServerInstance->Config->ServerName);
-	params.push_last(topic);
-
-	params.Broadcast();
+	CommandFTopic::Builder(ServerInstance->FakeClient, channel).Broadcast();
 }
 
 void SpanningTreeProtocolInterface::SendMode(User* source, User* u, Channel* c, const std::vector<std::string>& modedata, const std::vector<TranslateType>& translate)