Attachment 896389 Details for Bug 1098466 – proposed fix

[patch] proposed fix

0001-Check-for-invalid-score-in-pcs-constraint-location.patch (text/plain), 3.47 KB, created by Tomas Jelinek on 2014-05-16 13:43:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomas Jelinek

Created: 2014-05-16 13:43:05 UTC

Size: 3.47 KB

patch

obsolete

>From b2ca5a6c0c654cc7ac308756b69a7f0f14f37936 Mon Sep 17 00:00:00 2001
>From: Tomas Jelinek <tojeline@redhat.com>
>Date: Fri, 16 May 2014 14:12:19 +0200
>Subject: [PATCH] Check for invalid score in 'pcs constraint location'
>
>* move score validation to function and make it not raising
>  IndexError on empty string
>* validate the score before puting it into the CIB
>* validate the score before changing its sign
>  * avoid raising IndexError
>  * avoid confusing user by score with opposite sign in error message
>    when using 'pcs constraint location <resource> avoids' command
>---
> pcs/constraint.py            |  4 ++++
> pcs/test/test_constraints.py | 12 ++++++++++++
> pcs/utils.py                 |  9 ++++++++-
> 3 files changed, 24 insertions(+), 1 deletion(-)
>
>diff --git a/pcs/constraint.py b/pcs/constraint.py
>index d20cbcc..e806403 100644
>--- a/pcs/constraint.py
>+++ b/pcs/constraint.py
>@@ -756,6 +756,8 @@ def location_prefer(argv):
>                 score = "-INFINITY"
>         else:
>             score = nodeconf_a[1]
>+            if not utils.is_score(score):
>+                utils.err("invalid score '%s', use integer or INFINITY or -INFINITY" % score)
>             if not prefer:
>                 if score[0] == "-":
>                     score = score[1:]
>@@ -784,6 +786,8 @@ def location_add(argv,rm=False):
>         # If resource doesn't exist, we error out
>         if not utils.is_valid_constraint_resource(resource_name):
>             utils.err("Resource " + resource_name + "' does not exist")
>+        if not utils.is_score(score):
>+            utils.err("invalid score '%s', use integer or INFINITY or -INFINITY" % score)
> 
>     # Verify current constraint doesn't already exist
>     # If it does we replace it with the new constraint
>diff --git a/pcs/test/test_constraints.py b/pcs/test/test_constraints.py
>index 9dcafa9..50ccb38 100644
>--- a/pcs/test/test_constraints.py
>+++ b/pcs/test/test_constraints.py
>@@ -491,6 +491,18 @@ class ConstraintTest(unittest.TestCase):
>         ac(o,"Error: '25' is not a valid rule expression\n")
>         assert r == 1
> 
>+        o,r = pcs("constraint location D1 prefers rh7-1=foo")
>+        ac(o,"Error: invalid score 'foo', use integer or INFINITY or -INFINITY\n")
>+        assert r == 1
>+
>+        o,r = pcs("constraint location D1 avoids rh7-1=")
>+        ac(o,"Error: invalid score '', use integer or INFINITY or -INFINITY\n")
>+        assert r == 1
>+
>+        o,r = pcs("constraint location add location1 D1 rh7-1 bar")
>+        ac(o,"Error: invalid score 'bar', use integer or INFINITY or -INFINITY\n")
>+        assert r == 1
>+
>     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/utils.py b/pcs/utils.py
>index bc154d4..252454f 100644
>--- a/pcs/utils.py
>+++ b/pcs/utils.py
>@@ -1370,12 +1370,19 @@ def is_valid_op_attr(attr):
> 
> # Test if 'var' is a score or option (contains an '=')
> def is_score_or_opt(var):
>-    if var == "INFINITY" or var == "-INFINITY" or (var[0] == "-" and var[1:].isdigit()) or var.isdigit():
>+    if is_score(var):
>         return True
>     elif var.find('=') != -1:
>         return True
>     return False
> 
>+def is_score(var):
>+    return (
>+        var == "INFINITY" or var == "-INFINITY"
>+        or
>+        var.isdigit() or (len(var) > 1 and var[0] == "-" and var[1:].isdigit())
>+    )
>+
> def is_systemctl():
>     if os.path.exists('/usr/bin/systemctl'):
>         return True
>-- 
>1.9.1
>

Actions: View | Diff

Attachments on bug 1098466: 896389