18404810 linked image chokes on zonepaths which contain colons s12b44
authorEdward Pilatowicz <edward.pilatowicz@oracle.com>
Tue, 18 Mar 2014 14:46:17 -0700
changeset 3043 d0b8f7b429f3
parent 3042 4a135a864b2b
child 3044 7b051aa6466d
18404810 linked image chokes on zonepaths which contain colons
src/modules/client/api_errors.py
src/modules/client/linkedimage/zone.py
src/tests/cli/t_pkg_linked.py
--- a/src/modules/client/api_errors.py	Mon Mar 17 13:07:03 2014 -0700
+++ b/src/modules/client/api_errors.py	Tue Mar 18 14:46:17 2014 -0700
@@ -2397,7 +2397,7 @@
         """Used to collect ExpiredCertficate exceptions."""
 
         def __init__(self, errors):
-                
+
                 self.errors = []
 
                 assert (isinstance(errors, (list, tuple,
@@ -2693,6 +2693,7 @@
             child_path_notabs=None,
             child_unknown=None,
             cmd_failed=None,
+            cmd_output_invalid=None,
             detach_child_notsup=None,
             detach_from_parent=None,
             detach_parent_notsup=None,
@@ -2725,6 +2726,7 @@
                 self.child_path_notabs = child_path_notabs
                 self.child_unknown = child_unknown
                 self.cmd_failed = cmd_failed
+                self.cmd_output_invalid = cmd_output_invalid
                 self.detach_child_notsup = detach_child_notsup
                 self.detach_from_parent = detach_from_parent
                 self.detach_parent_notsup = detach_parent_notsup
@@ -2869,6 +2871,15 @@
                         err += _("\nAnd generated the following error "
                             "message:\n%(errout)s" % {"errout": errout})
 
+                if cmd_output_invalid is not None:
+                        (cmd, output) = cmd_output_invalid
+                        err = _(
+                            "The following subprocess:\n"
+                            "    %(cmd)s\n"
+                            "Generated the following unexpected output:\n"
+                            "%(output)s\n" %
+                            {"cmd": " ".join(cmd), "output": "\n".join(output)})
+
                 if detach_child_notsup is not None:
                         err = _("Linked image type does not support "
                             "child detach: %s") % detach_child_notsup
--- a/src/modules/client/linkedimage/zone.py	Mon Mar 17 13:07:03 2014 -0700
+++ b/src/modules/client/linkedimage/zone.py	Tue Mar 18 14:46:17 2014 -0700
@@ -407,6 +407,32 @@
         l = fout.readlines()[0].rstrip()
         return l
 
+def _zoneadm_list_parse(line, cmd, output):
+        """Parse zoneadm list -p output.  It's possible for zonepath to
+        contain a ":".  If it does it will be escaped to be "\:".  (But note
+        that if the zonepath contains a "\" it will not be escaped, which
+        is argubaly a bug.)"""
+
+        # zoneadm list output should never contain a NUL char, so
+        # temporarily replace any escaped colons with a NUL, split the string
+        # on any remaining colons, and then switch any NULs back to colons.
+        tmp_char = "\0"
+        fields = [
+                field.replace(tmp_char, ":")
+                for field in line.replace("\:", tmp_char).split(":")
+        ]
+
+        try:
+                # Unused variable; pylint: disable=W0612
+                z_id, z_name, z_state, z_path, z_uuid, z_brand, z_iptype = \
+                    fields[:7]
+                # pylint: enable=W0612
+        except ValueError:
+                raise apx.LinkedImageException(
+                    cmd_output_invalid=(cmd, output))
+
+        return z_name, z_state, z_path, z_brand
+
 def _list_zones(root, path_transform):
         """Get the zones associated with the image located at 'root'.  We
         return a dictionary where the keys are zone names and the values are
@@ -444,13 +470,25 @@
 
         # parse the command output
         fout.seek(0)
-        for l in fout.readlines():
+        output = fout.readlines()
+        for l in output:
                 l = l.rstrip()
 
-                # Unused variable; pylint: disable=W0612
-                z_id, z_name, z_state, z_path, z_uuid, z_brand, \
-                    z_iptype = l.strip().split(':', 6)
-                # pylint: enable=W0612
+                z_name, z_state, z_path, z_brand = \
+                    _zoneadm_list_parse(l, cmd, output)
+
+                # skip brands that we don't care about
+                # W0511 XXX / FIXME Comments; pylint: disable=W0511
+                # XXX: don't hard code brand names, use a brand attribute
+                # pylint: enable=W0511
+                if z_brand not in ["ipkg", "solaris", "sn1", "labeled"]:
+                        continue
+
+                # we don't care about the global zone.
+                if (z_name == "global"):
+                        continue
+
+                # append "/root" to zonepath
                 z_rootpath = os.path.join(z_path, "root")
                 assert z_rootpath.startswith(root), \
                     "zone path '%s' doesn't begin with '%s" % \
@@ -462,16 +500,6 @@
                         z_rootpath = li.path_transform_revert(z_rootpath,
                             path_transform)
 
-                # we don't care about the global zone.
-                if (z_name == "global"):
-                        continue
-
-                # W0511 XXX / FIXME Comments; pylint: disable=W0511
-                # XXX: don't hard code brand names, use a brand attribute
-                # pylint: enable=W0511
-                if z_brand not in ["ipkg", "solaris", "sn1", "labeled"]:
-                        continue
-
                 # we only care about zones that have been installed
                 if z_state not in zone_installed_states:
                         continue
--- a/src/tests/cli/t_pkg_linked.py	Mon Mar 17 13:07:03 2014 -0700
+++ b/src/tests/cli/t_pkg_linked.py	Tue Mar 18 14:46:17 2014 -0700
@@ -2485,7 +2485,7 @@
                 self._pkg([0, 1, 2], "list [email protected],5.11-0.1.1.0")
                 self._pkg([0, 1], "list [email protected],5.11-0.1.1.0")
                 self._pkg([2], "list network", rv=EXIT_OOPS)
-        
+
                 # Wildcards are purposefully used here for both patterns to
                 # ensure pattern matching works as expected for update.
                 self._pkg([0], "update -r -v "
@@ -4088,7 +4088,7 @@
 -:z2:unavailable:$PKG_GZR/z21::solaris:excl:-::
 -:z3:configured:$PKG_GZR/z3::solaris:excl:-::
 -:z4:incomplete:$PKG_GZR/z4::solaris:excl:-::
--:kz:installed:$PKG_GZR/kz1::solaris-kz:excl:-:solaris-kz:
+-:kz:installed:$PKG_GZR/system/volatile/zones/kz1/zonepath::solaris-kz:excl:-:solaris-kz:
 -:s10:installed:$PKG_GZR/s10::solaris10:excl:-::
 EOF
 exit 0""".strip("\n")
@@ -4834,6 +4834,84 @@
                 # Update the API object to point back to the old location.
                 api_inst._img.find_root(image1)
 
+        def test_linked_paths_bad_zoneadm_list_output(self):
+                """Test that we emit an error message if we fail to parse
+                zoneadm list -p output."""
+
+                base_path = self.img_path(0).rstrip(os.sep) + os.sep
+                gzpath = os.path.join(base_path, "gzpath/")
+                self.__ccmd("mkdir -p %s" % gzpath)
+
+                # fake zoneadm binary used for testing
+                zoneadm_sh = """
+#!/bin/sh
+cat <<-EOF
+this is invalid zoneadm list -p output.
+EOF
+exit 0""".strip("\n")
+
+                # create a zonename binary
+                bin_zonename = os.path.join(base_path, "zonename")
+                self.__mk_bin(bin_zonename, self.zonename_sh)
+
+                # create a zoneadm binary
+                bin_zoneadm = os.path.join(base_path, "zoneadm")
+                self.__mk_bin(bin_zoneadm, zoneadm_sh)
+
+                self.image_create(self.rurl1, destroy=False, img_path=gzpath)
+
+                self.pkg("--debug zones_supported=1 "
+                    "--debug bin_zonename='%s' "
+                    "--debug bin_zoneadm='%s' "
+                    "-R %s list-linked" %
+                    (bin_zonename, bin_zoneadm, gzpath), exit=EXIT_OOPS)
+
+                self.assert_(self.output == "")
+                self.assert_("this is invalid zoneadm list -p output." in
+                    self.errout)
+
+        def test_linked_paths_zone_paths_with_colon(self):
+                """Test that we can correctly parse zone paths that have a
+                colon in them."""
+
+                base_path = self.img_path(0).rstrip(os.sep) + os.sep
+                gzpath = os.path.join(base_path, "gzpath_with_a_:colon/")
+                self.__ccmd("mkdir -p %s" % gzpath)
+
+                os.environ["PKG_GZR"] = gzpath.rstrip(os.sep)
+
+
+                # fake zoneadm binary used for testing
+                zoneadm_sh = """
+#!/bin/sh
+while getopts "R:" OPT ; do
+case $OPT in
+        R )
+                [[ "$OPTARG" != "$PKG_GZR/" ]] && exit 0
+                ;;
+esac
+done
+PKG_GZR=$(echo "$PKG_GZR" | sed 's-:-\\\:-g')
+cat <<-EOF
+0:global:running:$PKG_GZR::solaris:shared:-:none:
+-:z1:installed:$PKG_GZR/ngzzone_path_with_a\:colon::solaris:excl:-::
+EOF
+exit 0""".strip("\n")
+
+                # create a zonename binary
+                bin_zonename = os.path.join(base_path, "zonename")
+                self.__mk_bin(bin_zonename, self.zonename_sh)
+
+                # create a zoneadm binary
+                bin_zoneadm = os.path.join(base_path, "zoneadm")
+                self.__mk_bin(bin_zoneadm, zoneadm_sh)
+
+                self.image_create(self.rurl1, destroy=False, img_path=gzpath)
+
+                ngzpath = gzpath + "ngzzone_path_with_a:colon/root/"
+                self.__list_linked_check(gzpath, [ngzpath],
+                    bin_zonename, bin_zoneadm)
+
 
 if __name__ == "__main__":
         unittest.main()