Merge "Detect group matches in replacements more robustly"

releasenotes/notes/group-subst-16dce373ccd24118.yaml

@@ -0,0 +1,3 @@
+fixes:
+  - Literal '$' characters are now handled correctly when they appear in
+    substitution strings for RedirectMatch directives.

whereto/rules.py

@@ -70,14 +70,19 @@ class Redirect(Rule):
 class RedirectMatch(Rule):
     "A RedirectMatch rule with a regular expression."
 
+    _group_subst = re.compile(r'(?<!\\)\$([0-9])')
+
     def __init__(self, linenum, *params):
         super(RedirectMatch, self).__init__(linenum, *params)
         self.regex = re.compile(self.pattern)
         if self.target:
-            self.target_repl = self.target.replace('$', '\\')
+            self.target_repl = self._get_target_repl()
         else:
             self.target_repl = None
 
+    def _get_target_repl(self):
+        return self._group_subst.sub(r'\\1', self.target).replace(r'\$', '$')
+
     def match(self, path):
         m = self.regex.search(path)
         if m:

whereto/tests/test_rules.py

@@ -106,6 +106,26 @@ class TestRedirectMatch(base.TestCase):
             rule.match('/user/foo'),
         )
 
+    def test_match_with_no_group_dollar(self):
+        rule = rules.RedirectMatch(
+            1,
+            'redirectmatch', '301', '^/user/(.*)$', '/pike/user/$a',
+        )
+        self.assertEqual(
+            ('301', '/pike/user/$a'),
+            rule.match('/user/foo'),
+        )
+
+    def test_match_with_group_escape(self):
+        rule = rules.RedirectMatch(
+            1,
+            'redirectmatch', '301', '^/user/(.*)$', '/pike/user/\\$1',
+        )
+        self.assertEqual(
+            ('301', '/pike/user/$1'),
+            rule.match('/user/foo'),
+        )
+
     def test_no_match(self):
         rule = rules.RedirectMatch(
             1,