Make more modules rehash atomically (#1535)

Have each module validate the values it loads before setting them, so
any errors don't result in partial application of the configs

8 files changed, 83 insertions, 62 deletions

diff --git a/src/coremods/core_channel/core_channel.cpp b/src/coremods/core_channel/core_channel.cpp
index 05bf113ed..76e220765 100644
--- a/src/coremods/core_channel/core_channel.cpp
+++ b/src/coremods/core_channel/core_channel.cpp
@@ -148,16 +148,6 @@ class CoreModChannel : public Module, public CheckExemption::EventListener
 	void ReadConfig(ConfigStatus& status) CXX11_OVERRIDE
 	{
 		ConfigTag* optionstag = ServerInstance->Config->ConfValue("options");
-		Implementation events[] = { I_OnCheckKey, I_OnCheckLimit, I_OnCheckChannelBan };
-		if (optionstag->getBool("invitebypassmodes", true))
-			ServerInstance->Modules.Attach(events, this, sizeof(events)/sizeof(Implementation));
-		else
-		{
-			for (unsigned int i = 0; i < sizeof(events)/sizeof(Implementation); i++)
-				ServerInstance->Modules.Detach(events[i], this);
-		}
-
-		joinhook.modefromuser = optionstag->getBool("cyclehostsfromuser");
 
 		std::string current;
 		irc::spacesepstream defaultstream(optionstag->getString("exemptchanops"));
@@ -174,26 +164,41 @@ class CoreModChannel : public Module, public CheckExemption::EventListener
 			ServerInstance->Logs->Log(MODNAME, LOG_DEBUG, "Exempting prefix %c from %s", prefix, restriction.c_str());
 			exempts[restriction] = prefix;
 		}
-		exemptions.swap(exempts);
 
 		ConfigTag* securitytag = ServerInstance->Config->ConfValue("security");
 		const std::string announceinvites = securitytag->getString("announceinvites", "dynamic");
+		Invite::AnnounceState newannouncestate;
 		if (stdalgo::string::equalsci(announceinvites, "none"))
-			cmdinvite.announceinvites = Invite::ANNOUNCE_NONE;
+			newannouncestate = Invite::ANNOUNCE_NONE;
 		else if (stdalgo::string::equalsci(announceinvites, "all"))
-			cmdinvite.announceinvites = Invite::ANNOUNCE_ALL;
+			newannouncestate = Invite::ANNOUNCE_ALL;
 		else if (stdalgo::string::equalsci(announceinvites, "ops"))
-			cmdinvite.announceinvites = Invite::ANNOUNCE_OPS;
+			newannouncestate = Invite::ANNOUNCE_OPS;
 		else if (stdalgo::string::equalsci(announceinvites, "dynamic"))
-			cmdinvite.announceinvites = Invite::ANNOUNCE_DYNAMIC;
+			newannouncestate = Invite::ANNOUNCE_DYNAMIC;
 		else
 			throw ModuleException(announceinvites + " is an invalid <security:announceinvites> value, at " + securitytag->getTagLocation());
 
+		// Config is valid, apply it
+
+		// Validates and applies <banlist> tags, so do it first
+		banmode.DoRehash();
+
+		exemptions.swap(exempts);
 		// In 2.0 we allowed limits of 0 to be set. This is non-standard behaviour
 		// and will be removed in the next major release.
-		limitmode.minlimit = optionstag->getBool("allowzerolimit", true) ? 0 : 1;
+		limitmode.minlimit = optionstag->getBool("allowzerolimit", true) ? 0 : 1;;
+		cmdinvite.announceinvites = newannouncestate;
+		joinhook.modefromuser = optionstag->getBool("cyclehostsfromuser");
 
-		banmode.DoRehash();
+		Implementation events[] = { I_OnCheckKey, I_OnCheckLimit, I_OnCheckChannelBan };
+		if (optionstag->getBool("invitebypassmodes", true))
+			ServerInstance->Modules.Attach(events, this, sizeof(events)/sizeof(Implementation));
+		else
+		{
+			for (unsigned int i = 0; i < sizeof(events)/sizeof(Implementation); i++)
+				ServerInstance->Modules.Detach(events[i], this);
+		}
 	}
 
 	void On005Numeric(std::map<std::string, std::string>& tokens) CXX11_OVERRIDE
diff --git a/src/coremods/core_info/core_info.cpp b/src/coremods/core_info/core_info.cpp
index 08e3df9bb..2b56e5e51 100644
--- a/src/coremods/core_info/core_info.cpp
+++ b/src/coremods/core_info/core_info.cpp
@@ -99,18 +99,14 @@ class CoreModInfo : public Module
 
 	void ReadConfig(ConfigStatus& status) CXX11_OVERRIDE
 	{
-		ConfigTag* tag = ServerInstance->Config->ConfValue("admin");
-		cmdadmin.AdminName = tag->getString("name");
-		cmdadmin.AdminEmail = tag->getString("email", "null@example.com");
-		cmdadmin.AdminNick = tag->getString("nick", "admin");
-
 		// Process the escape codes in the MOTDs.
-		cmdmotd.motds.clear();
+		ConfigFileCache newmotds;
 		for (ServerConfig::ClassVector::const_iterator iter = ServerInstance->Config->Classes.begin(); iter != ServerInstance->Config->Classes.end(); ++iter)
 		{
+			ConfigTag* tag = (*iter)->config;
 			// Don't process the file if it has already been processed.
-			const std::string motd = (*iter)->config->getString("motd", "motd");
-			if (cmdmotd.motds.find(motd) != cmdmotd.motds.end())
+			const std::string motd = tag->getString("motd", "motd");
+			if (newmotds.find(motd) != newmotds.end())
 				continue;
 
 			// We can't process the file if it doesn't exist.
@@ -119,10 +115,16 @@ class CoreModInfo : public Module
 				continue;
 
 			// Process escape codes.
-			cmdmotd.motds[file->first] = file->second;
-			InspIRCd::ProcessColors(cmdmotd.motds[file->first]);
+			newmotds[file->first] = file->second;
+			InspIRCd::ProcessColors(newmotds[file->first]);
 		}
 
+		cmdmotd.motds.swap(newmotds);
+
+		ConfigTag* tag = ServerInstance->Config->ConfValue("admin");
+		cmdadmin.AdminName = tag->getString("name");
+		cmdadmin.AdminEmail = tag->getString("email", "null@example.com");
+		cmdadmin.AdminNick = tag->getString("nick", "admin");
 	}
 
 	void OnUserConnect(LocalUser* user) CXX11_OVERRIDE
diff --git a/src/coremods/core_loadmodule.cpp b/src/coremods/core_loadmodule.cpp
index 69c6bbcef..faecab213 100644
--- a/src/coremods/core_loadmodule.cpp
+++ b/src/coremods/core_loadmodule.cpp
@@ -58,10 +58,13 @@ CmdResult CommandLoadmodule::Handle(User* user, const Params& parameters)
 class CommandUnloadmodule : public Command
 {
  public:
+	bool allowcoreunload;
+
 	/** Constructor for unloadmodule.
 	 */
 	CommandUnloadmodule(Module* parent)
-		: Command(parent,"UNLOADMODULE", 1)
+		: Command(parent, "UNLOADMODULE", 1)
+		, allowcoreunload(false)
 	{
 		flags_needed = 'o';
 		syntax = "<modulename>";
@@ -77,8 +80,7 @@ class CommandUnloadmodule : public Command
 
 CmdResult CommandUnloadmodule::Handle(User* user, const Params& parameters)
 {
-	if (!ServerInstance->Config->ConfValue("security")->getBool("allowcoreunload") &&
-		InspIRCd::Match(parameters[0], "core_*.so", ascii_case_insensitive_map))
+	if (!allowcoreunload && InspIRCd::Match(parameters[0], "core_*.so", ascii_case_insensitive_map))
 	{
 		user->WriteNumeric(ERR_CANTUNLOADMODULE, parameters[0], "You cannot unload core commands!");
 		return CMD_FAILURE;
@@ -120,6 +122,12 @@ class CoreModLoadModule : public Module
 	{
 		return Version("Provides the LOADMODULE and UNLOADMODULE commands", VF_VENDOR|VF_CORE);
 	}
+
+	void ReadConfig(ConfigStatus& status) CXX11_OVERRIDE
+	{
+		ConfigTag* tag = ServerInstance->Config->ConfValue("security");
+		cmdunloadmod.allowcoreunload = tag->getBool("allowcoreunload");
+	}
 };
 
 MODULE_INIT(CoreModLoadModule)
diff --git a/src/coremods/core_oper/cmd_die.cpp b/src/coremods/core_oper/cmd_die.cpp
index b25fe2407..8b80dd115 100644
--- a/src/coremods/core_oper/cmd_die.cpp
+++ b/src/coremods/core_oper/cmd_die.cpp
@@ -22,8 +22,9 @@
 #include "exitcodes.h"
 #include "core_oper.h"
 
-CommandDie::CommandDie(Module* parent)
-	: Command(parent, "DIE", 1)
+CommandDie::CommandDie(Module* parent, std::string& hashref)
+	: Command(parent, "DIE", 1, 1)
+	, hash(hashref)
 {
 	flags_needed = 'o';
 	syntax = "<server>";
@@ -61,7 +62,7 @@ void DieRestart::SendError(const std::string& message)
  */
 CmdResult CommandDie::Handle(User* user, const Params& parameters)
 {
-	if (DieRestart::CheckPass(user, parameters[0], "diepass"))
+	if (ServerInstance->PassCompare(user, password, parameters[0], hash))
 	{
 		{
 			std::string diebuf = "*** DIE command from " + user->GetFullHost() + ". Terminating.";
diff --git a/src/coremods/core_oper/cmd_restart.cpp b/src/coremods/core_oper/cmd_restart.cpp
index 936348f95..afadb911a 100644
--- a/src/coremods/core_oper/cmd_restart.cpp
+++ b/src/coremods/core_oper/cmd_restart.cpp
@@ -21,8 +21,9 @@
 #include "inspircd.h"
 #include "core_oper.h"
 
-CommandRestart::CommandRestart(Module* parent)
+CommandRestart::CommandRestart(Module* parent, std::string& hashref)
 	: Command(parent, "RESTART", 1, 1)
+	, hash(hashref)
 {
 	flags_needed = 'o';
 	syntax = "<server>";
@@ -31,7 +32,7 @@ CommandRestart::CommandRestart(Module* parent)
 CmdResult CommandRestart::Handle(User* user, const Params& parameters)
 {
 	ServerInstance->Logs->Log(MODNAME, LOG_DEFAULT, "Restart: %s", user->nick.c_str());
-	if (DieRestart::CheckPass(user, parameters[0], "restartpass"))
+	if (ServerInstance->PassCompare(user, password, parameters[0], hash))
 	{
 		ServerInstance->SNO->WriteGlobalSno('a', "RESTART command from %s, restarting server.", user->GetFullRealHost().c_str());
 
diff --git a/src/coremods/core_oper/core_oper.cpp b/src/coremods/core_oper/core_oper.cpp
index 54814b589..d4afab3b8 100644
--- a/src/coremods/core_oper/core_oper.cpp
+++ b/src/coremods/core_oper/core_oper.cpp
@@ -20,20 +20,10 @@
 #include "inspircd.h"
 #include "core_oper.h"
 
-namespace DieRestart
-{
-	bool CheckPass(User* user, const std::string& inputpass, const char* confentry)
-	{
-		ConfigTag* tag = ServerInstance->Config->ConfValue("power");
-		// The hash method for *BOTH* the die and restart passwords
-		const std::string hash = tag->getString("hash");
-		const std::string correctpass = tag->getString(confentry,  ServerInstance->Config->ServerName);
-		return ServerInstance->PassCompare(user, correctpass, inputpass, hash);
-	}
-}
-
 class CoreModOper : public Module
 {
+	std::string powerhash;
+
 	CommandDie cmddie;
 	CommandKill cmdkill;
 	CommandOper cmdoper;
@@ -42,12 +32,25 @@ class CoreModOper : public Module
 
  public:
 	CoreModOper()
-		: cmddie(this), cmdkill(this), cmdoper(this), cmdrehash(this), cmdrestart(this)
+		: cmddie(this, powerhash)
+		, cmdkill(this)
+		, cmdoper(this)
+		, cmdrehash(this)
+		, cmdrestart(this, powerhash)
 	{
 	}
 
 	void ReadConfig(ConfigStatus& status) CXX11_OVERRIDE
 	{
+		
+		ConfigTag* tag = ServerInstance->Config->ConfValue("power");
+	
+		// The hash method for *BOTH* the die and restart passwords
+		powerhash = tag->getString("hash");
+
+		cmddie.password = tag->getString("diepass", ServerInstance->Config->ServerName, 1);
+		cmdrestart.password = tag->getString("restartpass", ServerInstance->Config->ServerName, 1);
+
 		ConfigTag* security = ServerInstance->Config->ConfValue("security");
 		cmdkill.hidenick = security->getString("hidekills");
 		cmdkill.hideuline = security->getBool("hideulinekills");
@@ -55,7 +58,7 @@ class CoreModOper : public Module
 
 	Version GetVersion() CXX11_OVERRIDE
 	{
-		return Version("Provides the DIE, KILL, OPER, REHASH, and RESTART commands", VF_VENDOR|VF_CORE);
+		return Version("Provides the DIE, KILL, OPER, REHASH, and RESTART commands", VF_VENDOR | VF_CORE);
 	}
 };
 
diff --git a/src/coremods/core_oper/core_oper.h b/src/coremods/core_oper/core_oper.h
index bdb1ae9ee..7589b86d6 100644
--- a/src/coremods/core_oper/core_oper.h
+++ b/src/coremods/core_oper/core_oper.h
@@ -23,14 +23,6 @@
 
 namespace DieRestart
 {
-	/** Checks a die or restart password
-	 * @param user The user executing /DIE or /RESTART
-	 * @param inputpass The password given by the user
-	 * @param confkey The name of the key in the power tag containing the correct password
-	 * @return True if the given password was correct, false if it was not
-	 */
-	bool CheckPass(User* user, const std::string& inputpass, const char* confkey);
-
 	/** Send an ERROR to unregistered users and a NOTICE to all registered local users
 	 * @param message Message to send
 	 */
@@ -42,9 +34,12 @@ namespace DieRestart
 class CommandDie : public Command
 {
  public:
+	std::string& hash;
+	std::string password;
+
 	/** Constructor for die.
 	 */
-	CommandDie(Module* parent);
+	CommandDie(Module* parent, std::string& hashref);
 
 	/** Handle command.
 	 * @param parameters The parameters to the command
@@ -79,6 +74,7 @@ class CommandKill : public Command
 	 * @return A value from CmdResult to indicate command success or failure.
 	 */
 	CmdResult Handle(User* user, const Params& parameters) CXX11_OVERRIDE;
+
 	RouteDescriptor GetRouting(User* user, const Params& parameters) CXX11_OVERRIDE;
 
 	void EncodeParameter(std::string& param, unsigned int index) CXX11_OVERRIDE;
@@ -123,9 +119,12 @@ class CommandRehash : public Command
 class CommandRestart : public Command
 {
  public:
+	std::string& hash;
+	std::string password;
+
 	/** Constructor for restart.
 	 */
-	CommandRestart(Module* parent);
+	CommandRestart(Module* parent, std::string& hashref);
 
 	/** Handle command.
 	 * @param user User issuing the command
diff --git a/src/coremods/core_whois.cpp b/src/coremods/core_whois.cpp
index 455260051..0db634810 100644
--- a/src/coremods/core_whois.cpp
+++ b/src/coremods/core_whois.cpp
@@ -357,17 +357,19 @@ class CoreModWhois : public Module
 	{
 		ConfigTag* tag = ServerInstance->Config->ConfValue("options");
 		const std::string splitwhois = tag->getString("splitwhois", "no");
+		SplitWhoisState newsplitstate;
 		if (stdalgo::string::equalsci(splitwhois, "no"))
-			cmd.splitwhois = SPLITWHOIS_NONE;
+			newsplitstate = SPLITWHOIS_NONE;
 		else if (stdalgo::string::equalsci(splitwhois, "split"))
-			cmd.splitwhois = SPLITWHOIS_SPLIT;
+			newsplitstate = SPLITWHOIS_SPLIT;
 		else if (stdalgo::string::equalsci(splitwhois, "splitmsg"))
-			cmd.splitwhois = SPLITWHOIS_SPLITMSG;
+			newsplitstate = SPLITWHOIS_SPLITMSG;
 		else
 			throw ModuleException(splitwhois + " is an invalid <security:splitwhois> value, at " + tag->getTagLocation()); 
 
 		ConfigTag* security = ServerInstance->Config->ConfValue("security");
 		cmd.genericoper = security->getBool("genericoper");
+		cmd.splitwhois = newsplitstate;
 	}
 
 	Version GetVersion() CXX11_OVERRIDE