From 30fe5dae985644126e4832a3bd60178e98594abb Mon Sep 17 00:00:00 2001
From: Jan Hruban <jan.hruban@gooddata.com>
Date: Mon, 8 Jan 2018 17:11:49 +0100
Subject: [PATCH] Fix the formatter regex
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before:

{{var}} -> {var}, but the regex matches at the inner brace, but
re_replace returns the whole match (match.group(0) = "{var}") unchanged, passing
"{{var}}" to Formatter.

{{defined_var|def}} -> {{defined_var}}, because the regex matches at the
inner brace and re_replace returns "{%s}" % key ("{defined_var}"), passing just
"{{defined_var}}" to Formatter.

{{undefined_var|def}} -> exception, because the regex again matches at
the inner brace and re_replace returns default ("def"), passing "{def}"
to Formatter which then fails as def is not a defined variable (assuming
it isn't and allow_empty = False).

{{undefined_var|defined_var}} -> value of defined_var, same as above,
"{defined_var}" is passed to Formatter.

By preventing the regex from matching at inner braces, none of these
weird cases happen and even-braced strings are passed to Formatter
unmodified.

Change-Id: I8a74f9b4236ca7bcc72dd207fca23c2bf6a7c801
Co-Authored-By: Tomáš Janoušek <tomas.janousek@gooddata.com>
---
 jenkins_jobs/formatter.py                     | 12 +++++++---
 .../yamlparser/fixtures/variable_escaping.xml | 23 +++++++++++++++++++
 .../fixtures/variable_escaping.yaml           | 17 ++++++++++++++
 3 files changed, 49 insertions(+), 3 deletions(-)
 create mode 100644 tests/yamlparser/fixtures/variable_escaping.xml
 create mode 100644 tests/yamlparser/fixtures/variable_escaping.yaml

diff --git a/jenkins_jobs/formatter.py b/jenkins_jobs/formatter.py
index 008d66e74..31e9a3a40 100644
--- a/jenkins_jobs/formatter.py
+++ b/jenkins_jobs/formatter.py
@@ -81,19 +81,25 @@ class CustomFormatter(Formatter):
     Custom formatter to allow non-existing key references when formatting a
     string
     """
-    _expr = '{({{)*(?:obj:)?(?P<key>\w+)(?:\|(?P<default>[\w\s]*))?}(}})*'
+    _expr = """
+        (?<!{){({{)*                # non-pair opening {
+        (?:obj:)?                   # obj:
+        (?P<key>\w+)                # key
+        (?:\|(?P<default>[\w\s]*))? # default fallback
+        }(}})*(?!})                 # non-pair closing }
+    """
 
     def __init__(self, allow_empty=False):
         super(CustomFormatter, self).__init__()
         self.allow_empty = allow_empty
 
     def vformat(self, format_string, args, kwargs):
-        matcher = re.compile(self._expr)
+        matcher = re.compile(self._expr, re.VERBOSE)
 
         # special case of returning the object if the entire string
         # matches a single parameter
         try:
-            result = re.match('^%s$' % self._expr, format_string)
+            result = re.match('^%s$' % self._expr, format_string, re.VERBOSE)
         except TypeError:
             return format_string.format(**kwargs)
         if result is not None:
diff --git a/tests/yamlparser/fixtures/variable_escaping.xml b/tests/yamlparser/fixtures/variable_escaping.xml
new file mode 100644
index 000000000..16a37dd29
--- /dev/null
+++ b/tests/yamlparser/fixtures/variable_escaping.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="utf-8"?>
+<project>
+  <actions/>
+  <description>&lt;!-- Managed by Jenkins Job Builder --&gt;</description>
+  <keepDependencies>false</keepDependencies>
+  <blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
+  <blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
+  <concurrentBuild>false</concurrentBuild>
+  <canRoam>true</canRoam>
+  <properties/>
+  <scm class="hudson.scm.NullSCM"/>
+  <builders>
+    <hudson.tasks.Shell>
+      <command>echo &quot;{var}&quot;
+echo &quot;{defined_var|def}&quot;
+echo &quot;{undefined_var|def}&quot;
+echo &quot;{undefined_var|defined_var}&quot;
+</command>
+    </hudson.tasks.Shell>
+  </builders>
+  <publishers/>
+  <buildWrappers/>
+</project>
diff --git a/tests/yamlparser/fixtures/variable_escaping.yaml b/tests/yamlparser/fixtures/variable_escaping.yaml
new file mode 100644
index 000000000..44e8f7b44
--- /dev/null
+++ b/tests/yamlparser/fixtures/variable_escaping.yaml
@@ -0,0 +1,17 @@
+- project:
+    name: test_template_variable_escaping
+    jobs:
+        - 'template_variable_escaping':
+            defined_var: 'Hello'
+
+- job-template:
+    name: 'template_variable_escaping'
+    builders:
+      - shell: |
+          echo "{{var}}"
+          echo "{{defined_var|def}}"
+          echo "{{undefined_var|def}}"
+          echo "{{undefined_var|defined_var}}"
+
+
+