m_spanningtree FJOIN handler: Refactor, update doc

Split the core function into several smaller functions
Accept userlist entries having no comma at all

2 files changed, 128 insertions, 134 deletions

diff --git a/src/modules/m_spanningtree/commands.h b/src/modules/m_spanningtree/commands.h
index 1bca63564..c7dc08b59 100644
--- a/src/modules/m_spanningtree/commands.h
+++ b/src/modules/m_spanningtree/commands.h
@@ -83,16 +83,19 @@ class CommandOpertype : public Command
 	CmdResult Handle (const std::vector<std::string>& parameters, User *user);
 	RouteDescriptor GetRouting(User* user, const std::vector<std::string>& parameters) { return ROUTE_BROADCAST; }
 };
+class TreeSocket;
 class CommandFJoin : public Command
 {
+	/** Remove all modes from a channel, including statusmodes (+qaovh etc), simplemodes, parameter modes.
+	 * This does not update the timestamp of the target channel, this must be done seperately.
+	 */
+	static void RemoveStatus(Channel* c);
+	static void ApplyModeStack(User* srcuser, Channel* c, irc::modestacker& stack);
+	bool ProcessModeUUIDPair(const std::string& item, TreeSocket* src_socket, Channel* chan, irc::modestacker* modestack);
  public:
 	CommandFJoin(Module* Creator) : Command(Creator, "FJOIN", 3) { flags_needed = FLAG_SERVERONLY; }
 	CmdResult Handle (const std::vector<std::string>& parameters, User *user);
 	RouteDescriptor GetRouting(User* user, const std::vector<std::string>& parameters) { return ROUTE_BROADCAST; }
-	/** Remove all modes from a channel, including statusmodes (+qaovh etc), simplemodes, parameter modes.
-	 * This does not update the timestamp of the target channel, this must be done seperately.
-	 */
-	void RemoveStatus(User* source, parameterlist &params);
 };
 class CommandFMode : public Command
 {
diff --git a/src/modules/m_spanningtree/fjoin.cpp b/src/modules/m_spanningtree/fjoin.cpp
index 29ee9244c..be13f5318 100644
--- a/src/modules/m_spanningtree/fjoin.cpp
+++ b/src/modules/m_spanningtree/fjoin.cpp
@@ -29,7 +29,7 @@
 CmdResult CommandFJoin::Handle(const std::vector<std::string>& params, User *srcuser)
 {
 	SpanningTreeUtilities* Utils = ((ModuleSpanningTree*)(Module*)creator)->Utils;
-	/* 1.1 FJOIN works as follows:
+	/* 1.1+ FJOIN works as follows:
 	 *
 	 * Each FJOIN is sent along with a timestamp, and the side with the lowest
 	 * timestamp 'wins'. From this point on we will refer to this side as the
@@ -54,20 +54,23 @@ CmdResult CommandFJoin::Handle(const std::vector<std::string>& params, User *src
 	 * The winning side on the other hand will ignore all user modes from the
 	 * losing side, so only its own modes get applied. Life is simple for those
 	 * who succeed at internets. :-)
+	 *
+	 * Syntax:
+	 * :<sid> FJOIN <chan> <TS> <modes> :[[modes,]<uuid> [[modes,]<uuid> ... ]]
+	 * The last parameter is a list consisting of zero or more (modelist, uuid)
+	 * pairs (permanent channels may have zero users). The mode list for each
+	 * user is a concatenation of the mode letters the user has on the channel
+	 * (e.g.: "ov" if the user is opped and voiced). The order of the mode letters
+	 * are not important but if a server ecounters an unknown mode letter, it will
+	 * drop the link to avoid desync.
+	 *
+	 * InspIRCd 2.0 and older required a comma before the uuid even if the user
+	 * had no prefix modes on the channel, InspIRCd 2.2 and later does not require
+	 * a comma in this case anymore.
+	 *
 	 */
 
-	irc::modestacker modestack(true);			/* Modes to apply from the users in the user list */
-	User* who = NULL;		   				/* User we are currently checking */
-	std::string channel = params[0];				/* Channel name, as a string */
-	time_t TS = atoi(params[1].c_str());    			/* Timestamp given to us for remote side */
-	irc::tokenstream users((params.size() > 3) ? params[params.size() - 1] : "");   /* users from the user list */
-	bool apply_other_sides_modes = true;				/* True if we are accepting the other side's modes */
-	Channel* chan = ServerInstance->FindChan(channel);		/* The channel we're sending joins to */
-	bool created = !chan;						/* True if the channel doesnt exist here yet */
-	std::string item;						/* One item in the list of nicks */
-
-	TreeSocket* src_socket = Utils->FindServer(srcuser->server)->GetRoute()->GetSocket();
-
+	time_t TS = ConvToInt(params[1]);
 	if (!TS)
 	{
 		ServerInstance->Logs->Log("m_spanningtree",LOG_DEFAULT,"*** BUG? *** TS of 0 sent to FJOIN. Are some services authors smoking craq, or is it 1970 again?. Dropped.");
@@ -75,163 +78,151 @@ CmdResult CommandFJoin::Handle(const std::vector<std::string>& params, User *src
 		return CMD_INVALID;
 	}
 
-	if (created)
+	const std::string& channel = params[0];
+	Channel* chan = ServerInstance->FindChan(channel);
+	bool apply_other_sides_modes = true;
+
+	if (!chan)
 	{
 		chan = new Channel(channel, TS);
-		ServerInstance->SNO->WriteToSnoMask('d', "Creation FJOIN received for %s, timestamp: %lu", chan->name.c_str(), (unsigned long)TS);
 	}
 	else
 	{
 		time_t ourTS = chan->age;
-
 		if (TS != ourTS)
+		{
 			ServerInstance->SNO->WriteToSnoMask('d', "Merge FJOIN received for %s, ourTS: %lu, TS: %lu, difference: %lu",
 				chan->name.c_str(), (unsigned long)ourTS, (unsigned long)TS, (unsigned long)(ourTS - TS));
-		/* If our TS is less than theirs, we dont accept their modes */
-		if (ourTS < TS)
-		{
-			ServerInstance->SNO->WriteToSnoMask('d', "NOT Applying modes from other side");
-			apply_other_sides_modes = false;
-		}
-		else if (ourTS > TS)
-		{
-			/* Our TS greater than theirs, clear all our modes from the channel, accept theirs. */
-			ServerInstance->SNO->WriteToSnoMask('d', "Removing our modes, accepting remote");
-			parameterlist param_list;
-			if (Utils->AnnounceTSChange)
-				chan->WriteChannelWithServ(ServerInstance->Config->ServerName, "NOTICE %s :TS for %s changed from %lu to %lu", chan->name.c_str(), channel.c_str(), (unsigned long) ourTS, (unsigned long) TS);
-			// while the name is equal in case-insensitive compare, it might differ in case; use the remote version
-			chan->name = channel;
-			chan->age = TS;
-			chan->ClearInvites();
-			param_list.push_back(channel);
-			this->RemoveStatus(ServerInstance->FakeClient, param_list);
-
-			// XXX: If the channel does not exist in the chan hash at this point, create it so the remote modes can be applied on it.
-			// This happens to 0-user permanent channels on the losing side, because those are removed (from the chan hash, then
-			// deleted later) as soon as the permchan mode is removed from them.
-			if (ServerInstance->FindChan(channel) == NULL)
+			/* If our TS is less than theirs, we dont accept their modes */
+			if (ourTS < TS)
 			{
-				chan = new Channel(channel, TS);
+				apply_other_sides_modes = false;
+			}
+			else if (ourTS > TS)
+			{
+				/* Our TS greater than theirs, clear all our modes from the channel, accept theirs. */
+				if (Utils->AnnounceTSChange)
+					chan->WriteChannelWithServ(ServerInstance->Config->ServerName, "NOTICE %s :TS for %s changed from %lu to %lu", chan->name.c_str(), channel.c_str(), (unsigned long) ourTS, (unsigned long) TS);
+
+				// while the name is equal in case-insensitive compare, it might differ in case; use the remote version
+				chan->name = channel;
+				chan->age = TS;
+				chan->ClearInvites();
+
+				CommandFJoin::RemoveStatus(chan);
+
+				// XXX: If the channel does not exist in the chan hash at this point, create it so the remote modes can be applied on it.
+				// This happens to 0-user permanent channels on the losing side, because those are removed (from the chan hash, then
+				// deleted later) as soon as the permchan mode is removed from them.
+				if (ServerInstance->FindChan(channel) == NULL)
+				{
+					chan = new Channel(channel, TS);
+				}
 			}
 		}
-		// The silent case here is ourTS == TS, we don't need to remove modes here, just to merge them later on.
 	}
 
-	/* First up, apply their modes if they won the TS war */
+	/* First up, apply their channel modes if they won the TS war */
 	if (apply_other_sides_modes)
 	{
-		unsigned int idx = 2;
 		std::vector<std::string> modelist;
+		modelist.push_back(channel);
 
-		// Mode parser needs to know what channel to act on.
-		modelist.push_back(params[0]);
-
-		/* Remember, params[params.size() - 1] is nicklist, and we don't want to apply *that* */
-		for (idx = 2; idx != (params.size() - 1); idx++)
-		{
-			modelist.push_back(params[idx]);
-		}
-
+		/* Remember, params[params.size() - 1] is userlist, and we don't want to apply *that* */
+		modelist.insert(modelist.end(), params.begin()+2, params.end()-1);
 		ServerInstance->SendMode(modelist, srcuser);
 	}
 
-	/* Now, process every 'modes,nick' pair */
+	irc::modestacker modestack(true);
+	TreeSocket* src_socket = Utils->FindServer(srcuser->server)->GetRoute()->GetSocket();
+
+	/* Now, process every 'modes,uuid' pair */
+	irc::tokenstream users(*params.rbegin());
+	std::string item;
+	irc::modestacker* modestackptr = (apply_other_sides_modes ? &modestack : NULL);
 	while (users.GetToken(item))
 	{
-		const char* usr = item.c_str();
-		if (usr && *usr)
-		{
-			const char* unparsedmodes = usr;
-			std::string modes;
-
-
-			/* Iterate through all modes for this user and check they are valid. */
-			while ((*unparsedmodes) && (*unparsedmodes != ','))
-			{
-				ModeHandler *mh = ServerInstance->Modes->FindMode(*unparsedmodes, MODETYPE_CHANNEL);
-				if (!mh)
-				{
-					ServerInstance->Logs->Log("m_spanningtree", LOG_SPARSE, "Unrecognised mode %c, dropping link", *unparsedmodes);
-					return CMD_INVALID;
-				}
-
-				modes += *unparsedmodes;
-				usr++;
-				unparsedmodes++;
-			}
+		if (!ProcessModeUUIDPair(item, src_socket, chan, modestackptr))
+			return CMD_INVALID;
+	}
 
-			/* Advance past the comma, to the nick */
-			usr++;
+	/* Flush mode stacker if we lost the FJOIN or had equal TS */
+	if (apply_other_sides_modes)
+		CommandFJoin::ApplyModeStack(srcuser, chan, modestack);
 
-			/* Check the user actually exists */
-			who = ServerInstance->FindUUID(usr);
-			if (who)
-			{
-				/* Check that the user's 'direction' is correct */
-				TreeServer* route_back_again = Utils->BestRouteTo(who->server);
-				if ((!route_back_again) || (route_back_again->GetSocket() != src_socket))
-					continue;
+	return CMD_SUCCESS;
+}
 
-				/* Add any modes this user had to the mode stack */
-				for (std::string::iterator x = modes.begin(); x != modes.end(); ++x)
-					modestack.Push(*x, who->nick);
+bool CommandFJoin::ProcessModeUUIDPair(const std::string& item, TreeSocket* src_socket, Channel* chan, irc::modestacker* modestack)
+{
+	std::string::size_type comma = item.find(',');
 
-				Channel::JoinUser(who, channel, true, "", route_back_again->bursting, TS);
-			}
-			else
-			{
-				ServerInstance->Logs->Log("m_spanningtree",LOG_SPARSE, "Ignored nonexistant user %s in fjoin to %s (probably quit?)", usr, channel.c_str());
-				continue;
-			}
-		}
+	// Comma not required anymore if the user has no modes
+	std::string uuid = ((comma == std::string::npos) ? item : item.substr(comma+1));
+	User* who = ServerInstance->FindUUID(uuid);
+	if (!who)
+	{
+		// Probably KILLed, ignore
+		return true;
 	}
 
-	/* Flush mode stacker if we lost the FJOIN or had equal TS */
-	if (apply_other_sides_modes)
+	/* Check that the user's 'direction' is correct */
+	SpanningTreeUtilities* Utils = ((ModuleSpanningTree*)(Module*)creator)->Utils;
+	TreeServer* route_back_again = Utils->BestRouteTo(who->server);
+	if ((!route_back_again) || (route_back_again->GetSocket() != src_socket))
 	{
-		parameterlist stackresult;
-		stackresult.push_back(channel);
+		return true;
+	}
 
-		while (modestack.GetStackedLine(stackresult))
+	/* Check if the user received at least one mode */
+	if ((modestack) && (comma > 0) && (comma != std::string::npos))
+	{
+		/* Iterate through the modes and see if they are valid here, if so, apply */
+		std::string::const_iterator commait = item.begin()+comma;
+		for (std::string::const_iterator i = item.begin(); i != commait; ++i)
 		{
-			ServerInstance->SendMode(stackresult, srcuser);
-			stackresult.erase(stackresult.begin() + 1, stackresult.end());
+			if (!ServerInstance->Modes->FindMode(*i, MODETYPE_CHANNEL))
+			{
+				ServerInstance->SNO->WriteToSnoMask('d', "Unrecognised mode '%c' for a user in FJOIN, dropping link", *i);
+				return false;
+			}
+
+			/* Add any modes this user had to the mode stack */
+			modestack->Push(*i, who->nick);
 		}
 	}
-	return CMD_SUCCESS;
+
+	Channel::JoinUser(who, chan->name, true, "", route_back_again->bursting, chan->age);
+	return true;
 }
 
-void CommandFJoin::RemoveStatus(User* srcuser, parameterlist &params)
+void CommandFJoin::RemoveStatus(Channel* c)
 {
-	if (params.size() < 1)
-		return;
-
-	Channel* c = ServerInstance->FindChan(params[0]);
+	irc::modestacker stack(false);
 
-	if (c)
+	for (char modeletter = 'A'; modeletter <= 'z'; ++modeletter)
 	{
-		irc::modestacker stack(false);
-		parameterlist stackresult;
-		stackresult.push_back(c->name);
+		ModeHandler* mh = ServerInstance->Modes->FindMode(modeletter, MODETYPE_CHANNEL);
+
+		/* Passing a pointer to a modestacker here causes the mode to be put onto the mode stack,
+		 * rather than applied immediately. Module unloads require this to be done immediately,
+		 * for this function we require tidyness instead. Fixes bug #493
+		 */
+		if (mh)
+			mh->RemoveMode(c, &stack);
+	}
 
-		for (char modeletter = 'A'; modeletter <= 'z'; ++modeletter)
-		{
-			ModeHandler* mh = ServerInstance->Modes->FindMode(modeletter, MODETYPE_CHANNEL);
-
-			/* Passing a pointer to a modestacker here causes the mode to be put onto the mode stack,
-			 * rather than applied immediately. Module unloads require this to be done immediately,
-			 * for this function we require tidyness instead. Fixes bug #493
-			 */
-			if (mh)
-				mh->RemoveMode(c, &stack);
-		}
+	ApplyModeStack(ServerInstance->FakeClient, c, stack);
+}
 
-		while (stack.GetStackedLine(stackresult))
-		{
-			ServerInstance->SendMode(stackresult, srcuser);
-			stackresult.erase(stackresult.begin() + 1, stackresult.end());
-		}
+void CommandFJoin::ApplyModeStack(User* srcuser, Channel* c, irc::modestacker& stack)
+{
+	parameterlist stackresult;
+	stackresult.push_back(c->name);
+
+	while (stack.GetStackedLine(stackresult))
+	{
+		ServerInstance->SendMode(stackresult, srcuser);
+		stackresult.erase(stackresult.begin() + 1, stackresult.end());
 	}
 }
-