Move nick change code out of cmd_nick; fixes double-invocation of hooks and may eliminate bug #911

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

3 files changed, 108 insertions, 157 deletions

diff --git a/include/users.h b/include/users.h
index 51230431a..b71f32748 100644
--- a/include/users.h
+++ b/include/users.h
@@ -528,7 +528,7 @@ class CoreExport User : public Extensible
 	 * @param newnick The nickname to change to
 	 * @return True if the nickchange was successful.
 	 */
-	bool ForceNickChange(const char* newnick);
+	inline bool ForceNickChange(const char* newnick) { return ChangeNick(newnick, true); }
 
 	/** Oper down.
 	 * This will clear the +o usermode and unset the user's oper type
@@ -670,6 +670,13 @@ class CoreExport User : public Extensible
 	 */
 	bool ChangeName(const char* gecos);
 
+	/** Change a user's nick
+	 * @param newnick The new nick
+	 * @param force True if the change is being forced (should not be blocked by modes like +N)
+	 * @return True if the change succeeded
+	 */
+	bool ChangeNick(const std::string& newnick, bool force = false);
+
 	/** Send a command to all local users from this user
 	 * The command given must be able to send text with the
 	 * first parameter as a servermask (e.g. $*), so basically
diff --git a/src/commands/cmd_nick.cpp b/src/commands/cmd_nick.cpp
index 397aea2b7..a69f1ddac 100644
--- a/src/commands/cmd_nick.cpp
+++ b/src/commands/cmd_nick.cpp
@@ -24,7 +24,7 @@ class CommandNick : public Command
  public:
 	/** Constructor for nick.
 	 */
-	CommandNick ( Module* parent) : Command(parent,"NICK", 1, 1) { works_before_reg = true; syntax = "<newnick>"; Penalty = 3; }
+	CommandNick ( Module* parent) : Command(parent,"NICK", 1, 1) { works_before_reg = true; syntax = "<newnick>"; Penalty = 0; }
 	/** Handle command.
 	 * @param parameters The parameters to the comamnd
 	 * @param pcnt The number of parameters passed to teh command
@@ -41,156 +41,39 @@ class CommandNick : public Command
  */
 CmdResult CommandNick::Handle (const std::vector<std::string>& parameters, User *user)
 {
-	std::string oldnick;
+	std::string oldnick = user->nick;
+	std::string newnick = parameters[0];
 
-	if (parameters[0].empty())
-	{
-		/* We cant put blanks in the parameters, so for this (extremely rare) issue we just put '*' here. */
-		user->WriteNumeric(432, "%s * :Erroneous Nickname", user->nick.empty() ? user->nick.c_str() : "*");
-		return CMD_FAILURE;
-	}
+	// anything except the initial NICK gets a flood penalty
+	if (user->registered == REG_ALL && IS_LOCAL(user))
+		IS_LOCAL(user)->CommandFloodPenalty += 4000;
 
-	if (((!ServerInstance->IsNick(parameters[0].c_str(), ServerInstance->Config->Limits.NickMax))) && (IS_LOCAL(user)))
+	if (newnick.empty())
 	{
-		if (!ServerInstance->NICKForced.get(user))
-		{
-			if (parameters[0] == "0")
-			{
-				// Special case, Fake a /nick UIDHERE. Useful for evading "ERR: NICK IN USE" on connect etc.
-				std::vector<std::string> p2;
-				p2.push_back(user->uuid);
-				ServerInstance->NICKForced.set(user, 1);
-				this->Handle(p2, user);
-				ServerInstance->NICKForced.set(user, 0);
-				return CMD_SUCCESS;
-			}
-
-			user->WriteNumeric(432, "%s %s :Erroneous Nickname", user->nick.c_str(),parameters[0].c_str());
-			return CMD_FAILURE;
-		}
+		user->WriteNumeric(432, "%s * :Erroneous Nickname", oldnick.c_str());
+		return CMD_FAILURE;
 	}
 
-	if (assign(user->nick) == parameters[0])
+	if (newnick == "0")
 	{
-		/* If its exactly the same, even case, dont do anything. */
-		if (parameters[0] == user->nick)
-		{
-			return CMD_SUCCESS;
-		}
-
-		/* Its a change of case. People insisted that they should be
-		 * able to do silly things like this even though the RFC says
-		 * the nick AAA is the same as the nick aaa.
-		 */
-		oldnick.assign(user->nick, 0, IS_LOCAL(user) ? ServerInstance->Config->Limits.NickMax : MAXBUF);
-		ModResult MOD_RESULT;
-		FIRST_MOD_RESULT(OnUserPreNick, MOD_RESULT, (user,parameters[0]));
-		if (MOD_RESULT == MOD_RES_DENY)
-			return CMD_FAILURE;
-		if (user->registered == REG_ALL)
-			user->WriteCommon("NICK %s",parameters[0].c_str());
-		user->nick.assign(parameters[0], 0, IS_LOCAL(user) ? ServerInstance->Config->Limits.NickMax : MAXBUF);
-		user->InvalidateCache();
-		FOREACH_MOD(I_OnUserPostNick,OnUserPostNick(user,oldnick));
-		return CMD_SUCCESS;
+		newnick = user->uuid;
 	}
-	else
+	else if (!ServerInstance->IsNick(newnick.c_str(), ServerInstance->Config->Limits.NickMax))
 	{
-		/*
-		 * Don't check Q:Lines if it's a server-enforced change, just on the off-chance some fucking *moron*
-		 * tries to Q:Line SIDs, also, this means we just get our way period, as it really should be.
-		 * Thanks Kein for finding this. -- w00t
-		 *
-		 * Also don't check Q:Lines for remote nickchanges, they should have our Q:Lines anyway to enforce themselves.
-		 *		-- w00t
-		 */
-		if (!IS_LOCAL(user))
-		{
-			XLine* mq = ServerInstance->XLines->MatchesLine("Q",parameters[0]);
-			if (mq)
-			{
-				if (user->registered == REG_ALL)
-				{
-					ServerInstance->SNO->WriteGlobalSno('a', "Q-Lined nickname %s from %s!%s@%s: %s",
-						parameters[0].c_str(), user->nick.c_str(), user->ident.c_str(), user->host.c_str(), mq->reason.c_str());
-				}
-				user->WriteNumeric(432, "%s %s :Invalid nickname: %s",user->nick.c_str(), parameters[0].c_str(), mq->reason.c_str());
-				return CMD_FAILURE;
-			}
-
-			if (ServerInstance->Config->RestrictBannedUsers)
-			{
-				for (UCListIter i = user->chans.begin(); i != user->chans.end(); i++)
-				{
-					Channel *chan = *i;
-					if (chan->GetPrefixValue(user) < VOICE_VALUE && chan->IsBanned(user))
-					{
-						user->WriteNumeric(404, "%s %s :Cannot send to channel (you're banned)", user->nick.c_str(), chan->name.c_str());
-						return CMD_FAILURE;
-					}
-				}
-			}
-		}
-
-		/*
-		 * Uh oh.. if the nickname is in use, and it's not in use by the person using it (doh) --
-		 * then we have a potential collide. Check whether someone else is camping on the nick
-		 * (i.e. connect -> send NICK, don't send USER.) If they are camping, force-change the
-		 * camper to their UID, and allow the incoming nick change.
-		 *
-		 * If the guy using the nick is already using it, tell the incoming nick change to gtfo,
-		 * because the nick is already (rightfully) in use. -- w00t
-		 */
-		User* InUse = ServerInstance->FindNickOnly(parameters[0]);
-		if (InUse && (InUse != user))
-		{
-			if (InUse->registered != REG_ALL)
-			{
-				/* force the camper to their UUID, and ask them to re-send a NICK. */
-				InUse->WriteTo(InUse, "NICK %s", InUse->uuid.c_str());
-				InUse->WriteNumeric(433, "%s %s :Nickname overruled.", InUse->nick.c_str(), InUse->nick.c_str());
-				InUse->UpdateNickHash(InUse->uuid.c_str());
-				InUse->nick.assign(InUse->uuid, 0, IS_LOCAL(InUse) ? ServerInstance->Config->Limits.NickMax : MAXBUF);
-				InUse->InvalidateCache();
-				InUse->registered &= ~REG_NICK;
-			}
-			else
-			{
-				/* No camping, tell the incoming user  to stop trying to change nick ;p */
-				user->WriteNumeric(433, "%s %s :Nickname is already in use.", user->registered >= REG_NICK ? user->nick.c_str() : "*", parameters[0].c_str());
-				return CMD_FAILURE;
-			}
-		}
-	}
-
-
-	ModResult MOD_RESULT;
-	FIRST_MOD_RESULT(OnUserPreNick, MOD_RESULT, (user, parameters[0]));
-	if (MOD_RESULT == MOD_RES_DENY)
-		// if a module returns true, the nick change is silently forbidden.
+		user->WriteNumeric(432, "%s %s :Erroneous Nickname", user->nick.c_str(),newnick.c_str());
 		return CMD_FAILURE;
+	}
 
-	if (user->registered == REG_ALL)
-		user->WriteCommon("NICK %s", parameters[0].c_str());
-
-	oldnick.assign(user->nick, 0, IS_LOCAL(user) ? ServerInstance->Config->Limits.NickMax : MAXBUF);
-
-	/* change the nick of the user in the users_hash */
-	user = user->UpdateNickHash(parameters[0].c_str());
-
-	/* actually change the nick within the record */
-	if (!user)
+	if (!user->ChangeNick(newnick, false))
 		return CMD_FAILURE;
 
-	user->nick.assign(parameters[0], 0, IS_LOCAL(user) ? ServerInstance->Config->Limits.NickMax : MAXBUF);
-	user->InvalidateCache();
-
 	if (user->registered < REG_NICKUSER)
 	{
 		user->registered = (user->registered | REG_NICK);
 		if (user->registered == REG_NICKUSER)
 		{
 			/* user is registered now, bit 0 = USER command, bit 1 = sent a NICK command */
+			ModResult MOD_RESULT;
 			FIRST_MOD_RESULT(OnUserRegister, MOD_RESULT, (IS_LOCAL(user)));
 			if (MOD_RESULT == MOD_RES_DENY)
 				return CMD_FAILURE;
@@ -200,15 +83,7 @@ CmdResult CommandNick::Handle (const std::vector<std::string>& parameters, User
 		}
 	}
 
-	if (user->registered == REG_ALL)
-	{
-		if (IS_LOCAL(user))
-			IS_LOCAL(user)->CommandFloodPenalty += 5000;
-		FOREACH_MOD(I_OnUserPostNick,OnUserPostNick(user, oldnick));
-	}
-
 	return CMD_SUCCESS;
-
 }
 
 
diff --git a/src/users.cpp b/src/users.cpp
index 48a8ee87a..f429597da 100644
--- a/src/users.cpp
+++ b/src/users.cpp
@@ -894,13 +894,12 @@ void User::InvalidateCache()
 	cached_fullrealhost.clear();
 }
 
-bool User::ForceNickChange(const char* newnick)
+bool User::ChangeNick(const std::string& newnick, bool force)
 {
 	ModResult MOD_RESULT;
 
-	this->InvalidateCache();
-
-	ServerInstance->NICKForced.set(this, 1);
+	if (force)
+		ServerInstance->NICKForced.set(this, 1);
 	FIRST_MOD_RESULT(OnUserPreNick, MOD_RESULT, (this, newnick));
 	ServerInstance->NICKForced.set(this, 0);
 
@@ -910,20 +909,90 @@ bool User::ForceNickChange(const char* newnick)
 		return false;
 	}
 
-	std::deque<classbase*> dummy;
-	Command* nickhandler = ServerInstance->Parser->GetHandler("NICK");
-	if (nickhandler) // wtfbbq, when would this not be here
+	if (assign(newnick) == assign(nick))
 	{
-		std::vector<std::string> parameters;
-		parameters.push_back(newnick);
-		ServerInstance->NICKForced.set(this, 1);
-		bool result = (ServerInstance->Parser->CallHandler("NICK", parameters, this) == CMD_SUCCESS);
-		ServerInstance->NICKForced.set(this, 0);
-		return result;
+		// case change, don't need to check Q:lines and such
+		// and, if it's identical including case, we can leave right now
+		if (newnick == nick)
+			return true;
 	}
+	else
+	{
+		/*
+		 * Don't check Q:Lines if it's a server-enforced change, just on the off-chance some fucking *moron*
+		 * tries to Q:Line SIDs, also, this means we just get our way period, as it really should be.
+		 * Thanks Kein for finding this. -- w00t
+		 *
+		 * Also don't check Q:Lines for remote nickchanges, they should have our Q:Lines anyway to enforce themselves.
+		 *		-- w00t
+		 */
+		if (!IS_LOCAL(this))
+		{
+			XLine* mq = ServerInstance->XLines->MatchesLine("Q",newnick);
+			if (mq)
+			{
+				if (this->registered == REG_ALL)
+				{
+					ServerInstance->SNO->WriteGlobalSno('a', "Q-Lined nickname %s from %s!%s@%s: %s",
+						newnick.c_str(), this->nick.c_str(), this->ident.c_str(), this->host.c_str(), mq->reason.c_str());
+				}
+				this->WriteNumeric(432, "%s %s :Invalid nickname: %s",this->nick.c_str(), newnick.c_str(), mq->reason.c_str());
+				return false;
+			}
 
-	// Unreachable, we hope
-	return false;
+			if (ServerInstance->Config->RestrictBannedUsers)
+			{
+				for (UCListIter i = this->chans.begin(); i != this->chans.end(); i++)
+				{
+					Channel *chan = *i;
+					if (chan->GetPrefixValue(this) < VOICE_VALUE && chan->IsBanned(this))
+					{
+						this->WriteNumeric(404, "%s %s :Cannot send to channel (you're banned)", this->nick.c_str(), chan->name.c_str());
+						return false;
+					}
+				}
+			}
+		}
+
+		/*
+		 * Uh oh.. if the nickname is in use, and it's not in use by the person using it (doh) --
+		 * then we have a potential collide. Check whether someone else is camping on the nick
+		 * (i.e. connect -> send NICK, don't send USER.) If they are camping, force-change the
+		 * camper to their UID, and allow the incoming nick change.
+		 *
+		 * If the guy using the nick is already using it, tell the incoming nick change to gtfo,
+		 * because the nick is already (rightfully) in use. -- w00t
+		 */
+		User* InUse = ServerInstance->FindNickOnly(newnick);
+		if (InUse && (InUse != this))
+		{
+			if (InUse->registered != REG_ALL)
+			{
+				/* force the camper to their UUID, and ask them to re-send a NICK. */
+				InUse->WriteTo(InUse, "NICK %s", InUse->uuid.c_str());
+				InUse->WriteNumeric(433, "%s %s :Nickname overruled.", InUse->nick.c_str(), InUse->nick.c_str());
+				InUse->UpdateNickHash(InUse->uuid.c_str());
+				InUse->nick.assign(InUse->uuid, 0, IS_LOCAL(InUse) ? ServerInstance->Config->Limits.NickMax : MAXBUF);
+				InUse->InvalidateCache();
+				InUse->registered &= ~REG_NICK;
+			}
+			else
+			{
+				/* No camping, tell the incoming user  to stop trying to change nick ;p */
+				this->WriteNumeric(433, "%s %s :Nickname is already in use.", this->registered >= REG_NICK ? this->nick.c_str() : "*", newnick.c_str());
+				return CMD_FAILURE;
+			}
+		}
+	}
+
+	if (this->registered == REG_ALL)
+		this->WriteCommon("NICK %s",newnick.c_str());
+	std::string oldnick = nick;
+	nick = newnick;
+	InvalidateCache();
+	UpdateNickHash(newnick.c_str());
+	FOREACH_MOD(I_OnUserPostNick,OnUserPostNick(this,oldnick));
+	return true;
 }
 
 int LocalUser::GetServerPort()