From 4a61de1e8aa2fbdedef736edeec3804640a9b5c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= <lsedlar@redhat.com>
Date: Tue, 12 Jun 2018 12:24:20 +0200
Subject: [PATCH] variants: Reject values with whitespace
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If there is leading or trailing whitespace in a comps group name, it
will not be included in the compose and there will even be no error
message. Whitespace on module name results in a failure.

To avoid these errors, validating the variants file will now also check
that there is no whitespace in significant places, and abort the compose
if there a problem.

Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
---
 pungi/wrappers/variants.py    | 34 +++++++++++++++++++++++++++++++++-
 tests/test_variant_wrapper.py | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_variant_wrapper.py

diff --git a/pungi/wrappers/variants.py b/pungi/wrappers/variants.py
index a57daaf3..36bc6182 100755
--- a/pungi/wrappers/variants.py
+++ b/pungi/wrappers/variants.py
@@ -39,6 +39,21 @@ def get_variants_dtd(logger=None):
     return variants_dtd
 
 
+class VariantsValidationError(ValueError):
+    pass
+
+
+NO_WHITESPACE_ELEMENTS = [
+    "arch",
+    "environment",
+    "group",
+    "kojitag",
+    "module",
+    "name",
+    "package",
+]
+
+
 class VariantsXmlParser(object):
     def __init__(self, file_obj, tree_arches=None, tree_variants=None, logger=None):
         self.tree = lxml.etree.parse(file_obj)
@@ -61,7 +76,24 @@ class VariantsXmlParser(object):
     def validate(self):
         if not self.dtd.validate(self.tree):
             errors = [str(i) for i in self.dtd.error_log.filter_from_errors()]
-            raise ValueError("Variants XML doesn't validate:\n%s" % "\n".join(errors))
+            raise VariantsValidationError(
+                "Variants XML doesn't validate:\n%s" % "\n".join(errors)
+            )
+
+        errors = []
+        for text in self.tree.xpath("//text()"):
+            if text != text.strip() and not text.is_tail:
+                e = text.getparent()
+                if e.tag in NO_WHITESPACE_ELEMENTS:
+                    errors.append(
+                        "Tag %s on line %s contains leading or trailing whitespace"
+                        % (e.tag, e.sourceline)
+                    )
+
+        if errors:
+            raise VariantsValidationError(
+                "Variants XML doesn't validate:\n%s" % "\n".join(errors)
+            )
 
     def parse_variant_node(self, variant_node, parent=None):
         variant_dict = {
diff --git a/tests/test_variant_wrapper.py b/tests/test_variant_wrapper.py
new file mode 100644
index 00000000..34cf31ac
--- /dev/null
+++ b/tests/test_variant_wrapper.py
@@ -0,0 +1,35 @@
+# -*- coding: utf-8 -*-
+
+try:
+    import unittest2 as unittest
+except ImportError:
+    import unittest
+import os
+import sys
+from six.moves import cStringIO
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
+
+from pungi.wrappers.variants import VariantsXmlParser
+
+VARIANTS_WITH_WHITESPACE = """
+<variants>
+  <variant id="Foo" name="Foo" type="variant">
+    <arches><arch>x86_64 </arch></arches>
+    <groups><group> core</group></groups>
+    <environments><environment> foo </environment></environments>
+  </variant>
+</variants>
+"""
+
+
+class TestVariantsXmlParser(unittest.TestCase):
+    def test_whitespace_in_file(self):
+        input = cStringIO(VARIANTS_WITH_WHITESPACE)
+
+        with self.assertRaises(ValueError) as ctx:
+            VariantsXmlParser(input)
+
+        self.assertIn("Tag arch on line 4", str(ctx.exception))
+        self.assertIn("Tag group on line 5", str(ctx.exception))
+        self.assertIn("Tag environment on line 6", str(ctx.exception))