badips: try to fix sporadic test errors if badips-server timed out resp. not available (502 bad gateway or similar).

config/action.d/badips.py

@@ -114,6 +114,15 @@ class BadIPsAction(ActionBase): # pragma: no cover - may be unavailable
 		except Exception as e: # pragma: no cover
 			return False, e
 
+	def logError(self, response, what=''): # pragma: no cover - sporadical (502: Bad Gateway, etc)
+		messages = {}
+		try:
+			messages = json.loads(response.read().decode('utf-8'))
+		except:
+			pass
+		self._logSys.error(
+			"%s. badips.com response: '%s'", what,
+				messages.get('err', 'Unknown'))
 
 	def getCategories(self, incParents=False):
 		"""Get badips.com categories.
@@ -133,11 +142,8 @@ class BadIPsAction(ActionBase): # pragma: no cover - may be unavailable
 		try:
 			response = urlopen(
 				self._Request("/".join([self._badips, "get", "categories"])), timeout=self.timeout)
-		except HTTPError as response:
+		except HTTPError as response: # pragma: no cover
-			messages = json.loads(response.read().decode('utf-8'))
+			self.logError(response, "Failed to fetch categories")
-			self._logSys.error(
-				"Failed to fetch categories. badips.com response: '%s'",
-				messages['err'])
 			raise
 		else:
 			response_json = json.loads(response.read().decode('utf-8'))
@@ -188,11 +194,8 @@ class BadIPsAction(ActionBase): # pragma: no cover - may be unavailable
 				url = "&".join([url, urlencode({'key': key})])
 			self._logSys.debug('badips.com: get list, url: %r', url)
 			response = urlopen(self._Request(url), timeout=self.timeout)
-		except HTTPError as response:
+		except HTTPError as response: # pragma: no cover
-			messages = json.loads(response.read().decode('utf-8'))
+			self.logError(response, "Failed to fetch bad IP list")
-			self._logSys.error(
-				"Failed to fetch bad IP list. badips.com response: '%s'",
-				messages['err'])
 			raise
 		else:
 			return set(response.read().decode('utf-8').split())
@@ -286,7 +289,7 @@ class BadIPsAction(ActionBase): # pragma: no cover - may be unavailable
 					exc_info=self._logSys.getEffectiveLevel()<=logging.DEBUG)
 			else:
 				self._bannedips.add(ip)
-				self._logSys.info(
+				self._logSys.debug(
 					"Banned IP %s for jail '%s' with action '%s'",
 					ip, self._jail.name, self.banaction)
 
@@ -306,7 +309,7 @@ class BadIPsAction(ActionBase): # pragma: no cover - may be unavailable
 					ip, self._jail.name, self.banaction, e,
 					exc_info=self._logSys.getEffectiveLevel()<=logging.DEBUG)
 			else:
-				self._logSys.info(
+				self._logSys.debug(
 					"Unbanned IP %s for jail '%s' with action '%s'",
 					ip, self._jail.name, self.banaction)
 			finally:
@@ -338,7 +341,7 @@ class BadIPsAction(ActionBase): # pragma: no cover - may be unavailable
 				# Add new IPs which are now listed
 				self._banIPs(ips - self._bannedips)
 
-				self._logSys.info(
+				self._logSys.debug(
 					"Updated IPs for jail '%s'. Update again in %i seconds",
 					self._jail.name, self.updateperiod)
 			finally:
@@ -374,15 +377,12 @@ class BadIPsAction(ActionBase): # pragma: no cover - may be unavailable
 				url = "?".join([url, urlencode({'key': self.key})])
 			self._logSys.debug('badips.com: ban, url: %r', url)
 			response = urlopen(self._Request(url), timeout=self.timeout)
-		except HTTPError as response:
+		except HTTPError as response: # pragma: no cover
-			messages = json.loads(response.read().decode('utf-8'))
+			self.logError(response, "Failed to ban")
-			self._logSys.error(
-				"Response from badips.com report: '%s'",
-				messages['err'])
 			raise
 		else:
 			messages = json.loads(response.read().decode('utf-8'))
-			self._logSys.info(
+			self._logSys.debug(
 				"Response from badips.com report: '%s'",
 				messages['suc'])
 

fail2ban/tests/action_d/test_badips.py

@@ -20,6 +20,7 @@
 import os
 import unittest
 import sys
+from functools import wraps
 from socket import timeout
 from ssl import SSLError
 
@@ -28,6 +29,25 @@ from ..dummyjail import DummyJail
 from ..servertestcase import IPAddr
 from ..utils import LogCaptureTestCase, CONFIG_DIR
 
+if sys.version_info >= (3, ):
+	from urllib.error import HTTPError, URLError
+else:
+	from urllib2 import HTTPError, URLError
+
+def skip_if_not_available(f):
+	"""Helper to decorate tests to skip in case of timeout/http-errors like "502 bad gateway".
+	"""
+	@wraps(f)
+	def wrapper(self, *args):
+		try:
+			return f(self, *args)
+		except (SSLError, HTTPError, URLError, timeout) as e: # pragma: no cover - timeout/availability issues
+			if not isinstance(e, timeout) and 'timed out' not in str(e):
+				if not hasattr(e, 'code') or e.code > 200 and e.code <= 404:
+					raise
+			raise unittest.SkipTest('Skip test because of %s' % e)
+	return wrapper
+
 if sys.version_info >= (2,7): # pragma: no cover - may be unavailable
 	class BadIPsActionTest(LogCaptureTestCase):
 
@@ -75,6 +95,7 @@ if sys.version_info >= (2,7): # pragma: no cover - may be unavailable
 				self.action._timer.cancel()
 			super(BadIPsActionTest, self).tearDown()
 
+		@skip_if_not_available
 		def testCategory(self):
 			categories = self.action.getCategories()
 			self.assertIn("ssh", categories)
@@ -90,17 +111,20 @@ if sys.version_info >= (2,7): # pragma: no cover - may be unavailable
 			# but valid for blacklisting.
 			self.action.bancategory = "mail"
 
+		@skip_if_not_available
 		def testScore(self):
 			self.assertRaises(ValueError, setattr, self.action, "score", -5)
 			self.action.score = 3
 			self.action.score = "3"
 
+		@skip_if_not_available
 		def testBanaction(self):
 			self.assertRaises(
 				ValueError, setattr, self.action, "banaction",
 				"invalid-action")
 			self.action.banaction = "test"
 
+		@skip_if_not_available
 		def testUpdateperiod(self):
 			self.assertRaises(
 				ValueError, setattr, self.action, "updateperiod", -50)
@@ -109,18 +133,15 @@ if sys.version_info >= (2,7): # pragma: no cover - may be unavailable
 			self.action.updateperiod = 900
 			self.action.updateperiod = "900"
 
+		@skip_if_not_available
 		def testStartStop(self):
-			try:
+			self.action.start()
-				self.action.start()
+			self.assertTrue(len(self.action._bannedips) > 10,
-				self.assertTrue(len(self.action._bannedips) > 10,
+				"%s is fewer as 10: %r" % (len(self.action._bannedips), self.action._bannedips))
-					"%s is fewer as 10: %r" % (len(self.action._bannedips), self.action._bannedips))
+			self.action.stop()
-				self.action.stop()
+			self.assertTrue(len(self.action._bannedips) == 0)
-				self.assertTrue(len(self.action._bannedips) == 0)
-			except (SSLError, timeout) as e: # pragma: no cover - timeout only
-				if not isinstance(e, timeout) and 'timed out' not in str(e):
-					raise
-				raise unittest.SkipTest('Skip test because of %s' % e)
 
+		@skip_if_not_available
 		def testBanIP(self):
 			aInfo = CallingMap({
 				'ip': IPAddr('192.0.2.1')