clean up unnecessarily resp. directly unused action properties, because they are ambiguous now;

implemented caching functionality for same substitutions inside replaceTag: very actual and extreme performance growth (up to 1000 times) for ban/unban because too slow substituteRecursiveTags by several tags and many includes, but totally unnecessary as long as parameters are not changing;

fail2ban/server/action.py

@@ -33,6 +33,7 @@ from abc import ABCMeta
 from collections import MutableMapping
 
 from .ipdns import asip
+from .mytime import MyTime
 from .utils import Utils
 from ..helpers import getLogger
 
@@ -203,36 +204,40 @@ class CommandAction(ActionBase):
 
 	_escapedTags = set(('matches', 'ipmatches', 'ipjailmatches'))
 
+	timeout = 60
+	## Command executed in order to initialize the system.
+	actionstart = ''
+	## Command executed when an IP address gets banned.
+	actionban = ''
+	## Command executed when an IP address gets removed.
+	actionunban = ''
+	## Command executed in order to check requirements.
+	actioncheck = ''
+	## Command executed in order to stop the system.
+	actionstop = ''
+
 	def __init__(self, jail, name):
 		super(CommandAction, self).__init__(jail, name)
-		self.timeout = 60
+		self.__properties = None
-		## Command executed in order to initialize the system.
+		self.__substCache = {}
-		self.actionstart = ''
-		## Command executed when an IP address gets banned.
-		self.actionban = ''
-		## Command executed when an IP address gets removed.
-		self.actionunban = ''
-		## Command executed in order to check requirements.
-		self.actioncheck = ''
-		## Command executed in order to stop the system.
-		self.actionstop = ''
 		self._logSys.debug("Created %s" % self.__class__)
 
 	@classmethod
 	def __subclasshook__(cls, C):
 		return NotImplemented # Standard checks
 
-	@property
+	def __setattr__(self, name, value):
-	def timeout(self):
+		if not name.startswith('_') and not callable(value):
-		"""Time out period in seconds for execution of commands.
+			# special case for some pasrameters:
-		"""
+			if name == 'timeout':
-		return self._timeout
+				value = MyTime.str2seconds(value)
-
+			# parameters changed - clear properties and substitution cache:
-	@timeout.setter
+			self.__properties = None
-	def timeout(self, timeout):
+			self.__substCache.clear()
-		self._timeout = int(timeout)
+			#self._logSys.debug("Set action %r %s = %r", self._name, name, value)
-		self._logSys.debug("Set action %s timeout = %i" %
+			self._logSys.debug("  Set %s = %r", name, value)
-			(self._name, self.timeout))
+		# set:
+		self.__dict__[name] = value
 
 	@property
 	def _properties(self):
@@ -240,21 +245,20 @@ class CommandAction(ActionBase):
 
 		This is used to subsitute "tags" in the commands.
 		"""
-		return dict(
+		# if we have a properties - return it:
+		if self.__properties is not None:
+			return self.__properties
+		# otherwise retrieve:
+		self.__properties = dict(
 			(key, getattr(self, key))
 			for key in dir(self)
 			if not key.startswith("_") and not callable(getattr(self, key)))
+		#
+		return self.__properties
 
 	@property
-	def actionstart(self):
+	def _substCache(self):
-		"""The command executed on start of the jail/action.
+		return self.__substCache
-		"""
-		return self._actionstart
-
-	@actionstart.setter
-	def actionstart(self, value):
-		self._actionstart = value
-		self._logSys.debug("Set actionstart = %s" % value)
 
 	def start(self):
 		"""Executes the "actionstart" command.
@@ -263,29 +267,21 @@ class CommandAction(ActionBase):
 		and executes the resulting command.
 		"""
 		# check valid tags in properties (raises ValueError if self recursion, etc.):
-		if self._properties:
+		try:
-			self.substituteRecursiveTags(self._properties)
+			# common (resp. ipv4):
-		# common (resp. ipv4):
+			startCmd = self.replaceTag('<actionstart>', self._properties, 
-		startCmd = self.replaceTag('<actionstart>', self._properties, conditional='family=inet4')
+				conditional='family=inet4', cache=self.__substCache)
-		res = self.executeCmd(startCmd, self.timeout)
+			res = self.executeCmd(startCmd, self.timeout)
-		# start ipv6 actions if available:
+			# start ipv6 actions if available:
-		if allowed_ipv6:
+			if allowed_ipv6:
-			startCmd6 = self.replaceTag('<actionstart>', self._properties, conditional='family=inet6')
+				startCmd6 = self.replaceTag('<actionstart>', self._properties, 
-			if startCmd6 != startCmd:
+					conditional='family=inet6', cache=self.__substCache)
-				res &= self.executeCmd(startCmd6, self.timeout)
+				if startCmd6 != startCmd:
-		if not res:
+					res &= self.executeCmd(startCmd6, self.timeout)
-			raise RuntimeError("Error starting action")
+			if not res:
-
+				raise RuntimeError("Error starting action %s/%s" % (self._jail, self._name,))
-	@property
+		except ValueError, e:
-	def actionban(self):
+			raise RuntimeError("Error starting action %s/%s: %r" % (self._jail, self._name, e))
-		"""The command used when a ban occurs.
-		"""
-		return self._actionban
-
-	@actionban.setter
-	def actionban(self, value):
-		self._actionban = value
-		self._logSys.debug("Set actionban = %s" % value)
 
 	def ban(self, aInfo):
 		"""Executes the "actionban" command.
@@ -302,17 +298,6 @@ class CommandAction(ActionBase):
 		if not self._processCmd('<actionban>', aInfo):
 			raise RuntimeError("Error banning %(ip)s" % aInfo)
 
-	@property
-	def actionunban(self):
-		"""The command used when an unban occurs.
-		"""
-		return self._actionunban
-
-	@actionunban.setter
-	def actionunban(self, value):
-		self._actionunban = value
-		self._logSys.debug("Set actionunban = %s" % value)
-
 	def unban(self, aInfo):
 		"""Executes the "actionunban" command.
 
@@ -328,32 +313,6 @@ class CommandAction(ActionBase):
 		if not self._processCmd('<actionunban>', aInfo):
 			raise RuntimeError("Error unbanning %(ip)s" % aInfo)
 
-	@property
-	def actioncheck(self):
-		"""The command used to check the environment.
-
-		This is used prior to a ban taking place to ensure the
-		environment is appropriate. If this check fails, `stop` and
-		`start` is executed prior to the check being called again.
-		"""
-		return self._actioncheck
-
-	@actioncheck.setter
-	def actioncheck(self, value):
-		self._actioncheck = value
-		self._logSys.debug("Set actioncheck = %s" % value)
-
-	@property
-	def actionstop(self):
-		"""The command executed when the jail/actions stops.
-		"""
-		return self._actionstop
-
-	@actionstop.setter
-	def actionstop(self, value):
-		self._actionstop = value
-		self._logSys.debug("Set actionstop = %s" % value)
-
 	def stop(self):
 		"""Executes the "actionstop" command.
 
@@ -361,18 +320,20 @@ class CommandAction(ActionBase):
 		and executes the resulting command.
 		"""
 		# common (resp. ipv4):
-		stopCmd = self.replaceTag('<actionstop>', self._properties, conditional='family=inet4')
+		stopCmd = self.replaceTag('<actionstop>', self._properties, 
+			conditional='family=inet4', cache=self.__substCache)
 		res = self.executeCmd(stopCmd, self.timeout)
 		# ipv6 actions if available:
 		if allowed_ipv6:
-			stopCmd6 = self.replaceTag('<actionstop>', self._properties, conditional='family=inet6')
+			stopCmd6 = self.replaceTag('<actionstop>', self._properties, 
+				conditional='family=inet6', cache=self.__substCache)
 			if stopCmd6 != stopCmd:
 				res &= self.executeCmd(stopCmd6, self.timeout)
 		if not res:
 			raise RuntimeError("Error stopping action")
 
 	@classmethod
-	def substituteRecursiveTags(cls, tags, conditional=''):
+	def substituteRecursiveTags(cls, inptags, conditional=''):
 		"""Sort out tag definitions within other tags.
 		Since v.0.9.2 supports embedded interpolation (see test cases for examples).
 
@@ -382,7 +343,7 @@ class CommandAction(ActionBase):
 
 		Parameters
 		----------
-		tags : dict
+		inptags : dict
 			Dictionary of tags(keys) and their values.
 
 		Returns
@@ -391,6 +352,8 @@ class CommandAction(ActionBase):
 			Dictionary of tags(keys) and their values, with tags
 			within the values recursively replaced.
 		"""
+		# copy return tags dict to prevent modifying of inptags:
+		tags = inptags.copy()
 		t = TAG_CRE
 		# repeat substitution while embedded-recursive (repFlag is True)
 		while True:
@@ -467,7 +430,7 @@ class CommandAction(ActionBase):
 		return value
 
 	@classmethod
-	def replaceTag(cls, query, aInfo, conditional=''):
+	def replaceTag(cls, query, aInfo, conditional='', cache=None):
 		"""Replaces tags in `query` with property values.
 
 		Parameters
@@ -482,18 +445,32 @@ class CommandAction(ActionBase):
 		str
 			`query` string with tags replaced.
 		"""
+		# use cache if allowed:
+		if cache is not None:
+			ckey = (query, conditional)
+			string = cache.get(ckey)
+			if string is not None:
+				return string
+		# replace:
 		string = query
 		aInfo = cls.substituteRecursiveTags(aInfo, conditional)
 		for tag in aInfo:
 			if "<%s>" % tag in query:
-				value = str(aInfo[tag])			  # assure string
+				value = aInfo.get(tag + '?' + conditional)
+				if value is None:
+					value = aInfo.get(tag)
+				value = str(value)			  # assure string
 				if tag in cls._escapedTags:
 					# That one needs to be escaped since its content is
 					# out of our control
 					value = cls.escapeTag(value)
 				string = string.replace('<' + tag + '>', value)
-		# New line
+		# New line, space
 		string = reduce(lambda s, kv: s.replace(*kv), (("<br>", '\n'), ("<sp>", " ")), string)
+		# cache if properties:
+		if cache is not None:
+			cache[ckey] = string
+		#
 		return string
 
 	def _processCmd(self, cmd, aInfo=None, conditional=''):
@@ -520,6 +497,7 @@ class CommandAction(ActionBase):
 			self._logSys.debug("Nothing to do")
 			return True
 
+		# conditional corresponding family of the given ip:
 		if conditional == '':
 			conditional = 'family=inet4'
 			if allowed_ipv6:
@@ -530,7 +508,8 @@ class CommandAction(ActionBase):
 				except KeyError:
 					pass
 
-		checkCmd = self.replaceTag('<actioncheck>', self._properties, conditional=conditional)
+		checkCmd = self.replaceTag('<actioncheck>', self._properties, 
+			conditional=conditional, cache=self.__substCache)
 		if not self.executeCmd(checkCmd, self.timeout):
 			self._logSys.error(
 				"Invariant check failed. Trying to restore a sane environment")
@@ -541,7 +520,8 @@ class CommandAction(ActionBase):
 				return False
 
 		# Replace static fields
-		realCmd = self.replaceTag(cmd, self._properties, conditional=conditional)
+		realCmd = self.replaceTag(cmd, self._properties, 
+			conditional=conditional, cache=self.__substCache)
 
 		# Replace tags
 		if aInfo is not None:

fail2ban/tests/actiontestcase.py

@@ -139,6 +139,48 @@ class CommandActionTest(LogCaptureTestCase):
 			self.__action.replaceTag("abc",
 				CallingMap(matches=lambda: int("a"))), "abc")
 
+	def testReplaceTagConditionalCached(self):
+		setattr(self.__action, 'abc', "123")
+		setattr(self.__action, 'abc?family=inet4', "345")
+		setattr(self.__action, 'abc?family=inet6', "567")
+		setattr(self.__action, 'xyz', "890-<abc>")
+		setattr(self.__action, 'banaction', "Text <xyz> text <abc>")
+		# test replacement in sub tags and direct, conditional, cached:
+		cache = self.__action._substCache
+		for i in range(2):
+			self.assertEqual(
+				self.__action.replaceTag("<banaction>", self.__action._properties, 
+					conditional="", cache=cache),
+				"Text 890-123 text 123")
+			self.assertEqual(
+				self.__action.replaceTag("<banaction>", self.__action._properties, 
+					conditional="family=inet4", cache=cache),
+				"Text 890-345 text 345")
+			self.assertEqual(
+				self.__action.replaceTag("<banaction>", self.__action._properties, 
+					conditional="family=inet6", cache=cache),
+				"Text 890-567 text 567")
+		self.assertEqual(len(cache) if cache is not None else -1, 3)
+		# set one parameter - internal properties and cache should be reseted:
+		setattr(self.__action, 'xyz', "000-<abc>")
+		self.assertEqual(len(cache) if cache is not None else -1, 0)
+		# test againg, should have 000 instead of 890:
+		for i in range(2):
+			self.assertEqual(
+				self.__action.replaceTag("<banaction>", self.__action._properties, 
+					conditional="", cache=cache),
+				"Text 000-123 text 123")
+			self.assertEqual(
+				self.__action.replaceTag("<banaction>", self.__action._properties, 
+					conditional="family=inet4", cache=cache),
+				"Text 000-345 text 345")
+			self.assertEqual(
+				self.__action.replaceTag("<banaction>", self.__action._properties, 
+					conditional="family=inet6", cache=cache),
+				"Text 000-567 text 567")
+		self.assertEqual(len(cache), 3)
+
+
 	def testExecuteActionBan(self):
 		self.__action.actionstart = "touch /tmp/fail2ban.test"
 		self.assertEqual(self.__action.actionstart, "touch /tmp/fail2ban.test")