From 797cb67a37d67e41b2caa57aeeb6307b75b66bc8 Mon Sep 17 00:00:00 2001
From: Zane Bitter <zbitter@redhat.com>
Date: Wed, 8 Nov 2017 15:47:06 -0500
Subject: [PATCH] Use PCRE instead of Python's re for RedirectMatch tests

Previously, RedirectMatch rules were evaluated using Python's re regexp
module. However, Apache httpd uses the PCRE library for evaluating
regular expressions,[1] and there are subtle differences between the
Python and PCRE implementations. Using the PCRE library itself (via the
pcre-python binding) provides more representative results, and hence
more confidence to the user that any rules that pass whereto's tests
will work in the same way in a real Apache configuration.

[1] https://httpd.apache.org/docs/trunk/glossary.html#regex

Change-Id: Ibef3376d9da0688d0c97f5837dacc5b7cc52431c
---
 bindep.txt                                    |  3 +++
 releasenotes/notes/pcre-9c6e76b391620545.yaml |  4 ++++
 requirements.txt                              |  1 +
 tox.ini                                       |  4 ++++
 whereto/rules.py                              |  6 ++++--
 whereto/tests/test_rules.py                   | 19 +++++++++++++++++++
 6 files changed, 35 insertions(+), 2 deletions(-)
 create mode 100644 bindep.txt
 create mode 100644 releasenotes/notes/pcre-9c6e76b391620545.yaml

diff --git a/bindep.txt b/bindep.txt
new file mode 100644
index 0000000..eb11269
--- /dev/null
+++ b/bindep.txt
@@ -0,0 +1,3 @@
+libpcre3-dev [platform:dpkg]
+pcre-devel [platform:rpm]
+dev-libs/libpcre [platform:gentoo]
diff --git a/releasenotes/notes/pcre-9c6e76b391620545.yaml b/releasenotes/notes/pcre-9c6e76b391620545.yaml
new file mode 100644
index 0000000..8ad021d
--- /dev/null
+++ b/releasenotes/notes/pcre-9c6e76b391620545.yaml
@@ -0,0 +1,4 @@
+---
+features:
+  - whereto now uses the PCRE library - the same regex library used by
+    mod_alias - for compiling regular expressions.
diff --git a/requirements.txt b/requirements.txt
index 1d18dd3..ae2f7e8 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -3,3 +3,4 @@
 # process, which may cause wedges in the gate later.
 
 pbr>=2.0 # Apache-2.0
+python-pcre
diff --git a/tox.ini b/tox.ini
index 21c5397..4cc9171 100644
--- a/tox.ini
+++ b/tox.ini
@@ -25,6 +25,10 @@ commands = flake8 {posargs}
 basepython = python3
 commands = {posargs}
 
+[testenv:bindep]
+deps = bindep
+commands = bindep test
+
 [testenv:cover]
 commands = python setup.py test --coverage --testr-args='{posargs}'
 
diff --git a/whereto/rules.py b/whereto/rules.py
index 6acdac0..d70e671 100644
--- a/whereto/rules.py
+++ b/whereto/rules.py
@@ -16,6 +16,7 @@
 # under the License.
 
 import logging
+import pcre
 import re
 
 
@@ -74,14 +75,15 @@ class RedirectMatch(Rule):
 
     def __init__(self, linenum, *params):
         super(RedirectMatch, self).__init__(linenum, *params)
-        self.regex = re.compile(self.pattern)
+        self.regex = pcre.compile(self.pattern)
         if self.target:
             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'\$', '$')
+        escaped = pcre.escape_template(self.target)
+        return self._group_subst.sub(r'{\1}', escaped).replace(r'\$', '$')
 
     def match(self, path):
         m = self.regex.search(path)
diff --git a/whereto/tests/test_rules.py b/whereto/tests/test_rules.py
index fb379f4..3793494 100644
--- a/whereto/tests/test_rules.py
+++ b/whereto/tests/test_rules.py
@@ -96,6 +96,15 @@ class TestRedirectMatch(base.TestCase):
             rule.match('/user/'),
         )
 
+    def test_match_pcre_syntax(self):
+        rule = rules.RedirectMatch(
+            1,
+            'redirectmatch', '301', '^/((?i)pike)/user/.*$', '/pike/user/',
+        )
+        self.assertIsNone(
+            rule.match('/Pike/USER/')
+        )
+
     def test_match_with_group(self):
         rule = rules.RedirectMatch(
             1,
@@ -106,6 +115,16 @@ class TestRedirectMatch(base.TestCase):
             rule.match('/user/foo'),
         )
 
+    def test_match_with_group_braces(self):
+        rule = rules.RedirectMatch(
+            1,
+            'redirectmatch', '301', '^/user/(.*)$', '/pike/user/{1}/$1',
+        )
+        self.assertEqual(
+            ('301', '/pike/user/{1}/foo'),
+            rule.match('/user/foo'),
+        )
+
     def test_match_with_no_group_dollar(self):
         rule = rules.RedirectMatch(
             1,