Add an optional parameter to ConfigReader::ReadValue which defines if we want linefeeds or not.

Tidy up some duplicated code in configreader (error reporting)
Change ModuleException to inherit from CoreException, have the configreader throw and catch its CoreException on fatal error


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

4 files changed, 180 insertions, 156 deletions

diff --git a/include/configreader.h b/include/configreader.h
index 115f205c5..98a13034d 100644
--- a/include/configreader.h
+++ b/include/configreader.h
@@ -30,7 +30,14 @@ class InspSocket;
 
 /** Types of data in the core config
  */
-enum ConfigDataType { DT_NOTHING, DT_INTEGER, DT_CHARPTR, DT_BOOLEAN };
+enum ConfigDataType
+{
+	DT_NOTHING       = 0,
+	DT_INTEGER       = 1,
+	DT_CHARPTR       = 2,
+	DT_BOOLEAN       = 3,
+	DT_ALLOW_NEWLINE = 128
+};
 
 /** Holds a config value, either string, integer or boolean.
  * Callback functions receive one or more of these, either on
@@ -522,7 +529,20 @@ class ServerConfig : public Extensible
 	/** Load 'filename' into 'target', with the new config parser everything is parsed into
 	 * tag/key/value at load-time rather than at read-value time.
 	 */
+
+	/** Report a configuration error given in errormessage.
+	 * @param bail If this is set to true, the error is sent to the console, and the program exits
+	 * @param user If this is set to a non-null value, and bail is false, the errors are spooled to
+	 * this user as SNOTICEs.
+	 * If the parameter is NULL, the messages are spooled to all users via WriteOpers as SNOTICEs.
+	 */
+	void ReportConfigError(const std::string &errormessage, bool bail, userrec* user);
+
+	/** Load 'filename' into 'target', with the new config parser everything is parsed into
+	 * tag/key/value at load-time rather than at read-value time.
+	 */
 	bool LoadConf(ConfigDataHash &target, const char* filename, std::ostringstream &errorstream);
+
 	/** Load 'filename' into 'target', with the new config parser everything is parsed into
 	 * tag/key/value at load-time rather than at read-value time.
 	 */
@@ -532,10 +552,10 @@ class ServerConfig : public Extensible
 	
 	/** Writes 'length' chars into 'result' as a string
 	 */
-	bool ConfValue(ConfigDataHash &target, const char* tag, const char* var, int index, char* result, int length);
+	bool ConfValue(ConfigDataHash &target, const char* tag, const char* var, int index, char* result, int length, bool allow_linefeeds = false);
 	/** Writes 'length' chars into 'result' as a string
 	 */
-	bool ConfValue(ConfigDataHash &target, const std::string &tag, const std::string &var, int index, std::string &result);
+	bool ConfValue(ConfigDataHash &target, const std::string &tag, const std::string &var, int index, std::string &result, bool allow_linefeeds = false);
 	
 	/** Tries to convert the value to an integer and write it to 'result'
 	 */
diff --git a/include/modules.h b/include/modules.h
index 4a9fd87b8..ef103a1a5 100644
--- a/include/modules.h
+++ b/include/modules.h
@@ -322,24 +322,24 @@ class Event : public ModuleMessage
  * be loaded. If this happens, the error message returned by ModuleException::GetReason will be displayed to the user
  * attempting to load the module, or dumped to the console if the ircd is currently loading for the first time.
  */
-class ModuleException : public std::exception
+class CoreException : public std::exception
 {
- private:
+ protected:
 	/** Holds the error message to be displayed
 	 */
-	std::string err;
+	const std::string err;
  public:
-	/** Default constructor, just uses the error mesage 'Module threw an exception'.
+	/** Default constructor, just uses the error mesage 'Core threw an exception'.
 	 */
-	ModuleException() : err("Module threw an exception") {}
+	CoreException() : err("Core threw an exception") {}
 	/** This constructor can be used to specify an error message before throwing.
 	 */
-	ModuleException(std::string message) : err(message) {}
+	CoreException(const std::string &message) : err(message) {}
 	/** This destructor solves world hunger, cancels the world debt, and causes the world to end.
 	 * Actually no, it does nothing. Never mind.
 	 * @throws Nothing!
 	 */
-	virtual ~ModuleException() throw() {};
+	virtual ~CoreException() throw() {};
 	/** Returns the reason for the exception.
 	 * The module should probably put something informative here as the user will see this upon failure.
 	 */
@@ -349,6 +349,23 @@ class ModuleException : public std::exception
 	}
 };
 
+class ModuleException : public CoreException
+{
+ public:
+	/** Default constructor, just uses the error mesage 'Module threw an exception'.
+	 */
+	ModuleException() : CoreException("Module threw an exception") {}
+
+	/** This constructor can be used to specify an error message before throwing.
+	 */
+	ModuleException(const std::string &message) : CoreException(message) {}
+	/** This destructor solves world hunger, cancels the world debt, and causes the world to end.
+	 * Actually no, it does nothing. Never mind.
+	 * @throws Nothing!
+	 */
+	virtual ~ModuleException() throw() {};
+};
+
 /** Priority types which can be returned from Module::Prioritize()
  */
 enum Priority { PRIORITY_FIRST, PRIORITY_DONTCARE, PRIORITY_LAST, PRIORITY_BEFORE, PRIORITY_AFTER };
@@ -1364,7 +1381,7 @@ class ConfigReader : public classbase
 	 * This method retrieves a value from the config file. Where multiple copies of the tag
 	 * exist in the config file, index indicates which of the values to retrieve.
 	 */
-	std::string ReadValue(const std::string &tag, const std::string &name, int index);
+	std::string ReadValue(const std::string &tag, const std::string &name, int index, bool allow_linefeeds = false);
 	/** Retrieves a boolean value from the config file.
 	 * This method retrieves a boolean value from the config file. Where multiple copies of the tag
 	 * exist in the config file, index indicates which of the values to retrieve. The values "1", "yes"
diff --git a/src/configreader.cpp b/src/configreader.cpp
index 24e76eb49..edf431165 100644
--- a/src/configreader.cpp
+++ b/src/configreader.cpp
@@ -577,6 +577,45 @@ bool DoneMaxBans(ServerConfig* conf, const char* tag)
 	return true;
 }
 
+void ServerConfig::ReportConfigError(const std::string &errormessage, bool bail, userrec* user)
+{
+	ServerInstance->Log(DEFAULT, "There were errors in your configuration file: %s", errormessage.c_str());
+	if (bail)
+	{
+		/* Unneeded because of the ServerInstance->Log() aboive? */
+		printf("There were errors in your configuration:\n%s",errormessage.c_str());
+		InspIRCd::Exit(ERROR);
+	}
+	else
+	{
+		std::string errors = errormessage;
+		std::string::size_type start;
+		unsigned int prefixlen;
+		start = 0;
+		/* ":ServerInstance->Config->ServerName NOTICE user->nick :" */
+		prefixlen = strlen(this->ServerName) + strlen(user->nick) + 11;
+		if (user)
+		{
+			user->WriteServ("NOTICE %s :There were errors in the configuration file:",user->nick);
+			while (start < errors.length())
+			{
+				user->WriteServ("NOTICE %s :%s",user->nick, errors.substr(start, 510 - prefixlen).c_str());
+				start += 510 - prefixlen;
+			}
+		}
+		else
+		{
+			ServerInstance->WriteOpers("There were errors in the configuration file:");
+			while (start < errors.length())
+			{
+				ServerInstance->WriteOpers(errors.substr(start, 360).c_str());
+				start += 360;
+			}
+		}
+		return;
+	}
+}
+
 void ServerConfig::Read(bool bail, userrec* user)
 {
 	static char debug[MAXBUF];	/* Temporary buffer for debugging value */
@@ -702,143 +741,123 @@ void ServerConfig::Read(bool bail, userrec* user)
 	}
 	else
 	{
-		ServerInstance->Log(DEFAULT, "There were errors in your configuration:\n%s", errstr.str().c_str());
-
-		if (bail)
-		{
-			/* Unneeded because of the ServerInstance->Log() aboive? */
-			printf("There were errors in your configuration:\n%s",errstr.str().c_str());
-			InspIRCd::Exit(ERROR);
-		}
-		else
-		{
-			std::string errors = errstr.str();
-			std::string::size_type start;
-			unsigned int prefixlen;
-			
-			start = 0;
-			/* ":ServerInstance->Config->ServerName NOTICE user->nick :" */
-			prefixlen = strlen(this->ServerName) + strlen(user->nick) + 11;
-	
-			if (user)
-			{
-				user->WriteServ("NOTICE %s :There were errors in the configuration file:",user->nick);
-				
-				while(start < errors.length())
-				{
-					user->WriteServ("NOTICE %s :%s",user->nick, errors.substr(start, 510 - prefixlen).c_str());
-					start += 510 - prefixlen;
-				}
-			}
-			else
-			{
-				ServerInstance->WriteOpers("There were errors in the configuration file:");
-				
-				while(start < errors.length())
-				{
-					ServerInstance->WriteOpers(errors.substr(start, 360).c_str());
-					start += 360;
-				}
-			}
-
-			return;
-		}
+		ReportConfigError(errstr.str(), bail, user);
+		return;
 	}
 
-	/* Check we dont have more than one of singular tags, or any of them missing
-	 */
-	for (int Index = 0; Once[Index]; Index++)
-		if (!CheckOnce(Once[Index], bail, user))
-			return;
-
-	/* Read the values of all the tags which occur once or not at all, and call their callbacks.
-	 */
-	for (int Index = 0; Values[Index].tag; Index++)
+	/* The stuff in here may throw CoreException, be sure we're in a position to catch it. */
+	try
 	{
-		char item[MAXBUF];
-		ConfValue(this->config_data, Values[Index].tag, Values[Index].value, 0, item, MAXBUF);
-		ValueItem vi(item);
-
-		Values[Index].validation_function(this, Values[Index].tag, Values[Index].value, vi);
+		/* Check we dont have more than one of singular tags, or any of them missing
+		 */
+		for (int Index = 0; Once[Index]; Index++)
+			if (!CheckOnce(Once[Index], bail, user))
+				return;
 
-		switch (Values[Index].datatype)
+		/* Read the values of all the tags which occur once or not at all, and call their callbacks.
+		 */
+		for (int Index = 0; Values[Index].tag; Index++)
 		{
-			case DT_CHARPTR:
+			char item[MAXBUF];
+			int dt = Values[Index].datatype;
+			bool allow_newlines =  ((dt & DT_ALLOW_NEWLINE) > 0);
+			dt &= ~DT_ALLOW_NEWLINE;
+
+			ConfValue(this->config_data, Values[Index].tag, Values[Index].value, 0, item, MAXBUF, allow_newlines);
+			ValueItem vi(item);
+
+			Values[Index].validation_function(this, Values[Index].tag, Values[Index].value, vi);
+
+			switch (Values[Index].datatype)
+			{
+				case DT_CHARPTR:
 				{
 					ValueContainerChar* vcc = (ValueContainerChar*)Values[Index].val;
 					vcc->Set(vi.GetString(), strlen(vi.GetString()));
 				}
-			break;
-			case DT_INTEGER:
+				break;
+				case DT_INTEGER:
 				{
 					int val = vi.GetInteger();
 					ValueContainerInt* vci = (ValueContainerInt*)Values[Index].val;
 					vci->Set(&val, sizeof(int));
 				}
-			break;
-			case DT_BOOLEAN:
+				break;
+				case DT_BOOLEAN:
 				{
 					bool val = vi.GetBool();
 					ValueContainerBool* vcb = (ValueContainerBool*)Values[Index].val;
 					vcb->Set(&val, sizeof(bool));
 				}
-			break;
-			default:
-				/* You don't want to know what happens if someones bad code sends us here. */
-			break;
+				break;
+				default:
+					/* You don't want to know what happens if someones bad code sends us here. */
+				break;
+			}
+	
+			/* We're done with this now */
+			delete Values[Index].val;
 		}
 
-		/* We're done with this now */
-		delete Values[Index].val;
-	}
-
-	/* Read the multiple-tag items (class tags, connect tags, etc)
-	 * and call the callbacks associated with them. We have three
-	 * callbacks for these, a 'start', 'item' and 'end' callback.
-	 */
-	for (int Index = 0; MultiValues[Index].tag; Index++)
-	{
-		MultiValues[Index].init_function(this, MultiValues[Index].tag);
+		/* Read the multiple-tag items (class tags, connect tags, etc)
+		 * and call the callbacks associated with them. We have three
+		 * callbacks for these, a 'start', 'item' and 'end' callback.
+		 */
+		for (int Index = 0; MultiValues[Index].tag; Index++)
+		{
+			MultiValues[Index].init_function(this, MultiValues[Index].tag);
 
-		int number_of_tags = ConfValueEnum(this->config_data, MultiValues[Index].tag);
+			int number_of_tags = ConfValueEnum(this->config_data, MultiValues[Index].tag);
 
-		for (int tagnum = 0; tagnum < number_of_tags; tagnum++)
-		{
-			ValueList vl;
-			for (int valuenum = 0; MultiValues[Index].items[valuenum]; valuenum++)
+			for (int tagnum = 0; tagnum < number_of_tags; tagnum++)
 			{
-				switch (MultiValues[Index].datatype[valuenum])
+				ValueList vl;
+				for (int valuenum = 0; MultiValues[Index].items[valuenum]; valuenum++)
 				{
-					case DT_CHARPTR:
+					int dt = MultiValues[Index].datatype[valuenum];
+					bool allow_newlines =  ((dt & DT_ALLOW_NEWLINE) > 0);
+					dt &= ~DT_ALLOW_NEWLINE;
+
+					switch (dt)
+					{
+						case DT_CHARPTR:
 						{
 							char item[MAXBUF];
-							ConfValue(this->config_data, MultiValues[Index].tag, MultiValues[Index].items[valuenum], tagnum, item, MAXBUF);
+							ConfValue(this->config_data, MultiValues[Index].tag, MultiValues[Index].items[valuenum], tagnum, item, MAXBUF, allow_newlines);
 							vl.push_back(ValueItem(item));
 						}
-					break;
-					case DT_INTEGER:
+						break;
+						case DT_INTEGER:
 						{
 							int item;
 							ConfValueInteger(this->config_data, MultiValues[Index].tag, MultiValues[Index].items[valuenum], tagnum, item);
 							vl.push_back(ValueItem(item));
 						}
-					break;
-					case DT_BOOLEAN:
+						break;
+						case DT_BOOLEAN:
 						{
 							bool item = ConfValueBool(this->config_data, MultiValues[Index].tag, MultiValues[Index].items[valuenum], tagnum);
 							vl.push_back(ValueItem(item));
 						}
-					break;
-					default:
-						/* Someone was smoking craq if we got here, and we're all gonna die. */
-					break;
+						break;
+						default:
+							/* Someone was smoking craq if we got here, and we're all gonna die. */
+						break;
+					}
 				}
+	
+				MultiValues[Index].validation_function(this, MultiValues[Index].tag, (char**)MultiValues[Index].items, vl, MultiValues[Index].datatype);
 			}
-
-			MultiValues[Index].validation_function(this, MultiValues[Index].tag, (char**)MultiValues[Index].items, vl, MultiValues[Index].datatype);
+	
+			MultiValues[Index].finish_function(this, MultiValues[Index].tag);
 		}
 
-		MultiValues[Index].finish_function(this, MultiValues[Index].tag);
+	}
+
+	catch (CoreException &ce)
+	{
+		ReportConfigError(ce.GetReason(), bail, user);
+		return;
 	}
 
 	// write once here, to try it out and make sure its ok
@@ -1267,30 +1286,37 @@ bool ServerConfig::DoInclude(ConfigDataHash &target, const std::string &file, st
 	return LoadConf(target, newfile, errorstream);
 }
 
-bool ServerConfig::ConfValue(ConfigDataHash &target, const char* tag, const char* var, int index, char* result, int length)
+bool ServerConfig::ConfValue(ConfigDataHash &target, const char* tag, const char* var, int index, char* result, int length, bool allow_linefeeds)
 {
 	std::string value;
-	bool r = ConfValue(target, std::string(tag), std::string(var), index, value);
+	bool r = ConfValue(target, std::string(tag), std::string(var), index, value, allow_linefeeds);
 	strlcpy(result, value.c_str(), length);
 	return r;
 }
 
-bool ServerConfig::ConfValue(ConfigDataHash &target, const std::string &tag, const std::string &var, int index, std::string &result)
+bool ServerConfig::ConfValue(ConfigDataHash &target, const std::string &tag, const std::string &var, int index, std::string &result, bool allow_linefeeds)
 {
 	ConfigDataHash::size_type pos = index;
 	if((pos >= 0) && (pos < target.count(tag)))
 	{
 		ConfigDataHash::const_iterator iter = target.find(tag);
-		
+
 		for(int i = 0; i < index; i++)
 			iter++;
-		
+
 		for(KeyValList::const_iterator j = iter->second.begin(); j != iter->second.end(); j++)
 		{
 			if(j->first == var)
 			{
-				result = j->second;
-				return true;
+ 				if ((!allow_linefeeds) && (j->second.find('\n') != std::string::npos))
+				{
+					throw CoreException("Value of <" + tag + ":" + var+ "> contains a linefeed, and linefeeds in this value are not permitted");
+				}
+				else
+				{
+					result = j->second;
+					return true;
+				}
 			}
 		}
 	}
@@ -1302,7 +1328,7 @@ bool ServerConfig::ConfValue(ConfigDataHash &target, const std::string &tag, con
 	{
 		ServerInstance->Log(DEBUG, "ConfValue got an out-of-range index %d, there are only %d occurences of %s", pos, target.count(tag), tag.c_str());
 	}
-	
+
 	return false;
 }
 	
diff --git a/src/modules.cpp b/src/modules.cpp
index df7c9da40..773f87f66 100644
--- a/src/modules.cpp
+++ b/src/modules.cpp
@@ -619,12 +619,12 @@ ConfigReader::ConfigReader(InspIRCd* Instance, const std::string &filename) : Se
 		this->error = CONF_FILE_NOT_FOUND;
 };
 
-std::string ConfigReader::ReadValue(const std::string &tag, const std::string &name, int index)
+std::string ConfigReader::ReadValue(const std::string &tag, const std::string &name, int index, bool allow_linefeeds)
 {
 	/* Don't need to strlcpy() tag and name anymore, ReadConf() takes const char* */ 
 	std::string result;
 	
-	if (!ServerInstance->Config->ConfValue(*this->data, tag, name, index, result))
+	if (!ServerInstance->Config->ConfValue(*this->data, tag, name, index, result, allow_linefeeds))
 	{
 		this->error = CONF_VALUE_NOT_FOUND;
 		return "";
@@ -666,46 +666,7 @@ long ConfigReader::GetError()
 
 void ConfigReader::DumpErrors(bool bail, userrec* user)
 {
-	/* XXX - Duplicated code */
-	
-	if (bail)
-	{
-		printf("There were errors in your configuration:\n%s", this->errorlog->str().c_str());
-		InspIRCd::Exit(ERROR);
-	}
-	else
-	{
-		std::string errors = this->errorlog->str();
-		std::string::size_type start;
-		unsigned int prefixlen;
-		
-		start = 0;
-		/* ":ServerInstance->Config->ServerName NOTICE user->nick :" */
-		prefixlen = strlen(ServerInstance->Config->ServerName) + strlen(user->nick) + 11;
-	
-		if (user)
-		{
-			user->WriteServ("NOTICE %s :There were errors in the configuration file:",user->nick);
-			
-			while(start < errors.length())
-			{
-				user->WriteServ("NOTICE %s :%s",user->nick, errors.substr(start, 510 - prefixlen).c_str());
-				start += 510 - prefixlen;
-			}
-		}
-		else
-		{
-			ServerInstance->WriteOpers("There were errors in the configuration file:");
-			
-			while(start < errors.length())
-			{
-				ServerInstance->WriteOpers(errors.substr(start, 360).c_str());
-				start += 360;
-			}
-		}
-
-		return;
-	}
+	ServerInstance->Config->ReportConfigError(this->errorlog->str(), bail, user);
 }