Attachment 901477 Details for Bug 1103639 – proposed fix

[patch] proposed fix

0001-Check-for-invalid-id-of-resource-group-property-cons.patch (text/plain), 9.81 KB, created by Tomas Jelinek on 2014-06-02 15:30:04 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomas Jelinek

Created: 2014-06-02 15:30:04 UTC

Size: 9.81 KB

patch

obsolete

>From aa4fe1af58c834b601bda9b5bda0bc2cad71fb43 Mon Sep 17 00:00:00 2001
>From: Tomas Jelinek <tojeline@redhat.com>
>Date: Mon, 2 Jun 2014 16:12:06 +0200
>Subject: [PATCH] Check for invalid id of resource, group, property, constraint
> and rule
>
>---
> pcs/constraint.py            |  5 +++++
> pcs/prop.py                  |  5 +++++
> pcs/resource.py              | 14 ++++++++++----
> pcs/test/Makefile            |  1 +
> pcs/test/test_constraints.py | 12 ++++++++++++
> pcs/test/test_properties.py  |  4 ++++
> pcs/test/test_resource.py    |  8 ++++++++
> pcs/test/test_utils.py       | 38 ++++++++++++++++++++++++++++++++++++++
> pcs/utils.py                 | 14 ++++++++++++++
> 9 files changed, 97 insertions(+), 4 deletions(-)
> create mode 100644 pcs/test/test_utils.py
>
>diff --git a/pcs/constraint.py b/pcs/constraint.py
>index e019245..9649efd 100644
>--- a/pcs/constraint.py
>+++ b/pcs/constraint.py
>@@ -755,6 +755,11 @@ def location_add(argv,rm=False):
>         node = ""
>         score = ""
>     else:
>+        if not utils.is_xml_id(constraint_id):
>+            utils.err(
>+                "invalid constraint id '%s', need to match %s"
>+                % (constraint_id, utils.get_xml_id_re().pattern)
>+            )
>         resource_name = argv.pop(0)
>         node = argv.pop(0)
>         score = argv.pop(0)
>diff --git a/pcs/prop.py b/pcs/prop.py
>index 2574478..ffb43f6 100644
>--- a/pcs/prop.py
>+++ b/pcs/prop.py
>@@ -32,6 +32,11 @@ def set_property(argv):
>         if "--node" in utils.pcs_options:
>             utils.set_node_attribute(args[0], args[1], utils.pcs_options["--node"])
>         elif ("--force" in utils.pcs_options) or utils.is_valid_property(args[0]):
>+            if not utils.is_xml_id(args[0]):
>+                utils.err(
>+                    "invalid property name '%s', need to match %s"
>+                    % (args[0], utils.get_xml_id_re().pattern)
>+                )
>             utils.set_cib_property(args[0],args[1])
>         else:
>             utils.err("unknown cluster property: '%s', (use --force to override)" % args[0])
>diff --git a/pcs/resource.py b/pcs/resource.py
>index 8f76b55..209a8a5 100644
>--- a/pcs/resource.py
>+++ b/pcs/resource.py
>@@ -307,8 +307,11 @@ def format_desc(indent, desc):
> # Create a resource using cibadmin
> # ra_class, ra_type & ra_provider must all contain valid info
> def resource_create(ra_id, ra_type, ra_values, op_values, meta_values=[], clone_opts=[]):
>-    if len(ra_id) == 0:
>-        utils.err ("resource id cannot be empty")
>+    if not utils.is_xml_id(ra_id):
>+        utils.err(
>+            "invalid resource name '%s', need to match %s"
>+            % (ra_id, utils.get_xml_id_re().pattern)
>+        )
> 
>     dom = utils.get_cib_dom()
> 
>@@ -1432,8 +1435,11 @@ def resource_group_add(group_name, resource_ids, passed_dom = None):
>     resources_element = top_element.getElementsByTagName("resources")[0]
>     group_found = False
> 
>-    if group_name == "":
>-        utils.err("group name cannot be empty")
>+    if not utils.is_xml_id(group_name):
>+        utils.err(
>+            "invalid group name '%s', need to match %s"
>+            % (group_name, utils.get_xml_id_re().pattern)
>+        )
> 
>     for resource in top_element.getElementsByTagName("primitive"):
>         if resource.getAttribute("id") == group_name:
>diff --git a/pcs/test/Makefile b/pcs/test/Makefile
>index 36e560a..c737e24 100644
>--- a/pcs/test/Makefile
>+++ b/pcs/test/Makefile
>@@ -8,6 +8,7 @@ ifneq ($(python_version_major_minor),2.6)
> endif
> 
> test:
>+	python test_utils.py ${pyunit_flags}
> 	python test_cluster.py ${pyunit_flags}
> 	python test_resource.py ${pyunit_flags}
> 	python test_constraints.py ${pyunit_flags}
>diff --git a/pcs/test/test_constraints.py b/pcs/test/test_constraints.py
>index df2e115..500ab26 100644
>--- a/pcs/test/test_constraints.py
>+++ b/pcs/test/test_constraints.py
>@@ -483,6 +483,14 @@ class ConstraintTest(unittest.TestCase):
>         ac (o,"Error: 'non-existant-resource' is not a resource\n")
>         assert r == 1
> 
>+        output, returnVal = pcs(temp_cib, "constraint rule add location-D1-rh7-1-INFINITY '#uname' eq rh7-2")
>+        ac(output, "Error: Unable to find constraint: location-D1-rh7-1-INFINITY\n")
>+        assert returnVal == 1
>+
>+        output, returnVal = pcs(temp_cib, "constraint rule add location-D2-rh7-2-INFINITY id=123 #uname eq rh7-2")
>+        ac(output, "Error: invalid rule id '123', need to match ^[a-zA-Z_][a-zA-Z0-9_.-]*$\n")
>+        assert returnVal == 1
>+
>     def testLocationBadRules(self):
>         o,r = pcs("resource create stateful0 Dummy --master")
>         ac(o,"")
>@@ -524,6 +532,10 @@ class ConstraintTest(unittest.TestCase):
>         ac(o,"Error: invalid score 'bar', use integer or INFINITY or -INFINITY\n")
>         assert r == 1
> 
>+        output, returnVal = pcs(temp_cib, "constraint location add loc:dummy D1 rh7-1 100")
>+        assert returnVal == 1
>+        ac(output, "Error: invalid constraint id 'loc:dummy', need to match ^[a-zA-Z_][a-zA-Z0-9_.-]*$\n")
>+
>     def testMasterSlaveConstraint(self):
>         os.system("CIB_file="+temp_cib+" cibadmin -R --scope nodes --xml-text '<nodes><node id=\"1\" uname=\"rh7-1\"/><node id=\"2\" uname=\"rh7-2\"/></nodes>'")
> 
>diff --git a/pcs/test/test_properties.py b/pcs/test/test_properties.py
>index 1db6ff7..7e6280b 100644
>--- a/pcs/test/test_properties.py
>+++ b/pcs/test/test_properties.py
>@@ -111,6 +111,10 @@ class PropertyTest(unittest.TestCase):
>         assert r==1
>         ac(o,"Error: unknown cluster property: 'xxxx', (use --force to override)\n")
> 
>+        output, returnVal = pcs("property set 1234=5678 --force")
>+        ac(output, "Error: invalid property name '1234', need to match ^[a-zA-Z_][a-zA-Z0-9_.-]*$\n")
>+        assert returnVal == 1
>+
>         o,r = pcs("property unset zzzzz")
>         assert r==1
>         ac(o,"Error: can't remove property: 'zzzzz' that doesn't exist\n")
>diff --git a/pcs/test/test_resource.py b/pcs/test/test_resource.py
>index 81228e7..ff1a739 100644
>--- a/pcs/test/test_resource.py
>+++ b/pcs/test/test_resource.py
>@@ -169,6 +169,10 @@ class ResourceTest(unittest.TestCase):
>         assert returnVal == 0
>         assert output == " Resource: bad_resource2 (class=ocf provider=heartbeat type=idontexist2)\n  Attributes: test4=bad3 \n  Operations: monitor interval=60s (bad_resource2-monitor-interval-60s)\n",[output]
> 
>+        output, returnVal = pcs(temp_cib, "resource create dum:my Dummy")
>+        assert returnVal == 1
>+        ac(output, "Error: invalid resource name 'dum:my', need to match ^[a-zA-Z_][a-zA-Z0-9_.-]*$\n")
>+
>     def testDeleteResources(self):
> # Verify deleting resources works
>         line = "resource create --no-default-ops ClusterIP ocf:heartbeat:IPaddr2 ip=192.168.0.99 cidr_netmask=32 op monitor interval=30s"
>@@ -629,6 +633,10 @@ class ResourceTest(unittest.TestCase):
>         assert returnVal == 0
>         ac(output, '')
> 
>+        output, returnVal = pcs(temp_cib, "resource group add group:dummy dummy1")
>+        assert returnVal == 1
>+        ac(output, "Error: invalid group name 'group:dummy', need to match ^[a-zA-Z_][a-zA-Z0-9_.-]*$\n")
>+
>     def testGroupOrder(self):
>         output, returnVal = pcs(temp_cib, "resource create --no-default-ops A Dummy")
>         output, returnVal = pcs(temp_cib, "resource create --no-default-ops B Dummy")
>diff --git a/pcs/test/test_utils.py b/pcs/test/test_utils.py
>new file mode 100644
>index 0000000..7c7e3a0
>--- /dev/null
>+++ b/pcs/test/test_utils.py
>@@ -0,0 +1,38 @@
>+import os
>+import sys
>+import unittest
>+parentdir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
>+sys.path.insert(0, parentdir)
>+import utils
>+
>+class UtilsTest(unittest.TestCase):
>+    def testIsXmlId(self):
>+        self.assertTrue(utils.is_xml_id("dummy"))
>+        self.assertTrue(utils.is_xml_id("DUMMY"))
>+        self.assertTrue(utils.is_xml_id("dUmMy"))
>+        self.assertTrue(utils.is_xml_id("dummy0"))
>+        self.assertTrue(utils.is_xml_id("dum0my"))
>+        self.assertTrue(utils.is_xml_id("dummy-"))
>+        self.assertTrue(utils.is_xml_id("dum-my"))
>+        self.assertTrue(utils.is_xml_id("dummy."))
>+        self.assertTrue(utils.is_xml_id("dum.my"))
>+        self.assertTrue(utils.is_xml_id("_dummy"))
>+        self.assertTrue(utils.is_xml_id("dummy_"))
>+        self.assertTrue(utils.is_xml_id("dum_my"))
>+
>+        self.assertFalse(utils.is_xml_id(""))
>+        self.assertFalse(utils.is_xml_id("0"))
>+        self.assertFalse(utils.is_xml_id("-"))
>+        self.assertFalse(utils.is_xml_id("."))
>+        self.assertFalse(utils.is_xml_id(":"))
>+        self.assertFalse(utils.is_xml_id("0dummy"))
>+        self.assertFalse(utils.is_xml_id("-dummy"))
>+        self.assertFalse(utils.is_xml_id(".dummy"))
>+        self.assertFalse(utils.is_xml_id(":dummy"))
>+        self.assertFalse(utils.is_xml_id("dum:my"))
>+        self.assertFalse(utils.is_xml_id("dummy:"))
>+
>+
>+
>+if __name__ == "__main__":
>+    unittest.main()
>diff --git a/pcs/utils.py b/pcs/utils.py
>index 295b31b..c4a2f9a 100644
>--- a/pcs/utils.py
>+++ b/pcs/utils.py
>@@ -970,6 +970,11 @@ def rule_add(elem, argv):
>                 subexpression = ET.SubElement(expression,"date_spec")
>             continue
>         if arg[0] == "id":
>+            if not is_xml_id(arg[1]):
>+                err(
>+                    "invalid rule id '%s', need to match %s"
>+                    % (arg[1], get_xml_id_re().pattern)
>+                )
>             rule.set(arg[0], arg[1])
>         elif arg[0] == "score":
>             if is_score_or_opt(arg[1]):
>@@ -1375,6 +1380,15 @@ def is_score(var):
>         var.isdigit() or (len(var) > 1 and var[0] == "-" and var[1:].isdigit())
>     )
> 
>+def get_xml_id_re():
>+    # see NCName definition
>+    # http://www.w3.org/TR/REC-xml-names/#NT-NCName
>+    # http://www.w3.org/TR/REC-xml/#NT-Name
>+    return re.compile("^[a-zA-Z_][a-zA-Z0-9_.-]*$")
>+
>+def is_xml_id(var):
>+    return False if get_xml_id_re().match(var) is None else True
>+
> def is_systemctl():
>     if os.path.exists('/usr/bin/systemctl'):
>         return True
>-- 
>1.9.1
>

Actions: View | Diff

Attachments on bug 1103639: 901477 | 907294