25445472 manifest class caching can fail when multi-value variant attributes present
--- a/src/modules/_varcet.c Thu Jan 26 16:33:26 2017 +1100
+++ b/src/modules/_varcet.c Thu Jan 26 13:26:28 2017 -0800
@@ -20,7 +20,7 @@
*/
/*
- * Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2017, Oracle and/or its affiliates. All rights reserved.
*/
#include <Python.h>
@@ -136,6 +136,17 @@
prep_ret:
if (facet_ret != NULL) {
char *vs = PyBytes_AS_STRING(value);
+ if (vs == NULL) {
+ /*
+ * value is not a string; probably a list, so
+ * don't allow the action since older clients
+ * would fail anyway.
+ */
+ PyErr_Clear();
+ all_ret = Py_False;
+ break;
+ }
+
if (strcmp(vs, "all") == 0) {
/*
* If facet == 'all' and is False, then no more
@@ -202,14 +213,24 @@
while (PyDict_Next(act_attrs, &pos, &attr, &value)) {
char *as = PyBytes_AS_STRING(attr);
if (strncmp(as, "variant.", 8) == 0) {
+ char *av = PyBytes_AsString(value);
+ if (av == NULL) {
+ /*
+ * value is not a string; probably a list, so
+ * don't allow the action since older clients
+ * would fail anyway.
+ */
+ PyErr_Clear();
+ Py_DECREF(act_attrs);
+ Py_RETURN_FALSE;
+ }
+
PyObject *sysv = PyDict_GetItem(vars, attr);
- char *av = PyBytes_AsString(value);
- char *sysav = NULL;
-
if (sysv == NULL) {
/*
* If system variant value doesn't exist, then
- * allow the action if it is a variant that is "false".
+ * allow the action if it is a variant that is
+ * "false".
*/
if ((strncmp(as, "variant.", 8) == 0) &&
(strncmp(av, "false", 5) != 0)) {
@@ -219,7 +240,7 @@
continue;
}
- sysav = PyBytes_AsString(sysv);
+ char *sysav = PyBytes_AsString(sysv);
if (strcmp(av, sysav) != 0) {
/*
* If system variant value doesn't match action
--- a/src/modules/manifest.py Thu Jan 26 16:33:26 2017 +1100
+++ b/src/modules/manifest.py Thu Jan 26 13:26:28 2017 -0800
@@ -21,7 +21,7 @@
#
#
-# Copyright (c) 2007, 2016, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2007, 2017, Oracle and/or its affiliates. All rights reserved.
#
from __future__ import print_function
@@ -471,12 +471,14 @@
afacets = []
avariants = []
- for attr, val in six.iteritems(attrs):
+ for attr in attrs:
if attr[:8] == "variant.":
- variants[attr].add(val)
- avariants.append((attr, val))
+ for val in a.attrlist(attr):
+ variants[attr].add(val)
+ avariants.append((attr, val))
elif attr[:6] == "facet.":
- afacets.append((attr, val))
+ for val in a.attrlist(attr):
+ afacets.append((attr, val))
for name, val in afacets:
# Facet applicable to this particular variant
--- a/src/tests/api/t_manifest.py Thu Jan 26 16:33:26 2017 +1100
+++ b/src/tests/api/t_manifest.py Thu Jan 26 13:26:28 2017 -0800
@@ -21,7 +21,7 @@
# CDDL HEADER END
#
-# Copyright (c) 2008, 2016, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2008, 2017, Oracle and/or its affiliates. All rights reserved.
import unittest
import tempfile
@@ -39,6 +39,7 @@
import pkg.manifest as manifest
import pkg.misc as misc
import pkg.portable as portable
+import pkg.facet as facet
import pkg.variant as variant
# Set the path so that modules above can be found
@@ -423,6 +424,15 @@
dir owner=root path="opt/dir with " \\
"whitespaces " \\
"in value" group=bin mode=0755 variant.debug.osnet=true
+dir owner=root path=test group=bin mode=0755 facet.test=true
+dir owner=root path=vdo group=bin mode=0755 variant.debug.osnet=true \\
+ variant.debug.osnet=false
+dir owner=root path=vo group=bin mode=0755 variant.osnet=true \\
+ variant.osnet=false
+dir owner=root path=fdm group=bin mode=0755 facet.debug.multi=true \\
+ facet.debug.multi=false
+dir owner=root path=fm group=bin mode=0755 facet.multi=true \\
+ facet.multi=false
"""
def setUp(self):
@@ -468,12 +478,20 @@
"""Verifies that get_directories() works as expected."""
v = variant.Variants({ "variant.arch": "sparc" })
- excludes = [v.allow_action, lambda x, publisher: True]
+ vexcludes = [v.allow_action, lambda x, publisher: True]
+
+ f = facet.Facets({ "facet.test": False })
+ fexcludes = [lambda x, publisher: True, f.allow_action]
m1 = manifest.FactoredManifest("[email protected]", self.cache_dir,
pathname=self.foo_content_p5m)
all_expected = [
+ "fdm",
+ "fm",
+ "vdo",
+ "vo",
+ "test",
"opt/dir with spaces in value",
"opt",
"usr/bin",
@@ -482,17 +500,34 @@
]
var_expected = [
+ "fdm",
+ "fm",
"opt/dir with spaces in value",
- "opt"
+ "opt",
+ "test"
+ ]
+
+ facet_expected = [
+ "fm",
+ "vdo",
+ "vo",
+ "opt/dir with spaces in value",
+ "opt",
+ "usr/bin",
+ "opt/dir with whitespaces in value",
+ "usr"
]
def do_get_dirs():
actual = m1.get_directories([])
self.assertEqualDiff(sorted(all_expected), sorted(actual))
- actual = m1.get_directories(excludes)
+ actual = m1.get_directories(vexcludes)
self.assertEqualDiff(sorted(var_expected), sorted(actual))
+ actual = m1.get_directories(fexcludes)
+ self.assertEqualDiff(sorted(facet_expected), sorted(actual))
+
# Verify get_directories works for initial load.
do_get_dirs()