test cases extended;

code review

fail2ban/server/jail.py

@@ -192,7 +192,7 @@ class Jail:
 		Used by filter to add a failure for banning.
 		"""
 		self.__queue.put(ticket)
-		# add ban to database moved to actions (should previously check not already banned 
+		# add ban to database moved to observer (should previously check not already banned 
 		# and increase ticket time if "bantime.increment" set)
 
 	def getFailTicket(self):
@@ -256,15 +256,9 @@ class Jail:
 			return self._banExtra.get(opt, None)
 		return self._banExtra
 
-	def start(self):
+	def restoreCurrentBans(self):
-		"""Start the jail, by starting filter and actions threads.
+		"""Restore any previous valid bans from the database.
-
-		Once stated, also queries the persistent database to reinstate
-		any valid bans.
 		"""
-		self.filter.start()
-		self.actions.start()
-		# Restore any previous valid bans from the database
 		try:
 			if self.database is not None:
 				forbantime = None;
@@ -276,11 +270,21 @@ class Jail:
 					if not self.filter.inIgnoreIPList(ticket.getIP()):
 						# mark ticked was restored from database - does not put it again into db:
 						ticket.setRestored(True)
-						self.__queue.put(ticket)
+						self.putFailTicket(ticket)
 		except Exception as e:
 			logSys.error('%s', e, exc_info=logSys.getEffectiveLevel()<=logging.DEBUG)
 			#logSys.error('%s', e, exc_info=True)
 
+	def start(self):
+		"""Start the jail, by starting filter and actions threads.
+
+		Once stated, also queries the persistent database to reinstate
+		any valid bans.
+		"""
+		self.filter.start()
+		self.actions.start()
+		self.restoreCurrentBans()
+
 		logSys.info("Jail '%s' started" % self.name)
 
 	def stop(self):

fail2ban/server/observer.py

@@ -263,10 +263,15 @@ class ObserverThread(threading.Thread):
 	def wait_empty(self, sleeptime=None):
 		"""Wait observer is running and returns if observer has no more events (queue is empty)
 		"""
-		# block queue with not operation to be sure all really jobs are executed if nop goes from queue :
+		time.sleep(0.001)
-		self._queue.append(('nop',))
+		if not self.is_full:
+			return not self.is_full
 		if sleeptime is not None:
 			e = MyTime.time() + sleeptime
+		# block queue with not operation to be sure all really jobs are executed if nop goes from queue :
+		self.add_wn('nop')
+		if self.is_full and self.idle:
+			self.pulse_notify()
 		while self.is_full:
 			if sleeptime is not None and MyTime.time() > e:
 				break

fail2ban/tests/dummyjail.py

@@ -30,10 +30,10 @@ from ..server.actions import Actions
 class DummyJail(Jail, object):
 	"""A simple 'jail' to suck in all the tickets generated by Filter's
 	"""
-	def __init__(self):
+	def __init__(self, backend=None):
 		self.lock = Lock()
 		self.queue = []
-		super(DummyJail, self).__init__(name='DummyJail', backend=None)
+		super(DummyJail, self).__init__(name='DummyJail', backend=backend)
 		self.__db = None
 		self.__actions = Actions(self)
 

fail2ban/tests/observertestcase.py

@@ -35,6 +35,7 @@ from ..server.ticket import FailTicket
 from ..server.failmanager import FailManager
 from ..server.observer import Observers, ObserverThread
 from .utils import LogCaptureTestCase
+from ..server.filter import Filter
 from .dummyjail import DummyJail
 try:
 	from ..server.database import Fail2BanDb
@@ -60,7 +61,9 @@ class BanTimeIncr(LogCaptureTestCase):
 	def testDefault(self, multipliers = None):
 		a = self.__jail;
 		a.setBanTimeExtra('increment', 'true')
+		self.assertEqual(a.getBanTimeExtra('increment'), True)
 		a.setBanTimeExtra('maxtime', '1d')
+		self.assertEqual(a.getBanTimeExtra('maxtime'), 24*60*60)
 		a.setBanTimeExtra('rndtime', None)
 		a.setBanTimeExtra('factor', None)
 		# tests formulat or multipliers:
@@ -377,10 +380,10 @@ class BanTimeIncrDB(unittest.TestCase):
 		self.assertEqual(restored_tickets, [])
 
     # two separate jails :
-		jail1 = DummyJail()
+		jail1 = DummyJail(backend='polling')
 		jail1.database = self.db
 		self.db.addJail(jail1)
-		jail2 = DummyJail()
+		jail2 = DummyJail(backend='polling')
 		jail2.database = self.db
 		self.db.addJail(jail2)
 		ticket1 = FailTicket(ip, stime, [])
@@ -415,6 +418,14 @@ class BanTimeIncrDB(unittest.TestCase):
 		for row in self.db.getBan(ip, overalljails=True):
 			self.assertEqual(row, (3, stime, 18000))
 			break
+		# test restoring bans from database:
+		jail1.restoreCurrentBans()
+		self.assertEqual(str(jail1.getFailTicket()), 
+			'FailTicket: ip=%s time=%s bantime=%s bancount=1 #attempts=0 matches=[]' % (ip, stime, 6000)
+		)
+		# jail2 does not restore any bans (because all ban tickets should be already expired: stime-6000):
+		jail2.restoreCurrentBans()
+		self.assertEqual(jail2.getFailTicket(), False)
 
 	def testObserver(self):
 		if Fail2BanDb is None: # pragma: no cover