From 43aada5b33c78431d60a87149e68a4071151f8cf Mon Sep 17 00:00:00 2001 From: Valentin Lorentz Date: Sun, 30 May 2021 19:06:19 +0200 Subject: [PATCH] Store ignored hostmasks in Expiring HostmaskSet to prevent their pattern cache from expiring too soon See e0fdcb67c09b26a79bbb3cbb2244d18149c5b0c2 for the rationale (tl;dr: prevents triggering a degenerate case of the LRU cache when there are over 1000 ignore masks) --- src/ircdb.py | 28 +++++++--------------- src/ircutils.py | 54 +++++++++++++++++++++++++++++++++++++++++++ test/test_ircutils.py | 26 +++++++++++++++++++++ 3 files changed, 88 insertions(+), 20 deletions(-) diff --git a/src/ircdb.py b/src/ircdb.py index 804d3931b..a72af0b8d 100644 --- a/src/ircdb.py +++ b/src/ircdb.py @@ -385,7 +385,7 @@ class IrcChannel(object): self.defaultAllow = defaultAllow self.expiredBans = [] self.bans = bans or {} - self.ignores = ignores or {} + self.ignores = ircutils.ExpiringHostmaskDict(ignores) self.silences = silences or [] self.exceptions = exceptions or [] self.capabilities = capabilities or CapabilitySet() @@ -471,14 +471,8 @@ class IrcChannel(object): assert ircutils.isUserHostmask(hostmask), 'got %s' % hostmask if self.checkBan(hostmask): return True - now = time.time() - for (pattern, expiration) in list(self.ignores.items()): - if now < expiration or not expiration: - if ircutils.hostmaskPatternEqual(pattern, hostmask): - return True - else: - del self.ignores[pattern] - # Later we may wish to keep expiredIgnores, but not now. + if self.ignores.match(hostmask): + return True return False def preserve(self, fd, indent=''): @@ -1059,7 +1053,7 @@ class IgnoresDB(object): __slots__ = ('filename', 'hostmasks') def __init__(self): self.filename = None - self.hostmasks = {} + self.hostmasks = ircutils.ExpiringHostmaskDict() def open(self, filename): self.filename = filename @@ -1098,26 +1092,20 @@ class IgnoresDB(object): def reload(self): if self.filename is not None: - oldhostmasks = self.hostmasks.copy() + oldhostmasks = list(self.hostmasks.items()) self.hostmasks.clear() try: self.open(self.filename) except EnvironmentError as e: log.warning('IgnoresDB.reload failed: %s', e) # Let's be somewhat transactional. - self.hostmasks.update(oldhostmasks) + for (hostmask, expiration) in oldhostmasks: + self.hostmasks.add(hostmask, expiration) else: log.warning('IgnoresDB.reload called without self.filename.') def checkIgnored(self, prefix): - now = time.time() - for (hostmask, expiration) in list(self.hostmasks.items()): - if expiration and now > expiration: - del self.hostmasks[hostmask] - else: - if ircutils.hostmaskPatternEqual(hostmask, prefix): - return True - return False + return bool(self.hostmasks.match(prefix)) def add(self, hostmask, expiration=0): assert ircutils.isUserHostmask(hostmask), 'got %s' % hostmask diff --git a/src/ircutils.py b/src/ircutils.py index 10ede24c7..a4d246ff8 100644 --- a/src/ircutils.py +++ b/src/ircutils.py @@ -252,6 +252,60 @@ class HostmaskSet(collections.abc.MutableSet): return pattern return None + def __repr__(self): + return 'HostmaskSet(%r)' % (list(self.data),) + + +class ExpiringHostmaskDict(collections.abc.MutableMapping): + """Like HostmaskSet, but behaves like a dict with expiration timestamps + as values.""" + + # To keep it thread-safe, add to self.patterns first, then + # self.data; and remove from self.data first. + # And never iterate on self.patterns + + def __init__(self, hostmasks=None): + if isinstance(hostmasks, (list, tuple)): + hostmasks = dict(hostmasks) + self.data = hostmasks or {} + self.patterns = HostmaskSet(list(self.data)) + + def __getitem__(self, hostmask): + return self.data[hostmask] + + def __setitem__(self, hostmask, expiration): + """For backward compatibility, in case any plugin depends on it + being dict-like.""" + self.patterns.add(hostmask) + self.data[hostmask] = expiration + + def __iter__(self): + return iter(self.data) + + def __len__(self): + return len(self.data) + + def __delitem__(self, hostmask): + del self.data[hostmask] + self.patterns.discard(hostmask) + + def expire(self): + now = time.time() + for (hostmask, expiration) in list(self.data.items()): + if now >= expiration and expiration: + self.pop(hostmask, None) + + def match(self, hostname): + self.expire() + return self.patterns.match(hostname) + + def clear(self): + self.data.clear() + self.patterns.clear() + + def __repr__(self): + return 'ExpiringHostmaskSet(%r)' % (self.expirations,) + def banmask(hostmask): """Returns a properly generic banning hostmask for a hostmask. diff --git a/test/test_ircutils.py b/test/test_ircutils.py index f1f35d92d..5629686cc 100644 --- a/test/test_ircutils.py +++ b/test/test_ircutils.py @@ -73,6 +73,32 @@ class FunctionsTestCase(SupyTestCase): hs = ircutils.HostmaskSet(["*!user@host"]) self.assertEqual(hs.match("nick!user@host"), "*!user@host") + def testExpiringHostmaskDict(self): + hs = ircutils.ExpiringHostmaskDict() + self.assertEqual(hs.match("nick!user@host"), None) + time1 = time.time() + 15 + time2 = time.time() + 10 + hs["*!user@host"] = time1 + hs["*!user@host2"] = time2 + self.assertEqual(hs.match("nick!user@host"), "*!user@host") + self.assertEqual(hs.match("nick!user@host2"), "*!user@host2") + self.assertCountEqual(list(hs.items()), + [("*!user@host", time1), ("*!user@host2", time2)]) + del hs["*!user@host2"] + self.assertEqual(hs.match("nick!user@host"), "*!user@host") + self.assertEqual(hs.match("nick!user@host2"), None) + timeFastForward(10) + self.assertEqual(hs.match("nick!user@host"), "*!user@host") + timeFastForward(10) + self.assertEqual(hs.match("nick!user@host"), None) + + hs = ircutils.ExpiringHostmaskDict([("*!user@host", time.time() + 10)]) + self.assertEqual(hs.match("nick!user@host"), "*!user@host") + self.assertEqual(hs.match("nick!user@host2"), None) + timeFastForward(11) + self.assertEqual(hs.match("nick!user@host"), None) + self.assertEqual(hs.match("nick!user@host2"), None) + def testIsUserHostmask(self): self.assertTrue(ircutils.isUserHostmask(self.hostmask)) self.assertTrue(ircutils.isUserHostmask('a!b@c'))