From 00fdf5ce0a7309b9575d04463c136bdf1e2394fa Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 10 Jun 2014 12:31:55 +0200 Subject: [PATCH] test cases extended; code review --- fail2ban/server/jail.py | 24 ++++++++++++++---------- fail2ban/server/observer.py | 9 +++++++-- fail2ban/tests/dummyjail.py | 4 ++-- fail2ban/tests/observertestcase.py | 15 +++++++++++++-- 4 files changed, 36 insertions(+), 16 deletions(-) diff --git a/fail2ban/server/jail.py b/fail2ban/server/jail.py index 9d4eec29..d87395de 100644 --- a/fail2ban/server/jail.py +++ b/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): - """Start the jail, by starting filter and actions threads. - - Once stated, also queries the persistent database to reinstate - any valid bans. + def restoreCurrentBans(self): + """Restore any previous valid bans from the database. """ - 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): diff --git a/fail2ban/server/observer.py b/fail2ban/server/observer.py index fa5a5f62..930acc87 100644 --- a/fail2ban/server/observer.py +++ b/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 : - self._queue.append(('nop',)) + time.sleep(0.001) + 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 diff --git a/fail2ban/tests/dummyjail.py b/fail2ban/tests/dummyjail.py index 4ca7e108..c5624f02 100644 --- a/fail2ban/tests/dummyjail.py +++ b/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) diff --git a/fail2ban/tests/observertestcase.py b/fail2ban/tests/observertestcase.py index 096d6181..6b8400ff 100644 --- a/fail2ban/tests/observertestcase.py +++ b/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