From 0298c8a31e666e789583f350f27e718724a3591b Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 27 Dec 2018 18:07:23 +0100 Subject: [PATCH 1/2] closes gh-2277: fixed cache-object clean-up process (if max-size reached) used multi-threaded (del can throw KeyError if get/unset changes the list); additionally OrderedDict is used now for cache (if available, so >= 2.7) - avoids (slow) search of expired items in full cache and always prefers older objects to remove (like FIFO). --- fail2ban/server/utils.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/fail2ban/server/utils.py b/fail2ban/server/utils.py index 8569a3f2..7104bb45 100644 --- a/fail2ban/server/utils.py +++ b/fail2ban/server/utils.py @@ -27,9 +27,15 @@ import os import signal import subprocess import sys +from threading import Lock import time from ..helpers import getLogger, _merge_dicts, uni_decode +try: + from collections import OrderedDict +except ImportError: # pragma: 3.x no cover + OrderedDict = dict + if sys.version_info >= (3, 3): import importlib.machinery else: @@ -69,7 +75,8 @@ class Utils(): def __init__(self, *args, **kwargs): self.setOptions(*args, **kwargs) - self._cache = {} + self._cache = OrderedDict() + self.__lock = Lock() def setOptions(self, maxCount=1000, maxTime=60): self.maxCount = maxCount @@ -83,7 +90,7 @@ class Utils(): if v: if v[1] > time.time(): return v[0] - del self._cache[k] + self.unset(k) return defv def set(self, k, v): @@ -91,12 +98,21 @@ class Utils(): cache = self._cache # for shorter local access # clean cache if max count reached: if len(cache) >= self.maxCount: - for (ck, cv) in cache.items(): - if cv[1] < t: - del cache[ck] - # if still max count - remove any one: - if len(cache) >= self.maxCount: - cache.popitem() + # avoid multiple modification of list multi-threaded: + with self.__lock: + if len(cache) >= self.maxCount: + for (ck, cv) in cache.items(): + # if expired: + if cv[1] <= t: + self.unset(ck) + elif OrderedDict is not dict: + break + # if still max count - remove any one: + if len(cache) >= self.maxCount: + if OrderedDict is not dict: # first (older): + cache.popitem(False) + else: + cache.popitem() cache[k] = (v, t + self.maxTime) def unset(self, k): From 4a4780be0456ec958c93833f9c2beb02dd08e5e2 Mon Sep 17 00:00:00 2001 From: sebres Date: Thu, 27 Dec 2018 18:10:09 +0100 Subject: [PATCH 2/2] test-cases: prevent sporadic timing errors (unban if ban still not occurred) --- fail2ban/tests/servertestcase.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fail2ban/tests/servertestcase.py b/fail2ban/tests/servertestcase.py index 610a1a5d..8c2d0e3e 100644 --- a/fail2ban/tests/servertestcase.py +++ b/fail2ban/tests/servertestcase.py @@ -64,7 +64,7 @@ class TestServer(Server): pass -class TransmitterBase(unittest.TestCase): +class TransmitterBase(LogCaptureTestCase): def setUp(self): """Call before every test case.""" @@ -332,11 +332,11 @@ class Transmitter(TransmitterBase): self.assertEqual( self.transm.proceed(["set", self.jailName, "banip", "127.0.0.1"]), (0, "127.0.0.1")) - time.sleep(Utils.DEFAULT_SLEEP_TIME) # Give chance to ban + self.assertLogged("Ban 127.0.0.1", wait=True) # Give chance to ban self.assertEqual( self.transm.proceed(["set", self.jailName, "banip", "Badger"]), (0, "Badger")) #NOTE: Is IP address validated? Is DNS Lookup done? - time.sleep(Utils.DEFAULT_SLEEP_TIME) # Give chance to ban + self.assertLogged("Ban Badger", wait=True) # Give chance to ban # Unban IP self.assertEqual( self.transm.proceed(