From b92316ff17024ef1c0f0fa48298b5b51f60026b6 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Mon, 22 Jul 2013 11:47:36 -0400 Subject: [PATCH 1/3] RF(ENH): JailsReader.getOptions -- avoid code duplication when asking for 1 jail or all --- client/jailsreader.py | 45 +++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/client/jailsreader.py b/client/jailsreader.py index 408a40bf..00c63e3c 100644 --- a/client/jailsreader.py +++ b/client/jailsreader.py @@ -18,7 +18,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. # Author: Cyril Jaquier -# +# __author__ = "Cyril Jaquier" __copyright__ = "Copyright (c) 2004 Cyril Jaquier" @@ -32,7 +32,7 @@ from jailreader import JailReader logSys = logging.getLogger("fail2ban.client.config") class JailsReader(ConfigReader): - + def __init__(self, force_enable=False, **kwargs): """ Parameters @@ -44,17 +44,25 @@ class JailsReader(ConfigReader): ConfigReader.__init__(self, **kwargs) self.__jails = list() self.__force_enable = force_enable - + def read(self): return ConfigReader.read(self, "jail") - - def getOptions(self, section = None): + + def getOptions(self, section=None): + """Reads configuration for jail(s) and adds enabled jails to __jails + """ opts = [] self.__opts = ConfigReader.getOptions(self, "Definition", opts) - if section: - # Get the options of a specific jail. - jail = JailReader(section, basedir=self.getBaseDir(), force_enable=self.__force_enable) + if section is None: + sections = self.sections() + else: + sections = [ section ] + + # Get the options of all jails. + for sec in sections: + jail = JailReader(sec, basedir=self.getBaseDir(), + force_enable=self.__force_enable) jail.read() ret = jail.getOptions() if ret: @@ -62,23 +70,10 @@ class JailsReader(ConfigReader): # We only add enabled jails self.__jails.append(jail) else: - logSys.error("Errors in jail '%s'. Skipping..." % section) + logSys.error("Errors in jail %r. Skipping..." % sec) return False - else: - # Get the options of all jails. - for sec in self.sections(): - jail = JailReader(sec, basedir=self.getBaseDir(), force_enable=self.__force_enable) - jail.read() - ret = jail.getOptions() - if ret: - if jail.isEnabled(): - # We only add enabled jails - self.__jails.append(jail) - else: - logSys.error("Errors in jail '" + sec + "'. Skipping...") - return False return True - + def convert(self, allow_no_files=False): """Convert read before __opts and jails to the commands stream @@ -99,6 +94,6 @@ class JailsReader(ConfigReader): # Start jails for jail in self.__jails: stream.append(["start", jail.getName()]) - + return stream - + From 149a83545fe45d1b66d0ee525ee29ccce5c2288d Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Mon, 22 Jul 2013 11:52:51 -0400 Subject: [PATCH 2/3] TST: basic test for reading of a bogus jail --- testcases/clientreadertestcase.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/testcases/clientreadertestcase.py b/testcases/clientreadertestcase.py index 695b3877..4e8cdd9d 100644 --- a/testcases/clientreadertestcase.py +++ b/testcases/clientreadertestcase.py @@ -126,6 +126,13 @@ class JailsReaderTest(unittest.TestCase): # commands to communicate to the server self.assertEqual(comm_commands, []) + # We should not "read" some bogus jail + old_comm_commands = comm_commands[:] # make a copy + self.assertFalse(jails.getOptions("BOGUS")) + # and there should be no side-effects + self.assertEqual(jails.convert(), old_comm_commands) + + def testReadStockJailConfForceEnabled(self): # more of a smoke test to make sure that no obvious surprises # on users' systems when enabling shipped jails From 3b52eca608d858ce42a6b6109ad5d1cc7a01d3b9 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Mon, 22 Jul 2013 12:09:33 -0400 Subject: [PATCH 3/3] ENH+TST: Ticket -- drop unused/bogus get|setFile + enh __str__ + basic testing --- server/ticket.py | 8 +------- testcases/failmanagertestcase.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/server/ticket.py b/server/ticket.py index 8826f26b..e0b2cb2e 100644 --- a/server/ticket.py +++ b/server/ticket.py @@ -47,7 +47,7 @@ class Ticket: def __str__(self): return "%s: ip=%s time=%s #attempts=%d" % \ - (self.__class__, self.__ip, self.__time, self.__attempt) + (self.__class__.__name__.split('.')[-1], self.__ip, self.__time, self.__attempt) def setIP(self, value): @@ -59,12 +59,6 @@ class Ticket: def getIP(self): return self.__ip - def setFile(self, value): - self.__file = value - - def getFile(self): - return self.__file - def setTime(self, value): self.__time = value diff --git a/testcases/failmanagertestcase.py b/testcases/failmanagertestcase.py index 1bfab339..bd34e25f 100644 --- a/testcases/failmanagertestcase.py +++ b/testcases/failmanagertestcase.py @@ -78,6 +78,20 @@ class AddFailure(unittest.TestCase): ticket = self.__failManager.toBan() self.assertEqual(ticket.getIP(), "193.168.0.128") self.assertTrue(isinstance(ticket.getIP(), str)) + + # finish with rudimentary tests of the ticket + # verify consistent str + ticket_str = str(ticket) + self.assertEqual( + ticket_str, + 'FailTicket: ip=193.168.0.128 time=1167605999.0 #attempts=5') + # and some get/set-ers otherwise not tested + ticket.setTime(1000002000.0) + self.assertEqual(ticket.getTime(), 1000002000.0) + # and str() adjusted correspondingly + self.assertEqual( + str(ticket), + 'FailTicket: ip=193.168.0.128 time=1000002000.0 #attempts=5') def testbanNOK(self): self.__failManager.setMaxRetry(10)