3262 symlink loops can cause operation traceback
authorShawn Walker <shawn.walker@oracle.com>
Wed, 27 Jul 2011 13:42:21 -0700
changeset 2476 25342deb3749
parent 2475 77cfb483dd88
child 2477 161dd58eb684
3262 symlink loops can cause operation traceback 14805 action creation shouldn't allow multiple values for key attributes 14807 depend action doesn't validate fmri attribute syntax 17863 multiple values for standard pkg attributes can cause publication traceback 18629 validate_fsobj_common needs to check for lists of attribute values 18635 pkgdepend shouldn't require build-release in fmris 18717 traceback from pkgsend publish when -s forgotten 18718 pkgsend publish does not display errors 18719 depend action's clean_fmri needs to be sent to the dustbin 18720 action validation shouldn't permit strings for numeric attributes 18721 user action install doesn't preserve password and lastchg entries for existing users
src/man/pkg.5.txt
src/modules/actions/__init__.py
src/modules/actions/attribute.py
src/modules/actions/depend.py
src/modules/actions/directory.py
src/modules/actions/driver.py
src/modules/actions/file.py
src/modules/actions/generic.py
src/modules/actions/group.py
src/modules/actions/hardlink.py
src/modules/actions/legacy.py
src/modules/actions/license.py
src/modules/actions/link.py
src/modules/actions/signature.py
src/modules/actions/unknown.py
src/modules/actions/user.py
src/modules/client/imageplan.py
src/modules/publish/dependencies.py
src/modules/publish/transaction.py
src/publish.py
src/tests/api/t_action.py
src/tests/cli/t_pkg_install.py
src/tests/cli/t_pkgsend.py
src/tests/perf/actionbench.py
src/tests/pkg5unittest.py
--- a/src/man/pkg.5.txt	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/man/pkg.5.txt	Wed Jul 27 13:42:21 2011 -0700
@@ -584,7 +584,7 @@
                  of "true" indicates that the user is permitted to
                  login via FTP.  See ftpusers(4).
 
-     lastchng    The number of days between January 1, 1970,  and
+     lastchg     The number of days between January 1, 1970,  and
                  the  date  that  the password was last modified.
                  Default value is empty.  See shadow(4).
 
--- a/src/modules/actions/__init__.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/__init__.py	Wed Jul 27 13:42:21 2011 -0700
@@ -154,7 +154,7 @@
 
 
 class InvalidActionAttributesError(ActionError):
-        """Used to indicate that one more action attributes were invalid."""
+        """Used to indicate that one or more action attributes were invalid."""
 
         def __init__(self, act, errors, fmri=None):
                 """'act' is an Action (object or string).
@@ -207,14 +207,6 @@
                 raise UnknownActionError(string, atype)
 
         action = types[atype](data=data, **attr_dict)
-
-        if not action.key_attr_opt:
-                ka = action.key_attr
-                if ka is not None and (ka not in action.attrs or
-                    action.attrs[ka] is None):
-                        raise InvalidActionError(string,
-                            _("required attribute '%s' was not provided.") % ka)
-
         if ahash:
                 action.hash = ahash
 
@@ -290,14 +282,6 @@
                         "%s action cannot have a 'data' attribute" % atype)
 
         action = types[atype](data=None, **attrs)
-
-        ka = action.key_attr
-        if ka is not None and (ka not in action.attrs or
-            action.attrs[ka] is None):
-                raise InvalidActionError(("%s %s" % (atype,
-                    " ".join(args))).strip(), _("required attribute, "
-                    "'%s', was not provided.") % ka)
-
         if ahash:
                 action.hash = ahash
 
--- a/src/modules/actions/attribute.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/attribute.py	Wed Jul 27 13:42:21 2011 -0700
@@ -48,9 +48,13 @@
                 # For convenience, we allow people to express attributes as
                 # "<name>=<value>", rather than "name=<name> value=<value>", but
                 # we always convert to the latter.
-                if len(attrs) == 1:
-                        self.attrs["name"], self.attrs["value"] = \
-                            self.attrs.popitem()
+                try:
+                        if len(attrs) == 1:
+                                self.attrs["name"], self.attrs["value"] = \
+                                    self.attrs.popitem()
+                except KeyError:
+                        # Let error check below deal with this.
+                        pass
 
                 if "name" not in self.attrs or "value" not in self.attrs:
                         raise pkg.actions.InvalidActionError(str(self),
@@ -158,3 +162,27 @@
                                 cats = val
                         rval.append((scheme, cats))
                 return rval
+
+        def validate(self, fmri=None):
+                """Performs additional validation of action attributes that
+                for performance or other reasons cannot or should not be done
+                during Action object creation.  An ActionError exception (or
+                subclass of) will be raised if any attributes are not valid.
+                This is primarily intended for use during publication or during
+                error handling to provide additional diagonostics.
+
+                'fmri' is an optional package FMRI (object or string) indicating
+                what package contained this action.
+                """
+
+                name = self.attrs["name"]
+                if name in ("pkg.summary", "pkg.obsolete", "pkg.renamed",
+                    "pkg.description"):
+                        # If set action is for any of the above, only a single
+                        # value is permitted.
+                        generic.Action._validate(self, fmri=fmri,
+                            single_attrs=("value",))
+                else:
+                        # In all other cases, multiple values are assumed to be
+                        # permissible.
+                        generic.Action._validate(self, fmri=fmri)
--- a/src/modules/actions/depend.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/depend.py	Wed Jul 27 13:42:21 2011 -0700
@@ -32,6 +32,7 @@
 """
 
 import generic
+import pkg.actions
 import pkg.fmri
 import pkg.version
 
@@ -100,127 +101,11 @@
                         raise pkg.actions.InvalidActionError(
                             str(self), _("Missing type attribute"))
 
-                if "fmri" not in self.attrs:
-                        raise pkg.actions.InvalidActionError(
-                            str(self), _("Missing fmri attribute"))
-
-                if len(self.attrlist("fmri")) > 1 and \
-                    self.attrs["type"] != "require-any":
-                        raise pkg.actions.InvalidActionError(str(self),
-                            _("Multiple fmris specified for %s dependency type") %
-                            self.attrs["type"])
-
                 if self.attrs["type"] not in known_types:
                         raise pkg.actions.InvalidActionError(str(self),
                             _("Unknown type (%s) in depend action") %
                             self.attrs["type"])
 
-                if "type" == "conditional" and \
-                    "predicate" not in self.attrs:
-                        raise pkg.actions.InvalidActionError(str(self),
-                            _("No predicate specified for conditional dependency"))
-                try:
-                        if "fmri" in self.attrs:
-                                self.clean_fmri()
-                except ValueError:
-                        print "Warning: failed to clean FMRI: %s" % \
-                            self.attrs["fmri"]
-
-        def clean_fmri(self):
-                """ Clean up an invalid depend fmri into one which
-                we can recognize.
-
-                Example: 2.01.01.38-0.96  -> 2.1.1.38-0.96
-                This also corrects self.attrs["fmri"] as external code
-                knows about that, too."""
-
-                # This hack corrects a problem in pre-2008.11 packaging
-                # metadata: some depend actions were specified with invalid
-                # fmris of the form 2.38.01.01.3 (the padding zero is considered
-                # invalid).  When we get an invalid FMRI, we use regular
-                # expressions to perform a replacement operation which
-                # cleans up these problems.  It is hoped that someday this
-                # function may be removed completely once applicable releases
-                # are EOL'd.
-                #
-                # n.b. that this parser is not perfect: it will fix only
-                # the 'release' and 'branch' part of depend fmris-- these
-                # are the only places we've seen rules violations.
-                #
-                # Lots of things could go wrong here-- the caller should
-                # catch ValueError.
-                #
-                fmri_string = self.attrs["fmri"]
-                if not isinstance(fmri_string, basestring):
-                        return
-                #
-                # First, try to eliminate fmris that don't need cleaning since
-                # this process is relatively expensive (when considering tens
-                # of thousands of executions).  This currently leaves us with
-                # about 5-8% false positives, but is still a huge win overall.
-                # This won't account for cases like '[email protected]', but there
-                # are currently no known cases of that and the publication
-                # tools don't allow that syntax (currently) anyway.
-                #
-                if fmri_string.find(".0") == -1:
-                        # Nothing to do.
-                        return
-
-                #
-                # Next, locate the @ and the "," or "-" or ":" which
-                # is to the right of said @.
-                #
-                verbegin = fmri_string.find("@")
-                if verbegin == -1:
-                        return
-                verend = fmri_string.find(",", verbegin)
-                if verend == -1:
-                        verend = fmri_string.find("-", verbegin)
-                if verend == -1:
-                        verend = fmri_string.find(":", verbegin)
-                if verend == -1:
-                        verend = len(fmri_string)
-
-                # skip over the @ sign
-                verbegin += 1
-                verdots = fmri_string[verbegin:verend]
-                dots = verdots.split(".")
-
-                # Do the correction
-                cleanvers = ".".join([str(int(s)) for s in dots])
-
-                #
-                # Next, find the branch if it exists, the first '-'
-                # following the version.
-                #
-                branchbegin = fmri_string.find("-", verend)
-                if branchbegin != -1:
-                        branchend = fmri_string.find(":", branchbegin)
-                        if branchend == -1:
-                                branchend = len(fmri_string)
-
-                        # skip over the -
-                        branchbegin += 1
-                        branchdots = fmri_string[branchbegin:branchend]
-                        dots = branchdots.split(".")
-
-                        # Do the correction
-                        cleanbranch = ".".join([str(int(x)) for x in dots])
-
-                if branchbegin == -1:
-                        cleanfmri = fmri_string[:verbegin] + cleanvers + \
-                            fmri_string[verend:]
-                else:
-                        cleanfmri = fmri_string[:verbegin] + cleanvers + \
-                            fmri_string[verend:branchbegin] + cleanbranch + \
-                            fmri_string[branchend:]
-
-                # XXX enable if you need to debug
-                #if cleanfmri != fmri_string:
-                #       print "corrected invalid fmri: %s -> %s" % \
-                #           (fmri_string, cleanfmri)
-                self.attrs["fmri"] = cleanfmri
-
         def __check_parent_installed(self, image, fmri):
 
                 if not image.linked.ischild():
@@ -373,15 +258,18 @@
                             (image.avoid_set_get() | image.obsolete_set_get()):
                                 required = True
                 elif ctype == "require-any":
-                        for ifmri, pfmri in zip(installed_versions, pfmris):
-                                e = self.__check_installed(image, ifmri, pfmri, None, True, ctype)
-                                if ifmri and not e: # this one is present and happy
+                        for ifmri, rpfmri in zip(installed_versions, pfmris):
+                                e = self.__check_installed(image, ifmri, rpfmri,
+                                    None, True, ctype)
+                                if ifmri and not e:
+                                        # this one is present and happy
                                         return [], [], []
                                 else:
                                         errors.extend(e)
 
                         if not errors: # none was installed
-                                errors.append(_("Required dependency on one of %s not met") %
+                                errors.append(_("Required dependency on one of "
+                                    "%s not met") %
                                     ", ".join((str(p) for p in pfmris)))
                         return errors, warnings, info
 
@@ -434,3 +322,51 @@
                                 stem = p.split("@")[0]
                                 inds.append(("depend", ctype, stem, None))
                 return inds
+
+        def validate(self, fmri=None):
+                """Performs additional validation of action attributes that
+                for performance or other reasons cannot or should not be done
+                during Action object creation.  An ActionError exception (or
+                subclass of) will be raised if any attributes are not valid.
+                This is primarily intended for use during publication or during
+                error handling to provide additional diagonostics.
+
+                'fmri' is an optional package FMRI (object or string) indicating
+                what package contained this action."""
+
+                required_attrs = ["type"]
+                dtype = self.attrs.get("type")
+                if dtype == "conditional":
+                        required_attrs.append("predicate")
+
+                errors = generic.Action._validate(self, fmri=fmri,
+                    raise_errors=False, required_attrs=required_attrs,
+                    single_attrs=("predicate", "root-image"))
+
+                if "predicate" in self.attrs and dtype != "conditional":
+                        errors.append(("predicate", _("a predicate may only be "
+                            "specified for conditional dependencies")))
+                if "root-image" in self.attrs and dtype != "origin":
+                        errors.append(("root-image", _("the root-image "
+                            "attribute is only valid for origin dependencies")))
+
+                # Logic here intentionally treats 'predicate' and 'fmri' as
+                # having multiple values for simplicity.
+                for attr in ("predicate", "fmri"):
+                        for f in self.attrlist(attr):
+                                try:
+                                        pkg.fmri.PkgFmri(f, "5.11")
+                                except (pkg.version.VersionError,
+                                    pkg.fmri.FmriError), e:
+                                        if attr == "fmri" and f == "__TBD":
+                                                # pkgdepend uses this special
+                                                # value.
+                                                continue
+                                        errors.append((attr, _("invalid "
+                                            "%(attr)s value '%(value)s': "
+                                            "%(error)s") % { "attr": attr,
+                                            "value": f, "error": str(e) }))
+
+                if errors:
+                        raise pkg.actions.InvalidActionAttributesError(self,
+                            errors, fmri=fmri)
--- a/src/modules/actions/directory.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/directory.py	Wed Jul 27 13:42:21 2011 -0700
@@ -259,4 +259,9 @@
                 'fmri' is an optional package FMRI (object or string) indicating
                 what package contained this action."""
 
-                return self.validate_fsobj_common(fmri=fmri)
+                errors = generic.Action._validate(self, fmri=fmri,
+                    raise_errors=False, required_attrs=("owner", "group"))
+                errors.extend(self._validate_fsobj_common())
+                if errors:
+                        raise pkg.actions.InvalidActionAttributesError(self,
+                            errors, fmri=fmri)
--- a/src/modules/actions/driver.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/driver.py	Wed Jul 27 13:42:21 2011 -0700
@@ -650,8 +650,7 @@
                                 else:
                                         raise
 
-                act = cls()
-                act.attrs["name"] = name
+                act = cls(name=name)
 
                 # Grab aliases
                 try:
--- a/src/modules/actions/file.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/file.py	Wed Jul 27 13:42:21 2011 -0700
@@ -34,13 +34,13 @@
 import tempfile
 import stat
 import generic
+
+import pkg.actions
+import pkg.client.api_errors as api_errors
 import pkg.misc as misc
 import pkg.portable as portable
-import pkg.client.api_errors as api_errors
-import pkg.actions
 
 from pkg.client.api_errors import ActionExecutionError
-from pkg.client.debugvalues import DebugValues
 
 try:
         import pkg.elf as elf
@@ -606,4 +606,12 @@
                 'fmri' is an optional package FMRI (object or string) indicating
                 what package contained this action."""
 
-                return self.validate_fsobj_common(fmri=fmri)
+                errors = generic.Action._validate(self, fmri=fmri,
+                    numeric_attrs=("pkg.csize", "pkg.size"), raise_errors=False,
+                    required_attrs=("owner", "group"), single_attrs=("chash",
+                    "preserve", "overlay", "elfarch", "elfbits", "elfhash",
+                    "original_name"))
+                errors.extend(self._validate_fsobj_common())
+                if errors:
+                        raise pkg.actions.InvalidActionAttributesError(self,
+                            errors, fmri=fmri)
--- a/src/modules/actions/generic.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/generic.py	Wed Jul 27 13:42:21 2011 -0700
@@ -43,6 +43,25 @@
 import pkg.variant as variant
 import stat
 
+# Used to define sort order between action types.
+_ORDER_DICT_LIST = [
+    "set",
+    "depend",
+    "group",
+    "user",
+    "dir",
+    "file",
+    "hardlink",
+    "link",
+    "driver",
+    "unknown",
+    "legacy",
+    "signature"
+]
+
+# EmptyI for argument defaults; no import to avoid pkg.misc dependency.
+EmptyI = tuple()
+
 class NSG(type):
         """This metaclass automatically assigns a subclass of Action a
         namespace_group member if it hasn't already been specified.  This is a
@@ -78,6 +97,7 @@
 
                 return type.__new__(mcs, name, bases, dict)
 
+
 class Action(object):
         """Class representing a generic packaging object.
 
@@ -97,8 +117,6 @@
         # key_attr would be the driver name.  When 'key_attr' is None, it means
         # that all attributes of the action are distinguishing.
         key_attr = None
-        # 'key_attr_opt' indicates if the 'key_attr' attribute is optional.
-        key_attr_opt = False
         # 'globally_identical' is True if all actions representing a single
         # object on a system must be identical.
         globally_identical = False
@@ -135,23 +153,10 @@
         __metaclass__ = NSG
 
         def loadorderdict(self):
-                ol = [
-                        "set",
-                        "depend",
-                        "group",
-                        "user",
-                        "dir",
-                        "file",
-                        "hardlink",
-                        "link",
-                        "driver",
-                        "unknown",
-                        "legacy",
-                        "signature"
-                        ]
                 self.orderdict.update(dict((
-                    (pkg.actions.types[t], i) for i, t in enumerate(ol)
-                    )))
+                    (pkg.actions.types[t], i)
+                    for i, t in enumerate(_ORDER_DICT_LIST)
+                )))
                 self.__class__.unknown = \
                     self.orderdict[pkg.actions.types["unknown"]]
 
@@ -193,7 +198,14 @@
 
                 if not self.orderdict:
                         self.loadorderdict()
-                self.ord = self.orderdict.get(type(self), self.unknown)
+
+
+                # A try/except is used here instead of get() as it is
+                # consistently 2% faster.
+                try:
+                        self.ord = self.orderdict[type(self)]
+                except KeyError:
+                        self.ord = self.unknown
 
                 self.attrs = attrs
 
@@ -204,19 +216,44 @@
                 else:
                         self.set_data(data)
 
-                if not self._strip_path or "path" not in self.attrs:
+                if not self.key_attr:
+                        # Nothing more to do.
                         return
 
+                # Test if method only string object will have is defined to
+                # determine if key attribute has been specified multiple times.
                 try:
-                        self.attrs["path"] = self.attrs["path"].lstrip("/")
+                        self.attrs[self.key_attr].decode
+                except KeyError:
+                        if self.name == "set" or self.name == "signature":
+                                # Special case for set and signature actions
+                                # since they allow two forms of syntax.
+                                return
+                        raise pkg.actions.InvalidActionError(str(self),
+                           _("no value specified for key attribute '%s'") %
+                           self.key_attr)
                 except AttributeError:
-                        raise pkg.actions.InvalidActionError(
-                            str(self), _("path attribute specified multiple "
-                                "times"))
+                       if self.name != "depend" or \
+                           self.attrs.get("type") != "require-any":
+                                # Assume list since fromstr() will only ever
+                                # provide a string or list and decode method
+                                # wasn't found.  This is much faster than
+                                # checking isinstance or 'type() ==' in
+                                # a hot path.
+                                raise pkg.actions.InvalidActionError(str(self),
+                                   _("%s attribute may only be specified "
+                                   "once") % self.key_attr)
 
-                if not self.attrs["path"]:
-                        raise pkg.actions.InvalidActionError(
-                            str(self), _("Empty path attribute"))
+                if self._strip_path:
+                        # Strip leading slash from path if requested.
+                        try:
+                                self.attrs["path"] = \
+                                    self.attrs["path"].lstrip("/")
+                        except KeyError:
+                                return
+                        if not self.attrs["path"]:
+                                raise pkg.actions.InvalidActionError(
+                                    str(self), _("Empty path attribute"))
 
         def set_data(self, data):
                 """This function sets the data field of the action.
@@ -521,8 +558,6 @@
 
                 if self.key_attr is None:
                         return str(self)
-                if self.key_attr_opt and self.key_attr not in self.attrs:
-                        return str(self)
                 return "%s: %s" % \
                     (self.name, self.attrs.get(self.key_attr, "???"))
 
@@ -574,7 +609,7 @@
                                     locals()
                                 raise apx.ActionExecutionError(self,
                                     details=err_txt, error=e,
-                                    fmri=kw.get("fmri", None))
+                                    fmri=kw.get("fmri"))
                 else:
                         # XXX Because the filelist codepath may create
                         # directories with incorrect permissions (see
@@ -613,7 +648,7 @@
                                     locals()
                                 raise apx.ActionExecutionError(self,
                                     details=err_txt, error=e,
-                                    fmri=kw.get("fmri", None))
+                                    fmri=kw.get("fmri"))
 
                         os.chmod(p, fs.st_mode)
                         try:
@@ -670,14 +705,16 @@
                 correctly installed in the given image."""
                 return [], [], []
 
-        def validate_fsobj_common(self, fmri=None):
-                """Common validation logic for filesystem objects."""
+        def _validate_fsobj_common(self):
+                """Private, common validation logic for filesystem objects that
+                returns a list of tuples of the form (attr_name, error_message).
+                """
 
                 errors = []
 
                 bad_mode = False
-                raw_mode = self.attrs.get("mode", None)
-                if not raw_mode:
+                raw_mode = self.attrs.get("mode")
+                if not raw_mode or isinstance(raw_mode, list):
                         bad_mode = True
                 else:
                         mlen = len(raw_mode)
@@ -707,22 +744,27 @@
                                 errors.append(("mode", _("mode is required; "
                                     "value must be of the form '644', "
                                     "'0644', or '04755'.")))
+                        elif isinstance(raw_mode, list):
+                                errors.append("mode", _("mode may only be "
+                                    "specified once"))
                         else:
                                 errors.append(("mode", _("'%s' is not a valid "
                                     "mode; value must be of the form '644', "
                                     "'0644', or '04755'.") % raw_mode))
 
-                owner = self.attrs.get("owner", "").rstrip()
-                if not owner:
-                        errors.append(("owner", _("owner is required")))
+                try:
+                        owner = self.attrs.get("owner", "").rstrip()
+                except AttributeError:
+                        errors.append(("owner", _("owner may only be specified "
+                            "once")))
 
-                group = self.attrs.get("group", "").rstrip()
-                if not group:
-                        errors.append(("group", _("group is required")))
+                try:
+                        group = self.attrs.get("group", "").rstrip()
+                except AttributeError:
+                        errors.append(("group", _("group may only be specified "
+                            "once")))
 
-                if errors:
-                        raise pkg.actions.InvalidActionAttributesError(self,
-                            errors, fmri=fmri)
+                return errors
 
         def get_fsobj_uid_gid(self, pkgplan, fmri):
                 """Returns a tuple of the form (owner, group) containing the uid
@@ -1047,8 +1089,55 @@
                 error handling to provide additional diagonostics.
 
                 'fmri' is an optional package FMRI (object or string) indicating
-                what package contained this action."""
-                pass
+                what package contained this action.
+                """
+
+                self._validate(fmri=fmri)
+
+        def _validate(self, fmri=None, numeric_attrs=EmptyI,
+             raise_errors=True, required_attrs=EmptyI, single_attrs=EmptyI):
+                """Common validation logic for all action types.
+
+                'fmri' is an optional package FMRI (object or string) indicating
+                what package contained this action.
+
+                'numeric_attrs' is a list of attributes that must have an
+                integer value.
+
+                'raise_errors' is a boolean indicating whether errors should be
+                raised as an exception or returned as a list of tuples of the
+                form (attr_name, error_message).
+
+                'single_attrs' is a list of attributes that should only be
+                specified once.
+                """
+
+                errors = []
+                for attr in self.attrs:
+                        if ((attr.startswith("facet.") or
+                            attr.startswith("variant.") or
+                            attr == "reboot-needed" or attr in single_attrs) and
+                            isinstance(self.attrs[attr], list)):
+                                errors.append((attr, _("%s may only be "
+                                    "specified once") % attr))
+                        elif attr in numeric_attrs:
+                                try:
+                                        int(self.attrs[attr])
+                                except (TypeError, ValueError):
+                                        errors.append((attr, _("%s must be an "
+                                            "integer") % attr))
+
+                for attr in required_attrs:
+                        val = self.attrs.get(attr)
+                        if not val or \
+                            (isinstance(val, basestring) and not val.strip()):
+                                errors.append((attr, _("%s is required") %
+                                    attr))
+
+                if raise_errors and errors:
+                        raise pkg.actions.InvalidActionAttributesError(self,
+                            errors, fmri=fmri)
+                return errors
 
         def fsobj_checkpath(self, pkgplan, final_path):
                 """Verifies that the specified path doesn't contain one or more
@@ -1090,6 +1179,6 @@
                 err_txt = _("Cannot install '%(final_path)s'; parent directory "
                     "%(parent_dir)s is a link to %(parent_target)s.  To "
                     "continue, move the directory to its original location and "
-                    "try again.") % locals() 
+                    "try again.") % locals()
                 raise apx.ActionExecutionError(self, details=err_txt,
                     fmri=fmri)
--- a/src/modules/actions/group.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/group.py	Wed Jul 27 13:42:21 2011 -0700
@@ -21,7 +21,7 @@
 #
 
 #
-# Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2008, 2011, Oracle and/or its affiliates. All rights reserved.
 #
 
 """module describing a user packaging object
@@ -174,3 +174,17 @@
 
                 return [("group", "name", self.attrs["groupname"], None)]
 
+        def validate(self, fmri=None):
+                """Performs additional validation of action attributes that
+                for performance or other reasons cannot or should not be done
+                during Action object creation.  An ActionError exception (or
+                subclass of) will be raised if any attributes are not valid.
+                This is primarily intended for use during publication or during
+                error handling to provide additional diagonostics.
+
+                'fmri' is an optional package FMRI (object or string) indicating
+                what package contained this action.
+                """
+
+                generic.Action._validate(self, fmri=fmri,
+                    numeric_attrs=("gid",), single_attrs=("gid",))
--- a/src/modules/actions/hardlink.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/hardlink.py	Wed Jul 27 13:42:21 2011 -0700
@@ -35,10 +35,8 @@
 import stat
 
 from pkg import misc
-import pkg.actions
 from pkg.client.api_errors import ActionExecutionError
 
-
 class HardLinkAction(link.LinkAction):
         """Class representing a hardlink-type packaging object."""
 
--- a/src/modules/actions/legacy.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/legacy.py	Wed Jul 27 13:42:21 2011 -0700
@@ -21,7 +21,7 @@
 #
 
 #
-# Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2007, 2011, Oracle and/or its affiliates. All rights reserved.
 #
 
 """module describing a legacy packaging object
@@ -178,3 +178,18 @@
                 generic.py for a more detailed explanation."""
 
                 return [("legacy", "legacy_pkg", self.attrs["pkg"], None)]
+
+        def validate(self, fmri=None):
+                """Performs additional validation of action attributes that
+                for performance or other reasons cannot or should not be done
+                during Action object creation.  An ActionError exception (or
+                subclass of) will be raised if any attributes are not valid.
+                This is primarily intended for use during publication or during
+                error handling to provide additional diagonostics.
+
+                'fmri' is an optional package FMRI (object or string) indicating
+                what package contained this action."""
+
+                generic.Action._validate(self, fmri=fmri,
+                    single_attrs=("category", "desc", "hotline", "name",
+                    "vendor", "version"))
--- a/src/modules/actions/license.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/license.py	Wed Jul 27 13:42:21 2011 -0700
@@ -258,3 +258,17 @@
 
                 return self.attrs.get("must-display", "").lower() == "true"
 
+        def validate(self, fmri=None):
+                """Performs additional validation of action attributes that
+                for performance or other reasons cannot or should not be done
+                during Action object creation.  An ActionError exception (or
+                subclass of) will be raised if any attributes are not valid.
+                This is primarily intended for use during publication or during
+                error handling to provide additional diagonostics.
+
+                'fmri' is an optional package FMRI (object or string) indicating
+                what package contained this action."""
+
+                generic.Action._validate(self, fmri=fmri,
+                    numeric_attrs=("pkg.csize", "pkg.size"),
+                    single_attrs=("chash", "must-accept", "must-display"))
--- a/src/modules/actions/link.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/link.py	Wed Jul 27 13:42:21 2011 -0700
@@ -31,13 +31,14 @@
 
 import errno
 import os
-import re
 import stat
 
 import generic
 import pkg.actions
 import pkg.mediator as med
+
 from pkg import misc
+from pkg.client.api_errors import ActionExecutionError
 
 class LinkAction(generic.Action):
         """Class representing a link-type packaging object."""
@@ -137,10 +138,18 @@
                 'fmri' is an optional package FMRI (object or string) indicating
                 what package contained this action."""
 
+                errors = generic.Action._validate(self, fmri=fmri,
+                    raise_errors=False, required_attrs=("target",),
+                    single_attrs=("target", "mediator", "mediator-version",
+                    "mediator-implementation", "mediator-priority"))
+
                 if "mediator" not in self.attrs and \
                     "mediator-version" not in self.attrs and \
                     "mediator-implementation" not in self.attrs and \
                     "mediator-priority" not in self.attrs:
+                        if errors:
+                                raise pkg.actions.InvalidActionAttributesError(
+                                    self, errors, fmri=fmri)
                         return
 
                 mediator = self.attrs.get("mediator")
@@ -148,17 +157,14 @@
                 med_implementation = self.attrs.get("mediator-implementation")
                 med_priority = self.attrs.get("mediator-priority")
 
-                errors = []
                 if not mediator and (med_version or med_implementation or
                     med_priority):
                         errors.append(("mediator", _("a mediator must be "
                             "provided when mediator-version, "
                             "mediator-implementation, or mediator-priority "
                             "is specified")))
-                if isinstance(mediator, list):
-                        errors.append(("mediator", _("only one mediator may be "
-                            "specified")))
-                else:
+                elif mediator is not None and \
+                    not isinstance(mediator, list):
                         valid, error = med.valid_mediator(mediator)
                         if not valid:
                                 errors.append(("mediator", error))
@@ -168,28 +174,22 @@
                             "mediator-implementation must be provided if a "
                             "mediator is specified")))
 
-                if isinstance(med_version, list):
-                        errors.append(("mediator-version", _("only one "
-                            "mediator-version may be specified")))
-                elif "mediator-version" in self.attrs:
+                if med_version is not None and \
+                    not isinstance(med_version, list):
                         valid, error = med.valid_mediator_version(med_version)
                         if not valid:
                                 errors.append(("mediator-version", error))
 
-                if isinstance(med_implementation, list):
-                        errors.append(("mediator-implementation", _("only one "
-                            "mediator-implementation may be specified")))
-                elif "mediator-implementation" in self.attrs:
+                if med_implementation is not None and \
+                    not isinstance(med_implementation, list):
                         valid, error = med.valid_mediator_implementation(
                             med_implementation)
                         if not valid:
                                 errors.append(("mediator-implementation",
                                     error))
 
-                if isinstance(med_priority, list):
-                        errors.append(("mediator-priority", _("only one "
-                            "mediator-priority may be specified")))
-                elif "mediator-priority" in self.attrs:
+                if med_priority is not None and \
+                    not isinstance(med_priority, list):
                         valid, error = med.valid_mediator_priority(med_priority)
                         if not valid:
                                 errors.append(("mediator-priority", error))
--- a/src/modules/actions/signature.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/signature.py	Wed Jul 27 13:42:21 2011 -0700
@@ -459,3 +459,23 @@
                 generic.Action.__setstate__(self, pstate)
                 for name in state:
                         setattr(self, name, state[name])
+
+        def validate(self, fmri=None):
+                """Performs additional validation of action attributes that
+                for performance or other reasons cannot or should not be done
+                during Action object creation.  An ActionError exception (or
+                subclass of) will be raised if any attributes are not valid.
+                This is primarily intended for use during publication or during
+                error handling to provide additional diagonostics.
+
+                'fmri' is an optional package FMRI (object or string) indicating
+                what package contained this action.
+                """
+
+                # 'value' can only be required at publication time since signing
+                # relies on the ability to construct actions without one despite
+                # the fact that it is the key attribute.
+                generic.Action._validate(self, fmri=fmri,
+                    numeric_attrs=("pkg.csize", "pkg.size"),
+                    required_attrs=("value",), single_attrs=("algorithm",
+                    "chash", "value"))
--- a/src/modules/actions/unknown.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/unknown.py	Wed Jul 27 13:42:21 2011 -0700
@@ -21,8 +21,7 @@
 #
 
 #
-# Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
-# Use is subject to license terms.
+# Copyright (c) 2008, 2011, Oracle and/or its affiliates. All rights reserved.
 #
 
 """module describing a unknown packaging object
@@ -40,7 +39,3 @@
         __slots__ = []
 
         name = "unknown"
-
-        def __init__(self, data=None, **attrs):
-                generic.Action.__init__(self, data, **attrs)
-
--- a/src/modules/actions/user.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/actions/user.py	Wed Jul 27 13:42:21 2011 -0700
@@ -21,7 +21,7 @@
 #
 
 #
-# Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2008, 2011, Oracle and/or its affiliates. All rights reserved.
 #
 
 """module describing a user packaging object
@@ -51,7 +51,7 @@
 
         # if these values are different on disk than in action
         # prefer on-disk version
-        use_existing_attrs = [ "passwd", "lastchng", "min",
+        use_existing_attrs = [ "password", "lastchg", "min",
                                "max", "expire", "flag", 
                                "warn", "inactive"]
 
@@ -287,3 +287,22 @@
                 generic.py for a more detailed explanation."""
 
                 return [("user", "name", self.attrs["username"], None)]
+
+        def validate(self, fmri=None):
+                """Performs additional validation of action attributes that
+                for performance or other reasons cannot or should not be done
+                during Action object creation.  An ActionError exception (or
+                subclass of) will be raised if any attributes are not valid.
+                This is primarily intended for use during publication or during
+                error handling to provide additional diagonostics.
+
+                'fmri' is an optional package FMRI (object or string) indicating
+                what package contained this action.
+                """
+
+                generic.Action._validate(self, fmri=fmri,
+                    numeric_attrs=("uid", "lastchg", "min", "max", "warn",
+                    "inactive","expire", "flag"), single_attrs=("password",
+                    "uid", "group", "gcos-field", "home-dir", "login-shell",
+                    "ftpuser", "lastchg", "min", "max", "warn", "inactive",
+                    "expire", "flag"))
--- a/src/modules/client/imageplan.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/client/imageplan.py	Wed Jul 27 13:42:21 2011 -0700
@@ -2979,6 +2979,14 @@
                                 elif e.errno == errno.EROFS:
                                         raise api_errors.ReadOnlyFileSystemException(
                                             e.filename)
+                                elif e.errno == errno.ELOOP:
+                                        act = pkg.actions.unknown.UnknownAction()
+                                        raise api_errors.ActionExecutionError(
+                                            act, _("A link targeting itself or "
+                                            "part of a link loop was found at "
+                                            "'%s'; a file or directory was "
+                                            "expected.  Please remove the link "
+                                            "and try again.") % e.filename)
                                 raise
                 except pkg.actions.ActionError:
                         exc_type, exc_value, exc_tb = sys.exc_info()
--- a/src/modules/publish/dependencies.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/publish/dependencies.py	Wed Jul 27 13:42:21 2011 -0700
@@ -358,7 +358,7 @@
         # name itself to generate its dependencies.  Also, the name is entirely
         # a private construction which should not escape to the user.
         add_fmri_path_mapping(files.delivered, links,
-            fmri.PkgFmri("INTERNAL@0-0", build_release="0"), mfst)
+            fmri.PkgFmri("INTERNAL@0-0", "0"), mfst)
         for a in mfst.gen_actions_by_type("file"):
                 pvars = a.get_variant_template()
                 if not pvars:
@@ -681,7 +681,7 @@
         name = mfst.get("pkg.fmri", mfst.get("fmri", None))
         if name is not None:
                 try:
-                        pfmri = fmri.PkgFmri(name)
+                        pfmri = fmri.PkgFmri(name, "5.11")
                 except fmri.IllegalFmri:
                         pfmri = None
                 return name, pfmri
@@ -1157,8 +1157,7 @@
                         try:
                                 # XXX version requires build string; 5.11 is not
                                 # sane.
-                                cur_fmris.append(
-                                    fmri.PkgFmri(f, build_release="5.11"))
+                                cur_fmris.append(fmri.PkgFmri(f, "5.11"))
                         except fmri.IllegalFmri:
                                 bad_fmris.add(f)
                 skip = False
@@ -1194,8 +1193,7 @@
                 for comp_dep, comp_vars in res:
                         if comp_dep.attrs["type"] != "require":
                                 continue
-                        comp_fmri = fmri.PkgFmri(comp_dep.attrs["fmri"],
-                            build_release="5.11")
+                        comp_fmri = fmri.PkgFmri(comp_dep.attrs["fmri"], "5.11")
                         successor = False
                         # Check to see whether the package required by the
                         # require dependency is a successor to any of the
@@ -1338,7 +1336,7 @@
                 pkg_list = list(api_inst.get_pkg_list(
                     api.ImageInterface.LIST_INSTALLED))
                 for (pub, stem, ver), summ, cats, states in pkg_list:
-                        pfmri = fmri.PkgFmri("pkg:/%s@%s" % (stem, ver))
+                        pfmri = fmri.PkgFmri("pkg:/%s@%s" % (stem, ver), "5.11")
                         if pfmri.pkg_name in resolving_pkgs:
                                 continue
                         mfst = api_inst.get_manifest(pfmri, all_variants=True)
@@ -1358,7 +1356,7 @@
         # the system.
         if use_system:
                 for (pub, stem, ver), summ, cats, states in pkg_list:
-                        pfmri = fmri.PkgFmri("pkg:/%s@%s" % (stem, ver))
+                        pfmri = fmri.PkgFmri("pkg:/%s@%s" % (stem, ver), "5.11")
                         if pfmri.pkg_name in resolving_pkgs:
                                 continue
                         mfst = api_inst.get_manifest(pfmri, all_variants=True)
--- a/src/modules/publish/transaction.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/modules/publish/transaction.py	Wed Jul 27 13:42:21 2011 -0700
@@ -146,7 +146,7 @@
                 try:
                         # Perform additional publication-time validation of
                         # actions before further processing is done.
-                        action.validate()
+                        action.validate(fmri=self.pkg_name)
                 except actions.ActionError, e:
                         raise TransactionOperationError("add",
                             trans_id=self.trans_id, msg=str(e))
--- a/src/publish.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/publish.py	Wed Jul 27 13:42:21 2011 -0700
@@ -51,11 +51,8 @@
 def error(text, cmd=None):
         """Emit an error message prefixed by the command name """
 
-        if cmd:
-                text = "%s: %s" % (cmd, text)
-        else:
-                # If we get passed something like an Exception, we can convert
-                # it down to a string.
+        if not isinstance(text, basestring):
+                # Assume it's an object that can be stringified.
                 text = str(text)
 
         # If the message starts with whitespace, assume that it should come
@@ -63,9 +60,15 @@
         text_nows = text.lstrip()
         ws = text[:len(text) - len(text_nows)]
 
+        if cmd:
+                text_nows = "%s: %s" % (cmd, text_nows)
+                pkg_cmd = "pkgsend "
+        else:
+                pkg_cmd = "pkgsend: "
+
         # This has to be a constant value as we can't reliably get our actual
         # program name on all platforms.
-        emsg(ws + "pkgsend: " + text_nows)
+        emsg(ws + pkg_cmd + text_nows)
 
 def usage(usage_error=None, cmd=None, retcode=2):
         """Emit a usage message and optionally prefix it with a more specific
@@ -312,6 +315,10 @@
                 elif opt == "--no-catalog":
                         add_to_catalog = False
 
+        if not repo_uri:
+                usage(_("A destination package repository must be provided "
+                    "using -s."), cmd="publish")
+ 
         if not pargs:
                 filelist = [("<stdin>", sys.stdin)]
         else:
@@ -804,10 +811,10 @@
         except (pkg.actions.ActionError, trans.TransactionError,
             EnvironmentError, RuntimeError, pkg.fmri.FmriError,
             apx.ApiException), _e:
-                if isinstance(_e, IOError) and _e.errno != errno.EPIPE:
+                if not (isinstance(_e, IOError) and _e.errno == errno.EPIPE):
                         # Only print message if failure wasn't due to
                         # broken pipe (EPIPE) error.
-                         print >> sys.stderr, "pkgsend: %s" % _e
+                        print >> sys.stderr, "pkgsend: %s" % _e
                 __ret = 1
         except SystemExit, _e:
                 raise _e
--- a/src/tests/api/t_action.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/tests/api/t_action.py	Wed Jul 27 13:42:21 2011 -0700
@@ -84,7 +84,7 @@
             "driver alias=pci1234,56 alias=pci4567,89 class=scsi name=lsimega",
             "signature 12345 algorithm=foo",
         ]
-        
+
         def assertAttributeValue(self, action, attr, value):
                 attrs = action.attrs[attr]
 
@@ -384,6 +384,9 @@
                 # Missing key attribute 'fmri'.
                 self.assertInvalid("depend type=require")
 
+                # Mutiple fmri values only allowed for require-any deps.
+                self.assertInvalid("depend type=require fmri=foo fmri=bar")
+
                 # 'path' attribute specified multiple times
                 self.assertInvalid("file 1234 path=foo path=foo mode=777 owner=root group=root")
                 self.assertInvalid("link path=foo path=foo target=link")
@@ -427,18 +430,71 @@
                                 bad_act.validate()
                         except Exception, e:
                                 self.debug(str(e))
+                        else:
+                                self.debug("expected failure validating: %s" %
+                                    astr)
 
                         self.assertRaises(
                             action.InvalidActionAttributesError,
                             bad_act.validate)
 
-                # Invalid attribute for file and directory actions.
+                # Verify predicate and target attributes of FMRIs must be valid.
+                for nact in (
+                    # FMRI value is invalid.
+                    "depend type=require-any fmri=foo fmri=bar fmri=invalid@abc",
+                    # Predicate is missing for conditional dependency.
+                    "depend type=conditional fmri=foo",
+                    # Predicate value is invalid.
+                    "depend type=conditional predicate=-invalid fmri=foo",
+                    # Predicate isn't valid for dependency type.
+                    "depend type=require predicate=1invalid fmri=foo",
+                    # root-image attribute is only valid for origin dependencies.
+                    "depend type=require fmri=foo root-image=true",
+                    # Multiple values for predicate are not allowed.
+                    "depend type=conditional predicate=foo predicate=bar fmri=baz"):
+                        assert_invalid_attrs(nact)
+
+                # Verify multiple values for file attributes are rejected.
+                for attr in ("pkg.size", "pkg.csize", "chash", "preserve",
+                    "overlay", "elfhash", "original_name", "facet.doc",
+                    "variant.count", "owner", "group"):
+                        nact = "file path=/usr/bin/foo owner=root group=root " \
+                            "mode=0555 %(attr)s=1 %(attr)s=2 %(attr)s=3" % {
+                            "attr": attr }
+                        assert_invalid_attrs(nact)
+
+                # Verify invalid values are not allowed for mode attribute on
+                # file and dir actions. 
                 for act in (fact, dact):
                         for bad_mode in ("", 'mode=""', "mode=???", 
                             "mode=44755", "mode=44", "mode=999", "mode=0898"):
                                 nact = act.replace("mode=XXX", bad_mode)
                                 assert_invalid_attrs(nact)
 
+                # Verify multiple values aren't allowed for legacy action
+                # attributes.
+                for attr in ("category", "desc", "hotline", "name", "vendor",
+                    "version"):
+                        nact = "legacy pkg=SUNWcs %(attr)s=1 %(attr)s=2" % {
+                            "attr": attr }
+                        assert_invalid_attrs(nact)
+
+                # Verify multiple values aren't allowed for gid of group.
+                nact = "group groupname=staff gid=100 gid=101"
+                assert_invalid_attrs(nact)
+
+                # Verify only numeric value is allowed for gid of group.
+                nact = "group groupname=staff gid=abc"
+                assert_invalid_attrs(nact)
+
+                # Verify multiple values are not allowed for must-accept and
+                # must-display attributes of license actions.
+                for attr in ("must-accept", "must-display"):
+                        nact = "license license=copyright %(attr)s=true " \
+                            "%(attr)s=false" % { "attr": attr }
+                        assert_invalid_attrs(nact)
+
+                # Ensure link and hardlink attributes are validated properly.
                 for aname in ("link", "hardlink"):
                         # Action with mediator without mediator properties is
                         # invalid.
@@ -493,7 +549,7 @@
                                     % (aname, value)
                                 assert_invalid_attrs(nact)
 
-                        # Verify invalid mediator-implementations ar rejected.
+                        # Verify invalid mediator-implementations are rejected.
                         for value in ("1.a", "@", "@1", "[email protected]",
                             "vim@abc"):
                                 nact = "%s path=usr/bin/vi target=vim " \
@@ -501,6 +557,43 @@
                                     % (aname, value)
                                 assert_invalid_attrs(nact)
 
+                        # Verify multiple targets are not allowed.
+                        nact = "%s path=/usr/bin/foo target=bar target=baz" % \
+                            aname
+                        assert_invalid_attrs(nact)
+
+                # Verify multiple values are not allowed for set actions such as
+                # pkg.description, pkg.obsolete, pkg.renamed, and pkg.summary.
+                for attr in ("pkg.description", "pkg.obsolete", "pkg.renamed",
+                    "pkg.summary"):
+                        nact = "set name=%s value=true value=false" % attr
+                        assert_invalid_attrs(nact)
+
+                # Verify signature action attribute 'value' is required during
+                # publication.
+                nact = "signature 12345 algorithm=foo"
+                assert_invalid_attrs(nact)
+
+                # Verify multiple values aren't allowed for user attributes.
+                for attr in ("password", "group", "gcos-field", "home-dir",
+                    "login-shell", "ftpuser"):
+                        nact = "user username=user %(attr)s=ab %(attr)s=cd " % \
+                            { "attr": attr }
+                        assert_invalid_attrs(nact)
+
+                for attr in ("uid", "lastchg", "min","max", "warn", "inactive",
+                    "expire", "flag"):
+                        nact = "user username=user %(attr)s=1 %(attr)s=2" % {
+                            "attr": attr }
+                        assert_invalid_attrs(nact)
+
+                # Verify only numeric values are allowed for user attributes
+                # expecting a number.
+                for attr in ("uid", "lastchg", "min","max", "warn", "inactive",
+                    "expire", "flag"):
+                        nact = "user username=user %s=abc" % attr
+                        assert_invalid_attrs(nact)
+
 
 if __name__ == "__main__":
         unittest.main()
--- a/src/tests/cli/t_pkg_install.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/tests/cli/t_pkg_install.py	Wed Jul 27 13:42:21 2011 -0700
@@ -5827,12 +5827,22 @@
             add dir path=dir mode=755 owner=root group=bin
             close """
 
+        dir11 = """
+            open [email protected],5.11-0
+            add dir path=dir mode=750 owner=root group=bin
+            close """
+
         # Purposefully omits depend on [email protected].
         filesub10 = """
             open [email protected],5.11-0
             add file tmp/file path=dir/file mode=755 owner=root group=bin
             close """
 
+        filesub11 = """
+            open [email protected],5.11-0
+            add file tmp/file path=dir/file mode=444 owner=root group=bin
+            close """
+
         # Dependency providing file intentionally omitted.
         hardlink10 = """
             open [email protected],5.11-0
@@ -5855,8 +5865,8 @@
                 pkg5unittest.SingleDepotTestCase.setUp(self)
                 self.make_misc_files(self.misc_files)
 
-                plist = self.pkgsend_bulk(self.rurl, (self.dir10,
-                    self.filesub10, self.hardlink10))
+                plist = self.pkgsend_bulk(self.rurl, (self.dir10, self.dir11,
+                    self.filesub10, self.filesub11, self.hardlink10))
 
                 self.plist = {}
                 for p in plist:
@@ -5901,7 +5911,7 @@
                 new_src = os.path.join(os.path.dirname(src), "export", "dir")
                 shutil.move(src, os.path.dirname(new_src))
                 os.symlink(new_src, src)
-                self.pkg("install filesub", exit=1)
+                self.pkg("install [email protected]", exit=1)
 
         def test_02_hardlink(self):
                 """Verify that hardlink install fails as expected when
@@ -5984,6 +5994,53 @@
                 self.pkg("install [email protected] [email protected]")
                 self.pkg("uninstall foo unsupported")
 
+        def test_04_loop(self):
+                """Verify that if a directory or file is replaced with a link
+                that targets itself (resulting in ELOOP) pkg fails gracefully.
+                """
+
+                # Create an image and install a package delivering a file.
+                self.image_create(self.rurl)
+                self.pkg("install [email protected] [email protected]")
+
+                # Now replace the file with a link that points to itself.
+                def create_link_loop(fpath):
+                        if os.path.isfile(fpath):
+                                portable.remove(fpath)
+                        else:
+                                shutil.rmtree(fpath)
+                        cwd = os.getcwd()
+                        os.chdir(os.path.dirname(fpath))
+                        os.symlink(os.path.basename(fpath),
+                            os.path.basename(fpath))
+                        os.chdir(cwd)
+
+                fpath = self.get_img_file_path("dir/file")
+                create_link_loop(fpath)
+
+                # Verify that pkg verify gracefully fails if traversing a
+                # link targeting itself.
+                self.pkg("verify", exit=1)
+
+                # Verify that pkg succeeds if attempting to update a
+                # package containing a file replaced with a link loop.
+                self.pkg("update filesub")
+                self.pkg("verify")
+
+                # Now remove the package delivering the file and replace the
+                # directory with a link loop.
+                self.pkg("uninstall filesub")
+                fpath = self.get_img_file_path("dir")
+                create_link_loop(fpath)
+
+                # Verify that pkg verify gracefully fails if traversing a
+                # link targeting itself.
+                self.pkg("verify", exit=1)
+
+                # Verify that pkg gracefully fails if attempting to update
+                # a package containing a directory replace with a link loop.
+                self.pkg("update", exit=1)
+
 
 class TestConflictingActions(pkg5unittest.SingleDepotTestCase):
         """This set of tests verifies that packages which deliver conflicting
--- a/src/tests/cli/t_pkgsend.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/tests/cli/t_pkgsend.py	Wed Jul 27 13:42:21 2011 -0700
@@ -112,6 +112,8 @@
 
                 # A destination repository must be specified.
                 self.pkgsend("", "close", exit=2)
+                self.pkgsend("", "publish", exit=2)
+                self.assert_("pkgsend publish:" in self.errout)
 
         def test_1_pkgsend_abandon(self):
                 """Verify that an abandoned tranasaction is not published."""
--- a/src/tests/perf/actionbench.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/tests/perf/actionbench.py	Wed Jul 27 13:42:21 2011 -0700
@@ -21,8 +21,7 @@
 #
 
 #
-# Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
-# Use is subject to license terms.
+# Copyright (c) 2008, 2011, Oracle and/or its affiliates. All rights reserved.
 #
 
 #
@@ -45,8 +44,12 @@
         print "action creation"
         n = 20000
         for i in (1, 2, 3):
-                t = timeit.Timer(str1, setup1).timeit(n)
-                print "%20f  %8d actions/sec" % (t, n / t)
+                try:
+                        t = timeit.Timer(str1, setup1).timeit(n)
+                        print "%20f  %8d actions/sec" % (t, n / t)
+                except KeyboardInterrupt:
+                        import sys
+                        sys.exit(0)
 
         setup2 = """import pkg.actions as actions
 a1 = actions.fromstr("file 1234 group=bin mode=0755 owner=root path=usr/lib/libzonecfg.so.1")
@@ -59,8 +62,13 @@
         print "action comparison"
         for i in (1, 2, 3):
 
-                t = timeit.Timer(str2, setup2).timeit(n)
-                print "%20f  %8d action comparisons/sec" % (t, n / t)
+                try:
+                        t = timeit.Timer(str2, setup2).timeit(n)
+                        print "%20f  %8d action comparisons/sec" % (t, n / t)
+                except KeyboardInterrupt:
+                        import sys
+                        sys.exit(0)
+
 
         print "minimalist comparison"
 
@@ -84,8 +92,13 @@
         str3 = "cmp(a, b)"
         for i in (1, 2, 3):
 
-                t = timeit.Timer(str3, setup3).timeit(n)
-                print "%20f  %8d comparisons/sec" % (t, n / t)
+                try:
+                        t = timeit.Timer(str3, setup3).timeit(n)
+                        print "%20f  %8d comparisons/sec" % (t, n / t)
+                except KeyboardInterrupt:
+                        import sys
+                        sys.exit(0)
+
 
         setup4 = """import pkg.actions as actions
 a1 = actions.fromstr("file 1234 group=bin mode=0755 owner=root path=usr/lib/libzonecfg.so.1")
@@ -97,8 +110,12 @@
         print "action to string conversion"
         for i in (1, 2, 3):
 
-                t = timeit.Timer(str4, setup4).timeit(n)
-                print "%20f  %8d actions to string/sec" % (t, n / t)
+                try:
+                        t = timeit.Timer(str4, setup4).timeit(n)
+                        print "%20f  %8d actions to string/sec" % (t, n / t)
+                except KeyboardInterrupt:
+                        import sys
+                        sys.exit(0)
 
         # I took an existing manifest and randomized the lines.
         setup5 = """
@@ -174,17 +191,21 @@
 mf.set_content(m)
 """
 
-        print "manifest contents loading"
-        for i in (1, 2, 3):
+        try:
+                print "manifest contents loading"
+                for i in (1, 2, 3):
 
-                t = timeit.Timer(str5, setup5).timeit(n)
-                print "%20f %8d manifest contents loads/sec (%d actions/sec)" % \
-                    (t, n / t, (n * 60) / t)
+                        t = timeit.Timer(str5, setup5).timeit(n)
+                        print "%20f %8d manifest contents loads/sec (%d actions/sec)" % \
+                            (t, n / t, (n * 60) / t)
 
-        n = 1000000
-        str6 = "id(a1)"
-        print "id() speed"
-        for i in (1, 2, 3):
+                n = 1000000
+                str6 = "id(a1)"
+                print "id() speed"
+                for i in (1, 2, 3):
 
-                t = timeit.Timer(str6, setup4).timeit(n)
-                print "%20f %8d calls to id(action) /sec" % (t, n / t)
+                        t = timeit.Timer(str6, setup4).timeit(n)
+                        print "%20f %8d calls to id(action) /sec" % (t, n / t)
+        except KeyboardInterrupt:
+                import sys
+                sys.exit(0)
--- a/src/tests/pkg5unittest.py	Tue Jul 26 18:22:34 2011 -0700
+++ b/src/tests/pkg5unittest.py	Wed Jul 27 13:42:21 2011 -0700
@@ -1514,6 +1514,12 @@
                 # for backward compatibilty
                 return self.img_path()
 
+        def get_img_file_path(self, relpath):
+                """Given a path relative to root, return the absolute path of
+                the item in the image."""
+
+                return os.path.join(self.img_path(), relpath)
+
         def get_img_api_obj(self, cmd_path=None, ii=None):
                 progresstracker = pkg.client.progress.NullProgressTracker()
                 if not cmd_path: