--- a/src/client.py Mon Oct 17 16:18:56 2016 -0700
+++ b/src/client.py Tue Oct 18 14:10:15 2016 -0700
@@ -1637,10 +1637,9 @@
# be printed on the same line as the spinner.
error("\n" + str(e), cmd=op)
return EXIT_OOPS
- if isinstance(e, api_errors.UnsupportedVariantGlobbing):
- # Prepend a newline because otherwise the exception will
- # be printed on the same line as the spinner.
- error("\n" + str(e), cmd=op)
+ if isinstance(e, (api_errors.UnsupportedVariantGlobbing,
+ api_errors.InvalidVarcetNames)):
+ error(str(e), cmd=op)
return EXIT_OOPS
# if we didn't deal with the exception above, pass it on.
--- a/src/modules/client/api.py Mon Oct 17 16:18:56 2016 -0700
+++ b/src/modules/client/api.py Tue Oct 18 14:10:15 2016 -0700
@@ -2165,15 +2165,24 @@
if not variants and facets is None:
raise ValueError("Nothing to do")
+ invalid_names = []
if variants:
op = API_OP_CHANGE_VARIANT
# Check whether '*' or '?' is in the input. Currently,
- # change-variant does not accept globbing.
+ # change-variant does not accept globbing. Also check
+ # for whitespaces.
for variant in variants:
if "*" in variant or "?" in variant:
raise apx.UnsupportedVariantGlobbing()
+ if not misc.valid_varcet_name(variant):
+ invalid_names.append(variant)
else:
op = API_OP_CHANGE_FACET
+ for facet in facets:
+ if not misc.valid_varcet_name(facet):
+ invalid_names.append(facet)
+ if invalid_names:
+ raise apx.InvalidVarcetNames(invalid_names)
return self.__plan_op(op, _act_timeout=act_timeout,
_backup_be=backup_be, _backup_be_name=backup_be_name,
--- a/src/modules/client/api_errors.py Mon Oct 17 16:18:56 2016 -0700
+++ b/src/modules/client/api_errors.py Tue Oct 18 14:10:15 2016 -0700
@@ -284,6 +284,19 @@
return output
+class InvalidVarcetNames(PlanPrepareException):
+ """Used to indicate that image plan evaluation or execution failed due
+ to illegal characters in variant/facet names."""
+
+ def __init__(self, invalid_names):
+ PlanPrepareException.__init__(self)
+ self.names = invalid_names
+
+ def __str__(self):
+ return _(", ".join(self.names) + " are not valid variant/facet "
+ "names; variant/facet names cannot contain whitespace.")
+
+
class ActuatorException(ApiException):
def __init__(self, e):
ApiException.__init__(self)
--- a/src/modules/client/client_api.py Mon Oct 17 16:18:56 2016 -0700
+++ b/src/modules/client/client_api.py Tue Oct 18 14:10:15 2016 -0700
@@ -1146,7 +1146,8 @@
api_errors.InvalidPlanError,
api_errors.ActionExecutionError,
api_errors.InvalidPackageErrors,
- api_errors.ImageBoundaryErrors)):
+ api_errors.ImageBoundaryErrors,
+ api_errors.InvalidVarcetNames)):
# Prepend a newline because otherwise the exception will
# be printed on the same line as the spinner.
_error_json("\n" + str(e), cmd=op, errors_json=errors_json)
--- a/src/modules/client/imageconfig.py Mon Oct 17 16:18:56 2016 -0700
+++ b/src/modules/client/imageconfig.py Tue Oct 18 14:10:15 2016 -0700
@@ -424,7 +424,11 @@
# how ssl cert and key paths are interpreted.)
idx = self.get_index()
self.variants.update(idx.get("variant", {}))
- # facets are encoded so they can contain '/' characters.
+ # Variants and facets are encoded so they can contain
+ # '/' characters.
+ for k, v in six.iteritems(idx.get("variant", {})):
+ # convert variant name from unicode to a string
+ self.variants[str(unquote(k))] = v
for k, v in six.iteritems(idx.get("facet", {})):
# convert facet name from unicode to a string
self.facets[str(unquote(k))] = v
@@ -566,7 +570,8 @@
except cfg.UnknownSectionError:
pass
for f in self.variants:
- self.set_property("variant", f, self.variants[f])
+ self.set_property("variant",
+ quote(f, ""), self.variants[f])
try:
self.remove_section("facet")
--- a/src/modules/config.py Mon Oct 17 16:18:56 2016 -0700
+++ b/src/modules/config.py Tue Oct 18 14:10:15 2016 -0700
@@ -217,7 +217,7 @@
class Property(object):
"""Base class for properties."""
- # Whitespace, '/', and '\' are never allowed.
+ # Whitespace (except single space), '/', and '\' are never allowed.
__name_re = re.compile(r"\A[^\t\n\r\f\v\\/]+\Z")
_value = None
@@ -882,10 +882,10 @@
provides an interface for adding and managing properties and sections
for the section."""
- # Whitespace, '/', and '\' are never allowed although consumers can
- # place additional restrictions by providing a name re. In addition,
- # the name "CONFIGURATION" is reserved for use by the configuration
- # serialization classes.
+ # Whitespace (except single space), '/', and '\' are never allowed
+ # although consumers can place additional restrictions by providing
+ # a name re. In addition, the name "CONFIGURATION" is reserved for
+ # use by the configuration serialization classes.
__name_re = re.compile(r"\A[^\t\n\r\f\v\\/]+\Z")
def __init__(self, name, properties=misc.EmptyI):
--- a/src/modules/misc.py Mon Oct 17 16:18:56 2016 -0700
+++ b/src/modules/misc.py Tue Oct 18 14:10:15 2016 -0700
@@ -3071,3 +3071,10 @@
" and try the requested operation again: {1}")\
.format(soft, e))
sys.exit(EXIT_OOPS)
+
+_varcetname_re = re.compile(r"\s")
+
+def valid_varcet_name(name):
+ """Check if the variant/facet name is valid. A valid variant/facet
+ name cannot contain whitespace"""
+ return _varcetname_re.search(name) is None
--- a/src/tests/cli/t_change_facet.py Mon Oct 17 16:18:56 2016 -0700
+++ b/src/tests/cli/t_change_facet.py Tue Oct 18 14:10:15 2016 -0700
@@ -643,6 +643,14 @@
("optional_fr.doc", False),
))
+ def test_07_invalid_facet_name(self):
+ """Test that invalid facet names are handled appropriately"""
+
+ self.image_create(self.rurl)
+ self.pkg("change-facet --no-refresh "
+ "facet.foo\ bar=false", exit=1)
+ self.assertTrue("facet.foo bar" in self.errout)
+
if __name__ == "__main__":
unittest.main()
--- a/src/tests/cli/t_change_variant.py Mon Oct 17 16:18:56 2016 -0700
+++ b/src/tests/cli/t_change_variant.py Tue Oct 18 14:10:15 2016 -0700
@@ -538,7 +538,7 @@
self.f_verify("usr/bin/foobar", "foo")
self.f_verify("usr/bin/foobar", "bar", negate=True)
- def test_cv_parsable(self):
+ def test_cv_13_parsable(self):
"""Test the parsable output of change-variant."""
self.image_create(self.rurl, variants={
@@ -554,7 +554,7 @@
self.assertEqualParsable(self.output, change_variants=[
["variant.arch", "i386"]])
- def test_invalid_variant(self):
+ def test_cv_14_invalid_variant(self):
"""Test that invalid input is handled appropriately"""
self.image_create(self.rurl, variants={
@@ -564,6 +564,19 @@
self.pkg("install pkg_shared")
self.pkg("change-variant variant.opensolaris.zone=bogus")
+ def test_cv_15_invalid_variant_name(self):
+ """Test that invalid variant names are handled appropriately"""
+
+ self.image_create(self.rurl)
+ # This should pass because there are no illegal characters
+ self.pkg("change-variant --no-refresh "
+ "variant.foobar=false", exit=0)
+ # Variant names contain space, should raise an exception
+ self.pkg("change-variant --no-refresh "
+ "variant.foo\ bar=false variant.bar\ foo=false", exit=1)
+ self.assertTrue("variant.foo bar" and "variant.bar foo"
+ in self.errout)
+
class TestPkgChangeVariantPerTestRepo(pkg5unittest.SingleDepotTestCase):
"""A separate test class is needed because these tests modify packages