From 12df12f282c63be40dd6475e9d4e2f15bb702f72 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 21 Apr 2013 10:21:54 +0100 Subject: [PATCH 1/7] BF: Change logging instance logSys `warn` method to `warning` `warn` is long time depreciated method, which may be dropped in python3.4 http://bugs.python.org/issue13235 --- bin/fail2ban-client | 2 +- fail2ban/client/beautifier.py | 2 +- fail2ban/client/configreader.py | 8 ++++---- fail2ban/client/jailreader.py | 2 +- fail2ban/server/actions.py | 4 ++-- fail2ban/server/asyncserver.py | 2 +- fail2ban/server/filter.py | 6 +++--- fail2ban/server/filterpoll.py | 4 ++-- fail2ban/server/transmitter.py | 2 +- 9 files changed, 16 insertions(+), 16 deletions(-) diff --git a/bin/fail2ban-client b/bin/fail2ban-client index 8068d60f..e0b7efc6 100755 --- a/bin/fail2ban-client +++ b/bin/fail2ban-client @@ -102,7 +102,7 @@ class Fail2banClient: def __sigTERMhandler(self, signum, frame): # Print a new line because we probably come from wait print - logSys.warn("Caught signal %d. Exiting" % signum) + logSys.warning("Caught signal %d. Exiting" % signum) sys.exit(-1) def __getCmdLineOptions(self, optList): diff --git a/fail2ban/client/beautifier.py b/fail2ban/client/beautifier.py index 153ab905..a0ff8ff3 100644 --- a/fail2ban/client/beautifier.py +++ b/fail2ban/client/beautifier.py @@ -133,7 +133,7 @@ class Beautifier: c += 1 msg = msg + "`- [" + str(c) + "]: " + response[len(response)-1] except Exception: - logSys.warn("Beautifier error. Please report the error") + logSys.warning("Beautifier error. Please report the error") logSys.error("Beautify " + `response` + " with " + `self.__inputCmd` + " failed") msg = msg + `response` diff --git a/fail2ban/client/configreader.py b/fail2ban/client/configreader.py index 6f1e7740..b2d3e392 100644 --- a/fail2ban/client/configreader.py +++ b/fail2ban/client/configreader.py @@ -70,7 +70,7 @@ class ConfigReader(SafeConfigParserWithIncludes): # files must carry .conf suffix as well config_files += sorted(glob.glob('%s/*.conf' % config_dir)) else: - logSys.warn("%s exists but not a directory or not accessible" + logSys.warning("%s exists but not a directory or not accessible" % config_dir) # check if files are accessible, warn if any is not accessible @@ -80,7 +80,7 @@ class ConfigReader(SafeConfigParserWithIncludes): if os.access(f, os.R_OK): config_files_accessible.append(f) else: - logSys.warn("%s exists but not accessible - skipping" % f) + logSys.warning("%s exists but not accessible - skipping" % f) if len(config_files_accessible): # at least one config exists and accessible @@ -122,11 +122,11 @@ class ConfigReader(SafeConfigParserWithIncludes): values[option[1]] = option[2] except NoOptionError: if not option[2] == None: - logSys.warn("'%s' not defined in '%s'. Using default one: %r" + logSys.warning("'%s' not defined in '%s'. Using default one: %r" % (option[1], sec, option[2])) values[option[1]] = option[2] except ValueError: - logSys.warn("Wrong value for '" + option[1] + "' in '" + sec + + logSys.warning("Wrong value for '" + option[1] + "' in '" + sec + "'. Using default one: '" + `option[2]` + "'") values[option[1]] = option[2] return values diff --git a/fail2ban/client/jailreader.py b/fail2ban/client/jailreader.py index 6e35bc0b..d8102971 100644 --- a/fail2ban/client/jailreader.py +++ b/fail2ban/client/jailreader.py @@ -105,7 +105,7 @@ class JailReader(ConfigReader): logSys.debug("Caught exception: %s" % (e,)) return False if not len(self.__actions): - logSys.warn("No actions were defined for %s" % self.__name) + logSys.warning("No actions were defined for %s" % self.__name) return True def convert(self): diff --git a/fail2ban/server/actions.py b/fail2ban/server/actions.py index c0373239..b7e58975 100644 --- a/fail2ban/server/actions.py +++ b/fail2ban/server/actions.py @@ -177,7 +177,7 @@ class Actions(JailThread): aInfo["time"] = bTicket.getTime() aInfo["matches"] = "".join(bTicket.getMatches()) if self.__banManager.addBanTicket(bTicket): - logSys.warn("[%s] Ban %s" % (self.jail.getName(), aInfo["ip"])) + logSys.warning("[%s] Ban %s" % (self.jail.getName(), aInfo["ip"])) for action in self.__actions: action.execActionBan(aInfo) return True @@ -217,7 +217,7 @@ class Actions(JailThread): aInfo["failures"] = ticket.getAttempt() aInfo["time"] = ticket.getTime() aInfo["matches"] = "".join(ticket.getMatches()) - logSys.warn("[%s] Unban %s" % (self.jail.getName(), aInfo["ip"])) + logSys.warning("[%s] Unban %s" % (self.jail.getName(), aInfo["ip"])) for action in self.__actions: action.execActionUnban(aInfo) diff --git a/fail2ban/server/asyncserver.py b/fail2ban/server/asyncserver.py index 8c905010..e6af4b26 100644 --- a/fail2ban/server/asyncserver.py +++ b/fail2ban/server/asyncserver.py @@ -135,7 +135,7 @@ class AsyncServer(asyncore.dispatcher): if os.path.exists(sock): logSys.error("Fail2ban seems to be already running") if force: - logSys.warn("Forcing execution of the server") + logSys.warning("Forcing execution of the server") os.remove(sock) else: raise AsyncServerException("Server already running") diff --git a/fail2ban/server/filter.py b/fail2ban/server/filter.py index a7062f16..d4108dc2 100644 --- a/fail2ban/server/filter.py +++ b/fail2ban/server/filter.py @@ -632,7 +632,7 @@ class FileContainer: try: line = line.decode(self.getEncoding(), 'strict') except UnicodeDecodeError: - logSys.warn("Error decoding line from '%s' with '%s': %s" % + logSys.warning("Error decoding line from '%s' with '%s': %s" % (self.getFileName(), self.getEncoding(), `line`)) if sys.version_info >= (3,): # In python3, must be decoded line = line.decode(self.getEncoding(), 'ignore') @@ -668,11 +668,11 @@ class DNSUtils: try: return socket.gethostbyname_ex(dns)[2] except socket.gaierror: - logSys.warn("Unable to find a corresponding IP address for %s" + logSys.warning("Unable to find a corresponding IP address for %s" % dns) return list() except socket.error, e: - logSys.warn("Socket error raised trying to resolve hostname %s: %s" + logSys.warning("Socket error raised trying to resolve hostname %s: %s" % (dns, e)) return list() dnsToIp = staticmethod(dnsToIp) diff --git a/fail2ban/server/filterpoll.py b/fail2ban/server/filterpoll.py index 3217e958..191b95b3 100644 --- a/fail2ban/server/filterpoll.py +++ b/fail2ban/server/filterpoll.py @@ -131,10 +131,10 @@ class FilterPoll(FileFilter): % (filename, e)) self.__file404Cnt[filename] += 1 if self.__file404Cnt[filename] > 2: - logSys.warn("Too many errors. Setting the jail idle") + logSys.warning("Too many errors. Setting the jail idle") if self.jail: self.jail.setIdle(True) else: - logSys.warn("No jail is assigned to %s" % self) + logSys.warning("No jail is assigned to %s" % self) self.__file404Cnt[filename] = 0 return False diff --git a/fail2ban/server/transmitter.py b/fail2ban/server/transmitter.py index 2d27ff6e..2a4514da 100644 --- a/fail2ban/server/transmitter.py +++ b/fail2ban/server/transmitter.py @@ -55,7 +55,7 @@ class Transmitter: ret = self.__commandHandler(command) ack = 0, ret except Exception, e: - logSys.warn("Command %r has failed. Received %r" + logSys.warning("Command %r has failed. Received %r" % (command, e)) ack = 1, e return ack From c9b1b88bfcde1ed14ca76360b161f102c184895a Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 21 Apr 2013 10:26:30 +0100 Subject: [PATCH 2/7] TST: Ensure files are closed in tests to remove ResourceWarnings --- fail2ban/tests/clientreadertestcase.py | 4 +++- fail2ban/tests/filtertestcase.py | 29 ++++++++++++++++---------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index d721ef00..6fb125e3 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -53,10 +53,12 @@ class ConfigReaderTest(unittest.TestCase): d_ = os.path.join(self.d, d) if not os.path.exists(d_): os.makedirs(d_) - open("%s/%s" % (self.d, fname), "w").write(""" + f = open("%s/%s" % (self.d, fname), "w") + f.write(""" [section] option = %s """ % value) + f.close() def _remove(self, fname): os.unlink("%s/%s" % (self.d, fname)) diff --git a/fail2ban/tests/filtertestcase.py b/fail2ban/tests/filtertestcase.py index 053086d4..e540e41e 100644 --- a/fail2ban/tests/filtertestcase.py +++ b/fail2ban/tests/filtertestcase.py @@ -123,7 +123,7 @@ def _assert_correct_last_attempt(utest, filter_, output, count=None): _assert_equal_entries(utest, found, output, count) -def _copy_lines_between_files(fin, fout, n=None, skip=0, mode='a', terminal_line=""): +def _copy_lines_between_files(in_, fout, n=None, skip=0, mode='a', terminal_line=""): """Copy lines from one file to another (which might be already open) Returns open fout @@ -132,8 +132,10 @@ def _copy_lines_between_files(fin, fout, n=None, skip=0, mode='a', terminal_line # on old Python st_mtime is int, so we should give at least 1 sec so # polling filter could detect the change time.sleep(1) - if isinstance(fin, str): # pragma: no branch - only used with str in test cases - fin = open(fin, 'r') + if isinstance(in_, str): # pragma: no branch - only used with str in test cases + fin = open(in_, 'r') + else: + fin = in_ # Skip for i in xrange(skip): _ = fin.readline() @@ -151,6 +153,9 @@ def _copy_lines_between_files(fin, fout, n=None, skip=0, mode='a', terminal_line fout = open(fout, mode) fout.write('\n'.join(lines)) fout.flush() + if isinstance(in_, str): # pragma: no branch - only used with str in test cases + # Opened earlier, therefore must close it + fin.close() # to give other threads possibly some time to crunch time.sleep(0.1) return fout @@ -291,7 +296,7 @@ class LogFileMonitor(unittest.TestCase): # # if we rewrite the file at once self.file.close() - _copy_lines_between_files(GetFailures.FILENAME_01, self.name) + _copy_lines_between_files(GetFailures.FILENAME_01, self.name).close() self.filter.getFailures(self.name) _assert_correct_last_attempt(self, self.filter, GetFailures.FAILURES_01) @@ -308,6 +313,7 @@ class LogFileMonitor(unittest.TestCase): def testNewChangeViaGetFailures_move(self): # # if we move file into a new location while it has been open already + self.file.close() self.file = _copy_lines_between_files(GetFailures.FILENAME_01, self.name, n=14, mode='w') self.filter.getFailures(self.name) @@ -316,7 +322,7 @@ class LogFileMonitor(unittest.TestCase): # move aside, but leaving the handle still open... os.rename(self.name, self.name + '.bak') - _copy_lines_between_files(GetFailures.FILENAME_01, self.name, skip=14) + _copy_lines_between_files(GetFailures.FILENAME_01, self.name, skip=14).close() self.filter.getFailures(self.name) _assert_correct_last_attempt(self, self.filter, GetFailures.FAILURES_01) self.assertEqual(self.filter.failManager.getFailTotal(), 3) @@ -454,7 +460,7 @@ def get_monitor_failures_testcase(Filter_): def test_rewrite_file(self): # if we rewrite the file at once self.file.close() - _copy_lines_between_files(GetFailures.FILENAME_01, self.name) + _copy_lines_between_files(GetFailures.FILENAME_01, self.name).close() self.assert_correct_last_attempt(GetFailures.FAILURES_01) # What if file gets overridden @@ -468,6 +474,7 @@ def get_monitor_failures_testcase(Filter_): def test_move_file(self): # if we move file into a new location while it has been open already + self.file.close() self.file = _copy_lines_between_files(GetFailures.FILENAME_01, self.name, n=14, mode='w') # Poll might need more time @@ -477,25 +484,25 @@ def get_monitor_failures_testcase(Filter_): # move aside, but leaving the handle still open... os.rename(self.name, self.name + '.bak') - _copy_lines_between_files(GetFailures.FILENAME_01, self.name, skip=14) + _copy_lines_between_files(GetFailures.FILENAME_01, self.name, skip=14).close() self.assert_correct_last_attempt(GetFailures.FAILURES_01) self.assertEqual(self.filter.failManager.getFailTotal(), 3) # now remove the moved file _killfile(None, self.name + '.bak') - _copy_lines_between_files(GetFailures.FILENAME_01, self.name, n=100) + _copy_lines_between_files(GetFailures.FILENAME_01, self.name, n=100).close() self.assert_correct_last_attempt(GetFailures.FAILURES_01) self.assertEqual(self.filter.failManager.getFailTotal(), 6) def test_new_bogus_file(self): # to make sure that watching whole directory does not effect - _copy_lines_between_files(GetFailures.FILENAME_01, self.name, n=100) + _copy_lines_between_files(GetFailures.FILENAME_01, self.name, n=100).close() self.assert_correct_last_attempt(GetFailures.FAILURES_01) # create a bogus file in the same directory and see if that doesn't affect - open(self.name + '.bak2', 'w').write('') - _copy_lines_between_files(GetFailures.FILENAME_01, self.name, n=100) + open(self.name + '.bak2', 'w').close() + _copy_lines_between_files(GetFailures.FILENAME_01, self.name, n=100).close() self.assert_correct_last_attempt(GetFailures.FAILURES_01) self.assertEqual(self.filter.failManager.getFailTotal(), 6) _killfile(None, self.name + '.bak2') From 393679341385d24db09a489d06fde494480d7b3c Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 21 Apr 2013 10:28:34 +0100 Subject: [PATCH 3/7] TST: Change depreciated unittest assertEquals method to assertEqual --- fail2ban/tests/clientreadertestcase.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fail2ban/tests/clientreadertestcase.py b/fail2ban/tests/clientreadertestcase.py index 6fb125e3..754f7998 100644 --- a/fail2ban/tests/clientreadertestcase.py +++ b/fail2ban/tests/clientreadertestcase.py @@ -118,7 +118,7 @@ class JailReaderTest(unittest.TestCase): action = "mail-whois[name=SSH]" expected = ['mail-whois', {'name': 'SSH'}] result = JailReader.splitAction(action) - self.assertEquals(expected, result) + self.assertEqual(expected, result) class FilterReaderTest(unittest.TestCase): @@ -150,7 +150,7 @@ class FilterReaderTest(unittest.TestCase): # Add sort as configreader uses dictionary and therefore order # is unreliable - self.assertEquals(sorted(filterReader.convert()), sorted(output)) + self.assertEqual(sorted(filterReader.convert()), sorted(output)) class JailsReaderTest(unittest.TestCase): From b182c5b5d455e68c343611f095f5caa05c64052a Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 21 Apr 2013 10:30:13 +0100 Subject: [PATCH 4/7] ENH: For python3.2+ use ConfigPaser which replaces SafeConfigParser Current SafeConfigParser alias to be dropped in future python versions --- fail2ban/client/configparserinc.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fail2ban/client/configparserinc.py b/fail2ban/client/configparserinc.py index 0ffa0728..7b054876 100644 --- a/fail2ban/client/configparserinc.py +++ b/fail2ban/client/configparserinc.py @@ -27,8 +27,12 @@ __date__ = '$Date$' __copyright__ = 'Copyright (c) 2007 Yaroslav Halchenko' __license__ = 'GPL' -import logging, os -from ConfigParser import SafeConfigParser +import logging, os, sys +if sys.version_info >= (3,2): # pragma: no cover + # SafeConfigParser deprecitated from python 3.2 (renamed ConfigParser) + from configparser import ConfigParser as SafeConfigParser +else: # pragma: no cover + from ConfigParser import SafeConfigParser # Gets the instance of the logger. logSys = logging.getLogger(__name__) From 9d2d907fc198dffee1366bb812d184c685022916 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 21 Apr 2013 10:59:14 +0100 Subject: [PATCH 5/7] BF: Remove warnings handler which breaks setup.py python2<2.7 and python3<3.2 --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index f99e4f35..5f65ff37 100755 --- a/setup.py +++ b/setup.py @@ -51,7 +51,6 @@ if setuptools and "test" in sys.argv: logSys.addHandler(hdlr) if set(["-q", "--quiet"]) & set(sys.argv): logSys.setLevel(logging.FATAL) - logging.captureWarnings(True) elif set(["-v", "--verbose"]) & set(sys.argv): logSys.setLevel(logging.DEBUG) else: From 6f3c66f466a9f9e5abc692d1c0c6131909d94cf3 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 21 Apr 2013 13:23:08 +0100 Subject: [PATCH 6/7] ENH: Reimplement warning suppression of setup.py test --quiet --- setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 5f65ff37..058c4b21 100755 --- a/setup.py +++ b/setup.py @@ -39,7 +39,7 @@ except ImportError: from distutils.command.build_py import build_py from distutils.command.build_scripts import build_scripts from os.path import isfile, join, isdir -import sys +import sys, warnings from glob import glob if setuptools and "test" in sys.argv: @@ -51,6 +51,8 @@ if setuptools and "test" in sys.argv: logSys.addHandler(hdlr) if set(["-q", "--quiet"]) & set(sys.argv): logSys.setLevel(logging.FATAL) + warnings.simplefilter("ignore") + sys.warnoptions.append("ignore") elif set(["-v", "--verbose"]) & set(sys.argv): logSys.setLevel(logging.DEBUG) else: From dadd6aed2f9d20a6d5dc08264433e9b4344ebbe9 Mon Sep 17 00:00:00 2001 From: Steven Hiscocks Date: Sun, 21 Apr 2013 17:39:56 +0100 Subject: [PATCH 7/7] BF+TST: Correctly reset time in tearDownMyTime --- fail2ban/tests/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/fail2ban/tests/utils.py b/fail2ban/tests/utils.py index 57814664..95646d63 100644 --- a/fail2ban/tests/utils.py +++ b/fail2ban/tests/utils.py @@ -118,6 +118,7 @@ def tearDownMyTime(): if old_TZ: os.environ['TZ'] = old_TZ time.tzset() + MyTime.myTime = None from fail2ban.tests import banmanagertestcase from fail2ban.tests import clientreadertestcase