Merge pull request #1419 from sebres/gh-1417

fixes gh-1417 Grave tags substitution bug. [part. cherry-picked from 0.10]

ChangeLog

@@ -15,6 +15,9 @@ ver. 0.9.5 (2016/XX/XXX) - wanna-be-released
      - failregex of previous monit version merged as single expression.
    * filter.d/postfix.conf, filter.d/postfix-sasl.conf
      - extended failregex daemon part, matching also `postfix/smtps/smtpd` now (gh-1391)
+   * fixed a grave bug within tags substitutions because of incorrect detection of recursion 
+     in case of multiple inline substitutions of the same tag (affected actions: `bsd-ipfw`, etc). 
+     Now tracks the actual list of the already substituted tags (per tag instead of single list)
 
 - New Features:
    * New Actions:

fail2ban/server/action.py

@@ -400,12 +400,16 @@ class CommandAction(ActionBase):
 				value = str(tags[tag])
 				# search and replace all tags within value, that can be interpolated using other tags:
 				m = t.search(value)
-				done = []
+				done = {}
+				last_found = tag
 				#logSys.log(5, 'TAG: %s, value: %s' % (tag, value))
 				while m:
 					found_tag = m.group(1)
 					#logSys.log(5, 'found: %s' % found_tag)
-					if found_tag == tag or found_tag in done:
+					curdone = done.get(last_found)
+					if curdone is None:
+						done[last_found] = curdone = []
+					if found_tag == tag or found_tag in curdone:
 						# recursive definitions are bad
 						#logSys.log(5, 'recursion fail tag: %s value: %s' % (tag, value) )
 						return False
@@ -417,7 +421,8 @@ class CommandAction(ActionBase):
 						continue
 					value = value.replace('<%s>' % found_tag , tags[found_tag])
 					#logSys.log(5, 'value now: %s' % value)
-					done.append(found_tag)
+					curdone.append(found_tag)
+					last_found = found_tag
 					m = t.search(value, m.start())
 				#logSys.log(5, 'TAG: %s, newvalue: %s' % (tag, value))
 				# was substituted?

fail2ban/tests/actiontestcase.py

@@ -29,6 +29,7 @@ import time
 import tempfile
 
 from ..server.action import CommandAction, CallingMap
+from ..server.actions import OrderedDict
 
 from .utils import LogCaptureTestCase
 from .utils import pid_exists
@@ -58,6 +59,12 @@ class CommandActionTest(LogCaptureTestCase):
 		# Unresolveable substition
 		self.assertFalse(CommandAction.substituteRecursiveTags({'A': 'to=<B> fromip=<IP>', 'C': '<B>', 'B': '<C>', 'D': ''}))
 		self.assertFalse(CommandAction.substituteRecursiveTags({'failregex': 'to=<honeypot> fromip=<IP>', 'sweet': '<honeypot>', 'honeypot': '<sweet>', 'ignoreregex': ''}))
+		# No-recursion, just multiple replacement of tag <T>, should be successful
+		if OrderedDict: # we need here an ordered, because the sequence of iteration is very important for this test
+			self.assertEqual(CommandAction.substituteRecursiveTags(
+				OrderedDict((('X', 'x=x<T>'), ('T', '1'), ('Z', '<X> <T> <Y>'), ('Y', 'y=y<T>')))
+				), {'X': 'x=x1', 'T': '1', 'Y': 'y=y1', 'Z': 'x=x1 1 y=y1'}
+			)
 		# missing tags are ok
 		self.assertEqual(CommandAction.substituteRecursiveTags({'A': '<C>'}), {'A': '<C>'})
 		self.assertEqual(CommandAction.substituteRecursiveTags({'A': '<C> <D> <X>','X':'fun'}), {'A': '<C> <D> fun', 'X':'fun'})