NodeSet: fix % escaping when using _fromlist1 initializer

clush uses the private optimized NodeSet._fromlist1 constructor, and
this one was lacking proper % character escaping. Cleaned up things so
that escaping is done by main EngineParser methods, and not in low level
_scan_* parsing methods anymore.

Closes #275.

Change-Id: Ib1f279a10ae82d655f0199cc753ed738a82e6ce5

Author: thiell

lib/ClusterShell/NodeSet.py

@@ -752,6 +752,14 @@ def __ixor__(self, other):
         return self
 
 
+def _strip_escape(nsstr):
+    """
+    Helper to prepare a nodeset string for parsing: trim boundary
+    whitespaces and escape special characters.
+    """
+    return nsstr.strip().replace('%', '%%')
+
+
 class ParsingEngine(object):
     """
     Class that is able to transform a source into a NodeSetBase.
@@ -801,6 +809,7 @@ def parse_string(self, nsstr, autostep, namespace=None):
         Return a NodeSetBase object.
         """
         nodeset = NodeSetBase()
+        nsstr = _strip_escape(nsstr)
 
         for opc, pat, rgnd in self._scan_string(nsstr, autostep):
             # Parser main debugging:
@@ -825,8 +834,8 @@ def parse_string(self, nsstr, autostep, namespace=None):
 
     def parse_string_single(self, nsstr, autostep):
         """Parse provided string and return a NodeSetBase object."""
-        # ignore node boundary whitespace(s)
-        pat, rangesets = self._scan_string_single(nsstr.strip(), autostep)
+        pat, rangesets = self._scan_string_single(_strip_escape(nsstr),
+                                                  autostep)
         if len(rangesets) > 1:
             rgobj = RangeSetND([rangesets], None, autostep, copy_rangeset=False)
         elif len(rangesets) == 1:
@@ -861,11 +870,14 @@ def parse_group_string(self, nodegroup, namespace=None):
         return ','.join(reslist), namespace
 
     def grouplist(self, namespace=None):
-        """Return a sorted list of groups from current resolver (in optional
-        group source / namespace)."""
+        """
+        Return a sorted list of groups from current resolver (in optional
+        group source / namespace).
+        """
         grpset = NodeSetBase()
         for grpstr in self.group_resolver.grouplist(namespace):
             # We scan each group string to expand any range seen...
+            grpstr = _strip_escape(grpstr)
             for opc, pat, rgnd in self._scan_string(grpstr, None):
                 getattr(grpset, opc)(NodeSetBase(pat, rgnd, False))
         return list(grpset)
@@ -947,20 +959,16 @@ def _scan_string_single(self, nsstr, autostep):
 
     def _scan_string(self, nsstr, autostep):
         """Parsing engine's string scanner method (iterator)."""
-        pat = nsstr.strip()
-        # avoid misformatting
-        if pat.find('%') >= 0:
-            pat = pat.replace('%', '%%')
         next_op_code = 'update'
-        while pat is not None:
+        while nsstr is not None:
             # Ignore whitespace(s) for convenience
-            pat = pat.lstrip()
+            nsstr = nsstr.lstrip()
 
             rsets = []
             op_code = next_op_code
 
-            op_idx, next_op_code = self._next_op(pat)
-            bracket_idx = pat.find(self.BRACKET_OPEN)
+            op_idx, next_op_code = self._next_op(nsstr)
+            bracket_idx = nsstr.find(self.BRACKET_OPEN)
 
             # Check if the operator is after the bracket, or if there
             # is no operator at all but some brackets.
@@ -970,13 +978,13 @@ def _scan_string(self, nsstr, autostep):
                 # Fill prefix, range and suffix from pattern
                 # eg. "forbin[3,4-10]-ilo" -> "forbin", "3,4-10", "-ilo"
                 newpat = ""
-                sfx = pat
+                sfx = nsstr
                 while bracket_idx >= 0 and (op_idx > bracket_idx or op_idx < 0):
                     pfx, sfx = sfx.split(self.BRACKET_OPEN, 1)
                     try:
                         rng, sfx = sfx.split(self.BRACKET_CLOSE, 1)
                     except ValueError:
-                        raise NodeSetParseError(pat, "missing bracket")
+                        raise NodeSetParseError(nsstr, "missing bracket")
 
                     # illegal closing bracket checks
                     if pfx.find(self.BRACKET_CLOSE) > -1:
@@ -995,7 +1003,7 @@ def _scan_string(self, nsstr, autostep):
 
                     # pfx + sfx cannot be empty
                     if pfxlen + sfxlen == 0:
-                        raise NodeSetParseError(pat, "empty node name")
+                        raise NodeSetParseError(nsstr, "empty node name")
 
                     if sfxlen > 0:
                         # amending trailing digits generates /steps
@@ -1031,12 +1039,12 @@ def _scan_string(self, nsstr, autostep):
                 # Check if we have a next op-separated node or pattern
                 op_idx, next_op_code = self._next_op(sfx)
                 if op_idx < 0:
-                    pat = None
+                    nsstr = None
                 else:
                     opc = self.OP_CODES[next_op_code]
-                    sfx, pat = sfx.split(opc, 1)
+                    sfx, nsstr = sfx.split(opc, 1)
                     # Detected character operator so right operand is mandatory
-                    if not pat:
+                    if not nsstr:
                         msg = "missing nodeset operand with '%s' operator" % opc
                         raise NodeSetParseError(None, msg)
 
@@ -1049,22 +1057,22 @@ def _scan_string(self, nsstr, autostep):
 
                 # pfx + sfx cannot be empty
                 if len(newpat) == 0:
-                    raise NodeSetParseError(pat, "empty node name")
+                    raise NodeSetParseError(nsstr, "empty node name")
 
             else:
                 # In this case, either there is no comma and no bracket,
                 # or the bracket is after the comma, then just return
                 # the node.
                 if op_idx < 0:
-                    node = pat
-                    pat = None # break next time
+                    node = nsstr
+                    nsstr = None # break next time
                 else:
                     opc = self.OP_CODES[next_op_code]
-                    node, pat = pat.split(opc, 1)
+                    node, nsstr = nsstr.split(opc, 1)
                     # Detected character operator so both operands are mandatory
-                    if not node or not pat:
+                    if not node or not nsstr:
                         msg = "missing nodeset operand with '%s' operator" % opc
-                        raise NodeSetParseError(node or pat, msg)
+                        raise NodeSetParseError(node or nsstr, msg)
 
                 # Check for illegal closing bracket
                 if node.find(self.BRACKET_CLOSE) > -1:

tests/NodeSetGroupTest.py

@@ -57,6 +57,8 @@ def makeTestG2():
 #
 para: montana[32-37,42-55]
 gpu: montana[38-41]
+escape%test: montana[87-90]
+esc%test2: @escape%test
 """)
     return f2
 
@@ -960,6 +962,36 @@ def testConfigCFGDIR(self):
         self.assertEqual(str(NodeSet("@%s" % tmpgroup, resolver=res)),
                          "example[1-100]")
 
+    def test_fromall_grouplist(self):
+        """test NodeSet.fromall() without all upcall"""
+        # Group Source that has no all upcall and that can handle special char
+        test_groups2 = makeTestG2()
+
+        source = UpcallGroupSource("simple",
+                                   "sed -n 's/^$GROUP:\(.*\)/\\1/p' %s" % test_groups2.name,
+                                   None,
+                                   "sed -n 's/^\([0-9A-Za-z_-\%%]*\):.*/\\1/p' %s"
+                                   % test_groups2.name,
+                                   None)
+        res = GroupResolver(source)
+
+        # fromall will trigger ParserEngine.grouplist() that we want to test here
+        nsall = NodeSet.fromall(resolver=res)
+
+        # if working, group resolution worked with % char
+        self.assertEqual(str(NodeSet.fromall(resolver=res)), "montana[32-55,87-90]")
+        self.assertEqual(len(nsall), 28)
+
+        # btw explicitly check escaped char
+        nsesc = NodeSet('@escape%test', resolver=res)
+        self.assertEqual(str(nsesc), 'montana[87-90]')
+        self.assertEqual(len(nsesc), 4)
+        nsesc2 = NodeSet('@esc%test2', resolver=res)
+        self.assertEqual(nsesc, nsesc2)
+        ns = NodeSet('montana[87-90]', resolver=res)
+        # could also result in escape%test?
+        self.assertEqual(ns.regroup(), '@esc%test2')
+
 
 class NodeSetGroup2GSTest(unittest.TestCase):
 

tests/NodeSetTest.py

@@ -100,6 +100,25 @@ def testNodeWithPercent(self):
         self.assertEqual(str(nodeset), "clust%ser[3-30]")
         nodeset = NodeSet("myclu%ster,clust%ser[3-30]")
         self.assertEqual(str(nodeset), "clust%ser[3-30],myclu%ster")
+        # issue #275
+        nodeset = NodeSet.fromlist(["cluster%eth0", "cluster%eth1"])
+        self.assertEqual(str(nodeset), "cluster%eth[0-1]")
+        nodeset = NodeSet.fromlist(["cluster%eth[0-8]", "cluster%eth9"])
+        self.assertEqual(str(nodeset), "cluster%eth[0-9]")
+        nodeset = NodeSet.fromlist(["super%cluster", "hyper%cluster"])
+        self.assertEqual(str(nodeset), "hyper%cluster,super%cluster")
+        # test also private _fromlist1 constructor
+        nodeset = NodeSet._fromlist1(["cluster%eth0", "cluster%eth1"])
+        self.assertEqual(str(nodeset), "cluster%eth[0-1]")
+        nodeset = NodeSet._fromlist1(["super%cluster", "hyper%cluster"])
+        self.assertEqual(str(nodeset), "hyper%cluster,super%cluster")
+        # real use-case!? exercise nD and escaping!
+        nodeset = NodeSet("fe80::5054:ff:feff:6944%eth0 ")
+        self._assertNode(nodeset, "fe80::5054:ff:feff:6944%eth0")
+        nodeset = NodeSet.fromlist(["fe80::5054:ff:feff:6944%eth0"])
+        self._assertNode(nodeset, "fe80::5054:ff:feff:6944%eth0")
+        nodeset = NodeSet._fromlist1(["fe80::5054:ff:feff:6944%eth0"])
+        self._assertNode(nodeset, "fe80::5054:ff:feff:6944%eth0")
 
     def testNodeEightPad(self):
         """test NodeSet padding feature"""