ExtendedCymruInfo: better availability check (code review and timeout's);

max sleep time check of too long sleep increased to 1 second (typo fix)

fail2ban/server/banmanager.py

@@ -28,7 +28,7 @@ from threading import Lock
 
 from .ticket import BanTicket
 from .mytime import MyTime
-from ..helpers import getLogger
+from ..helpers import getLogger, logging
 
 # Gets the instance of the logger.
 logSys = getLogger(__name__)
@@ -132,17 +132,23 @@ class BanManager:
 	#
 	# @return {"asn": [], "country": [], "rir": []} dict for self.__banList IPs
 
-	def getBanListExtendedCymruInfo(self):
+	def getBanListExtendedCymruInfo(self, timeout=10):
 		return_dict = {"asn": [], "country": [], "rir": []}
-		try:
+		if not hasattr(self, 'dnsResolver'):
-			import dns.exception
+			try:
-			import dns.resolver
+				import dns.exception
-		except ImportError:
+				import dns.resolver
-			logSys.error("dnspython package is required but could not be imported")
+				resolver = dns.resolver.Resolver()
-			return_dict["asn"].append("error")
+				resolver.lifetime = timeout
-			return_dict["country"].append("error")
+				resolver.timeout = timeout / 2
-			return_dict["rir"].append("error")
+				self.dnsResolver = resolver
-			return return_dict
+			except ImportError: # pragma: no cover
+				logSys.error("dnspython package is required but could not be imported")
+				return_dict["error"] = repr(e)
+				return_dict["asn"].append("error")
+				return_dict["country"].append("error")
+				return_dict["rir"].append("error")
+				return return_dict
 		# get ips in lock:
 		with self.__lock:
 			banIPs = [banData.getIP() for banData in self.__banList.values()]
@@ -155,7 +161,10 @@ class BanManager:
 					else "origin6.asn.cymru.com"
 				)
 				try:
-					answers = dns.resolver.query(question, "TXT")
+					resolver = self.dnsResolver
+					answers = resolver.query(question, "TXT")
+					if not answers:
+						raise ValueError("No data retrieved")
 					for rdata in answers:
 						asn, net, country, rir, changed =\
 							[answer.strip("'\" ") for answer in rdata.to_text().split("|")]
@@ -169,20 +178,23 @@ class BanManager:
 					return_dict["asn"].append("nxdomain")
 					return_dict["country"].append("nxdomain")
 					return_dict["rir"].append("nxdomain")
-				except dns.exception.DNSException as dnse:
+				except (dns.exception.DNSException, dns.resolver.NoNameservers, dns.exception.Timeout) as dnse: # pragma: no cover
-					logSys.error("Unhandled DNSException querying Cymru for %s TXT" % question)
+					logSys.error("DNSException %r querying Cymru for %s TXT", dnse, question)
-					logSys.exception(dnse)
+					if logSys.level <= logging.DEBUG:
-					return_dict["error"] = dnse
+						logSys.exception(dnse)
+					return_dict["error"] = repr(dnse)
 					break
-				except Exception as e:
+				except Exception as e: # pragma: no cover
-					logSys.error("Unhandled Exception querying Cymru for %s TXT" % question)
+					logSys.error("Unhandled Exception %r querying Cymru for %s TXT", e, question)
-					logSys.exception(e)
+					if logSys.level <= logging.DEBUG:
-					return_dict["error"] = e
+						logSys.exception(e)
+					return_dict["error"] = repr(e)
 					break
-		except Exception as e:
+		except Exception as e: # pragma: no cover
-			logSys.error("Failure looking up extended Cymru info")
+			logSys.error("Failure looking up extended Cymru info: %s", e)
-			logSys.exception(e)
+			if logSys.level <= logging.DEBUG:
-			return_dict["error"] = e
+				logSys.exception(e)
+			return_dict["error"] = repr(e)
 		return return_dict
 
 	##

fail2ban/tests/banmanagertestcase.py

@@ -147,9 +147,17 @@ class StatusExtendedCymruInfo(unittest.TestCase):
 		"""Call after every test case."""
 		pass
 
+	available = True, None
+
 	def _getBanListExtendedCymruInfo(self):
-		cymru_info = self.__banManager.getBanListExtendedCymruInfo()
+		tc = StatusExtendedCymruInfo
+		if tc.available[0]:
+			cymru_info = self.__banManager.getBanListExtendedCymruInfo(
+				timeout=(2 if unittest.F2B.fast else 20))
+		else:
+			cymru_info = tc.available[1]
 		if cymru_info.get("error"): # pragma: no cover - availability
+			tc.available = False, cymru_info
 			raise unittest.SkipTest('Skip test because service is not available: %s' % cymru_info["error"])
 		return cymru_info
 

fail2ban/tests/utils.py

@@ -248,7 +248,7 @@ def initTests(opts):
 		# sleep intervals are large - use replacement for sleep to check time to sleep:
 		_org_sleep = time.sleep
 		def _new_sleep(v):
-			if v > min(1, Utils.DEFAULT_SLEEP_TIME): # pragma: no cover
+			if v > max(1, Utils.DEFAULT_SLEEP_TIME): # pragma: no cover
 				raise ValueError('[BAD-CODE] To long sleep interval: %s, try to use conditional Utils.wait_for instead' % v)
 			_org_sleep(min(v, Utils.DEFAULT_SLEEP_TIME))
 		time.sleep = _new_sleep