Add ConfigTag::getDuration() with optional bounds checking

8 files changed, 45 insertions, 32 deletions

diff --git a/docs/conf/links.conf.example b/docs/conf/links.conf.example
index 94340cd31..76e9c09cb 100644
--- a/docs/conf/links.conf.example
+++ b/docs/conf/links.conf.example
@@ -96,7 +96,7 @@
 # Simple autoconnect block. This enables automatic connection of a server
 # Recommended setup is to have leaves connect to the hub, and have no
 # automatic connections started by the hub.
-<autoconnect period="300" server="hub.penguin.org">
+<autoconnect period="10m" server="hub.penguin.org">
 
 # Failover autoconnect block. If you have multiple hubs, or want your network
 # to automatically link even if the hub is down, you can specify multiple
diff --git a/include/configreader.h b/include/configreader.h
index bf5acbbc8..ee58c3bc9 100644
--- a/include/configreader.h
+++ b/include/configreader.h
@@ -50,6 +50,16 @@ class CoreExport ConfigTag : public refcountbase
 	/** Get the value of an option, using def if it does not exist */
 	bool getBool(const std::string& key, bool def = false);
 
+	/** Get the value in seconds of a duration that is in the user-friendly "1h2m3s" format,
+	 * using a default value if it does not exist or is out of bounds.
+	 * @param key The config key name
+	 * @param def Default value (optional)
+	 * @param min Minimum acceptable value (optional)
+	 * @param max Maximum acceptable value (optional)
+	 * @return The duration in seconds
+	 */
+	time_t getDuration(const std::string& key, time_t def = 0, long min = LONG_MIN, long max = LONG_MAX);
+
 	/** Get the value of an option
 	 * @param key The option to get
 	 * @param value The location to store the value (unmodified if does not exist)
@@ -58,6 +68,16 @@ class CoreExport ConfigTag : public refcountbase
 	 */
 	bool readString(const std::string& key, std::string& value, bool allow_newline = false);
 
+	/** Check for an out of range value. If the value falls outside the boundaries a warning is
+	 * logged and the value is corrected (set to def).
+	 * @param key The key name, used in the warning message
+	 * @param res The value to verify and modify if needed
+	 * @param def The default value, res will be set to this if (min <= res <= max) doesn't hold true
+	 * @param min Minimum accepted value for res
+	 * @param max Maximum accepted value for res
+	 */
+	void CheckRange(const std::string& key, long& res, long def, long min, long max);
+
 	std::string getTagLocation();
 
 	inline const std::vector<KeyVal>& getItems() const { return items; }
diff --git a/src/commands/cmd_whowas.cpp b/src/commands/cmd_whowas.cpp
index 8a1c9a820..093d948b4 100644
--- a/src/commands/cmd_whowas.cpp
+++ b/src/commands/cmd_whowas.cpp
@@ -299,16 +299,6 @@ class ModuleWhoWas : public Module
 {
 	CommandWhowas cmd;
 
-	void RangeCheck(int& value, int min, int max, int def, const char* msg)
-	{
-		// From ConfigReader
-		if (value >= min && value <= max)
-			return;
-
-		ServerInstance->Logs->Log("CONFIG", LOG_DEFAULT, "WARNING: %s value of %d is not between %d and %d; set to %d.", msg, value, min, max, def);
-		value = def;
-	}
-
  public:
 	ModuleWhoWas() : cmd(this)
 	{
@@ -344,9 +334,7 @@ class ModuleWhoWas : public Module
 		ConfigTag* tag = ServerInstance->Config->ConfValue("whowas");
 		int NewGroupSize = tag->getInt("groupsize", 10, 0, 10000);
 		int NewMaxGroups = tag->getInt("maxgroups", 10240, 0, 1000000);
-		int NewMaxKeep = InspIRCd::Duration(tag->getString("maxkeep"));
-
-		RangeCheck(NewMaxKeep, 3600, INT_MAX, 3600, "<whowas:maxkeep>");
+		int NewMaxKeep = tag->getDuration("maxkeep", 3600, 3600);
 
 		if ((NewGroupSize == cmd.WhoWasGroupSize) && (NewMaxGroups == cmd.WhoWasMaxGroups) && (NewMaxKeep == cmd.WhoWasMaxKeep))
 			return;
diff --git a/src/configparser.cpp b/src/configparser.cpp
index 1783e901e..d3723d350 100644
--- a/src/configparser.cpp
+++ b/src/configparser.cpp
@@ -445,13 +445,30 @@ long ConfigTag::getInt(const std::string &key, long def, long min, long max)
 			res = res * 1024 * 1024 * 1024;
 			break;
 	}
+
+	CheckRange(key, res, def, min, max);
+	return res;
+}
+
+void ConfigTag::CheckRange(const std::string& key, long& res, long def, long min, long max)
+{
 	if (res < min || res > max)
 	{
 		ServerInstance->Logs->Log("CONFIG", LOG_DEFAULT, "WARNING: <%s:%s> value of %ld is not between %ld and %ld; set to %ld.",
 			tag.c_str(), key.c_str(), res, min, max, def);
 		res = def;
 	}
-	return res;
+}
+
+time_t ConfigTag::getDuration(const std::string& key, long def, long min, long max)
+{
+	std::string duration;
+	if (!readString(key, duration))
+		return def;
+
+	time_t ret = InspIRCd::Duration(duration);
+	CheckRange(key, ret, def, min, max);
+	return ret;
 }
 
 double ConfigTag::getFloat(const std::string &key, double def)
diff --git a/src/modules/m_connectban.cpp b/src/modules/m_connectban.cpp
index 36e10ec5b..227373a36 100644
--- a/src/modules/m_connectban.cpp
+++ b/src/modules/m_connectban.cpp
@@ -46,9 +46,7 @@ class ModuleConnectBan : public Module
 		ipv4_cidr = tag->getInt("ipv4cidr", 32, 1, 32);
 		ipv6_cidr = tag->getInt("ipv6cidr", 128, 1, 128);
 		threshold = tag->getInt("threshold", 10, 1);
-		banduration = InspIRCd::Duration(tag->getString("duration", "10m"));
-		if (banduration == 0)
-			banduration = 10*60;
+		banduration = tag->getDuration("duration", 10*60, 1);
 	}
 
 	void OnSetUserIP(LocalUser* u) CXX11_OVERRIDE
diff --git a/src/modules/m_dnsbl.cpp b/src/modules/m_dnsbl.cpp
index b457648e6..becc7a6e8 100644
--- a/src/modules/m_dnsbl.cpp
+++ b/src/modules/m_dnsbl.cpp
@@ -289,7 +289,7 @@ class ModuleDNSBL : public Module
 			}
 
 			e->banaction = str2banaction(tag->getString("action"));
-			e->duration = InspIRCd::Duration(tag->getString("duration", "60"));
+			e->duration = tag->getDuration("duration", 60, 1);
 
 			/* Use portparser for record replies */
 
@@ -314,11 +314,6 @@ class ModuleDNSBL : public Module
 				std::string location = tag->getTagLocation();
 				ServerInstance->SNO->WriteGlobalSno('a', "DNSBL(%s): Invalid banaction", location.c_str());
 			}
-			else if (e->duration <= 0)
-			{
-				std::string location = tag->getTagLocation();
-				ServerInstance->SNO->WriteGlobalSno('a', "DNSBL(%s): Invalid duration", location.c_str());
-			}
 			else
 			{
 				if (e->reason.empty())
diff --git a/src/modules/m_filter.cpp b/src/modules/m_filter.cpp
index 30ee35f88..d60dd7942 100644
--- a/src/modules/m_filter.cpp
+++ b/src/modules/m_filter.cpp
@@ -675,7 +675,7 @@ void ModuleFilter::ReadFilters()
 		std::string reason = i->second->getString("reason");
 		std::string action = i->second->getString("action");
 		std::string flgs = i->second->getString("flags");
-		unsigned long gline_time = InspIRCd::Duration(i->second->getString("duration"));
+		unsigned long gline_time = i->second->getDuration("duration", 10*60, 1);
 		if (flgs.empty())
 			flgs = "*";
 
diff --git a/src/modules/m_spanningtree/utils.cpp b/src/modules/m_spanningtree/utils.cpp
index c689676a9..79cd74505 100644
--- a/src/modules/m_spanningtree/utils.cpp
+++ b/src/modules/m_spanningtree/utils.cpp
@@ -336,7 +336,7 @@ void SpanningTreeUtilities::ReadConfiguration()
 		L->RecvPass = tag->getString("recvpass", tag->getString("password"));
 		L->Fingerprint = tag->getString("fingerprint");
 		L->HiddenFromStats = tag->getBool("statshidden");
-		L->Timeout = tag->getInt("timeout", 30);
+		L->Timeout = tag->getDuration("timeout", 30);
 		L->Hook = tag->getString("ssl");
 		L->Bind = tag->getString("bind");
 		L->Hidden = tag->getBool("hidden");
@@ -380,7 +380,7 @@ void SpanningTreeUtilities::ReadConfiguration()
 	{
 		ConfigTag* tag = i->second;
 		reference<Autoconnect> A = new Autoconnect(tag);
-		A->Period = tag->getInt("period");
+		A->Period = tag->getDuration("period", 60, 1);
 		A->NextConnectTime = ServerInstance->Time() + A->Period;
 		A->position = -1;
 		irc::spacesepstream ss(tag->getString("server"));
@@ -390,11 +390,6 @@ void SpanningTreeUtilities::ReadConfiguration()
 			A->servers.push_back(server);
 		}
 
-		if (A->Period <= 0)
-		{
-			throw ModuleException("Invalid configuration for autoconnect, period not a positive integer!");
-		}
-
 		if (A->servers.empty())
 		{
 			throw ModuleException("Invalid configuration for autoconnect, server cannot be empty!");