Repository: oasis-open/cti-python-stix2
Pull request #602 (2024-09-15 21:36:08)

Fix DictionaryProperty valid_types bugs, and other misc property issues
Fix DictionaryProperty so that EnumProperty and ReferenceProperty both work as valid_types, and fix some bugs.
 - DictionaryProperty.__init__()'s valid_types argument now accepts either property classes or instances
 - DictionaryProperty now correctly propagates allow_custom down to its valid_types properties, and propagates their returned has_custom flag upward
 - DictionaryProperty's cleaned dictionaries actually contain the cleaned values now (before, the cleaned values were ignored)

Property class cleanup:
 - Property subclass .clean() methods now are correct overrides of the base class method, including parameter naming and default values
 - OpenVocabProperty now doesn't treat any value as custom, regardless of the allow_custom setting, which was our original intent.  A violation of strict mode now causes ValueError to be raised, not CustomContentError.
 - BooleanProperty.clean() now honors strict=True (it will only accept values True and False in that case).

Added various property tests of strictness, and additional DictionaryProperty tests which exercise custom value detection when used with ReferenceProperty.


Patch:
diff --git a/stix2/properties.py b/stix2/properties.py
index 59a5da1..424912e 100644
--- a/stix2/properties.py
+++ b/stix2/properties.py
@@ -172,7 +172,7 @@ class Property(object):
 
     """
 
-    def _default_clean(self, value, allow_custom=False):
+    def _default_clean(self, value, allow_custom=False, strict=False):
         if value != self._fixed_value:
             raise ValueError("must equal '{}'.".format(self._fixed_value))
         return value, False
@@ -226,7 +226,7 @@ class ListProperty(Property):
 
         super(ListProperty, self).__init__(**kwargs)
 
-    def clean(self, value, allow_custom, strict_flag=False):
+    def clean(self, value, allow_custom=False, strict=False):
         try:
             iter(value)
         except TypeError:
@@ -240,7 +240,7 @@ class ListProperty(Property):
         if isinstance(self.contained, Property):
             for item in value:
                 try:
-                    valid, temp_custom = self.contained.clean(item, allow_custom, strict=strict_flag)
+                    valid, temp_custom = self.contained.clean(item, allow_custom, strict=strict)
                 except TypeError:
                     valid, temp_custom = self.contained.clean(item, allow_custom)
                 result.append(valid)
@@ -281,11 +281,10 @@ class StringProperty(Property):
         super(StringProperty, self).__init__(**kwargs)
 
     def clean(self, value, allow_custom=False, strict=False):
-        if not isinstance(value, str):
-            if strict is True:
-                raise ValueError("Must be a string.")
+        if strict and not isinstance(value, str):
+            raise ValueError("Must be a string.")
 
-            value = str(value)
+        value = str(value)
         return value, False
 
 
@@ -320,7 +319,7 @@ class IntegerProperty(Property):
         super(IntegerProperty, self).__init__(**kwargs)
 
     def clean(self, value, allow_custom=False, strict=False):
-        if strict is True and not isinstance(value, int):
+        if strict and not isinstance(value, int):
             raise ValueError("must be an integer.")
 
         try:
@@ -347,7 +346,7 @@ class FloatProperty(Property):
         super(FloatProperty, self).__init__(**kwargs)
 
     def clean(self, value, allow_custom=False, strict=False):
-        if strict is True and not isinstance(value, float):
+        if strict and not isinstance(value, float):
             raise ValueError("must be a float.")
 
         try:
@@ -372,6 +371,9 @@ class BooleanProperty(Property):
 
     def clean(self, value, allow_custom=False, strict=False):
 
+        if strict and not isinstance(value, bool):
+            raise ValueError("must be a boolean value.")
+
         if isinstance(value, str):
             value = value.lower()
 
@@ -403,74 +405,134 @@ class DictionaryProperty(Property):
 
     def __init__(self, valid_types=None, spec_version=DEFAULT_VERSION, **kwargs):
         self.spec_version = spec_version
+        self.valid_types = self._normalize_valid_types(valid_types or [])
 
-        simple_types = [
-            BinaryProperty, BooleanProperty, FloatProperty, HexProperty, IntegerProperty, StringProperty,
-            TimestampProperty, ReferenceProperty, EnumProperty,
-        ]
-        if not valid_types:
-            valid_types = [Property]
-        else:
-            if not isinstance(valid_types, list):
-                valid_types = [valid_types]
-            for type_ in valid_types:
-                if isinstance(type_, ListProperty):
-                    found = False
-                    for simple_type in simple_types:
-                        if isinstance(type_.contained, simple_type):
-                            found = True
-                    if not found:
-                        raise ValueError("Dictionary Property does not support lists of type: ", type_.contained, type(type_.contained))
-                elif type_ not in simple_types:
-                    raise ValueError("Dictionary Property does not support this value's type: ", type_)
+        super(DictionaryProperty, self).__init__(**kwargs)
 
-        self.valid_types = valid_types
+    def _normalize_valid_types(self, valid_types):
+        """
+        Normalize valid_types to a list of property instances.  Also ensure any
+        property types given are supported for type enforcement.
 
-        super(DictionaryProperty, self).__init__(**kwargs)
+        :param valid_types: A single or iterable of Property instances or
+            subclasses
+        :return: A list of Property instances, or None if none were given
+        """
+        simple_types = (
+            BinaryProperty, BooleanProperty, FloatProperty, HexProperty,
+            IntegerProperty, StringProperty, TimestampProperty,
+            ReferenceProperty, EnumProperty,
+        )
 
-    def clean(self, value, allow_custom=False):
+        # Normalize single prop instances/classes to lists
+        try:
+            iter(valid_types)
+        except TypeError:
+            valid_types = [valid_types]
+
+        prop_instances = []
+        for valid_type in valid_types:
+            if inspect.isclass(valid_type):
+                # Note: this will fail as of this writing with EnumProperty
+                # ReferenceProperty, ListProperty.  Callers must instantiate
+                # those with suitable settings themselves.
+                prop_instance = valid_type()
+
+            else:
+                prop_instance = valid_type
+
+            # ListProperty's element type must be one of the supported
+            # simple types.
+            if isinstance(prop_instance, ListProperty):
+                if not isinstance(prop_instance.contained, simple_types):
+                    raise ValueError(
+                        "DictionaryProperty does not support lists of type: "
+                        + type(prop_instance.contained).__name__
+                    )
+
+            elif not isinstance(prop_instance, simple_types):
+                raise ValueError(
+                    "DictionaryProperty does not support value type: "
+                    + type(prop_instance).__name__
+                )
+
+            prop_instances.append(prop_instance)
+
+        return prop_instances or None
+
+    def _check_dict_key(self, k):
+        if self.spec_version == '2.0':
+            if len(k) < 3:
+                raise DictionaryKeyError(k, "shorter than 3 characters")
+            elif len(k) > 256:
+                raise DictionaryKeyError(k, "longer than 256 characters")
+        elif self.spec_version == '2.1':
+            if len(k) > 250:
+                raise DictionaryKeyError(k, "longer than 250 characters")
+        if not re.match(r"^[a-zA-Z0-9_-]+$", k):
+            msg = (
+                "contains characters other than lowercase a-z, "
+                "uppercase A-Z, numerals 0-9, hyphen (-), or "
+                "underscore (_)"
+            )
+            raise DictionaryKeyError(k, msg)
+
+    def clean(self, value, allow_custom=False, strict=False):
         try:
             dictified = _get_dict(value)
         except ValueError:
             raise ValueError("The dictionary property must contain a dictionary")
 
-        for k in dictified.keys():
-            if self.spec_version == '2.0':
-                if len(k) < 3:
-                    raise DictionaryKeyError(k, "shorter than 3 characters")
-                elif len(k) > 256:
-                    raise DictionaryKeyError(k, "longer than 256 characters")
-            elif self.spec_version == '2.1':
-                if len(k) > 250:
-                    raise DictionaryKeyError(k, "longer than 250 characters")
-            if not re.match(r"^[a-zA-Z0-9_-]+$", k):
-                msg = (
-                    "contains characters other than lowercase a-z, "
-                    "uppercase A-Z, numerals 0-9, hyphen (-), or "
-                    "underscore (_)"
-                )
-                raise DictionaryKeyError(k, msg)
+        has_custom = False
+        for k, v in dictified.items():
 
-            clean = False
-            for type_ in self.valid_types:
-                if isinstance(type_, ListProperty):
-                    type_.clean(value=dictified[k], allow_custom=False, strict_flag=True)
-                    clean = True
-                else:
-                    type_instance = type_()
+            self._check_dict_key(k)
+
+            if self.valid_types:
+                for type_ in self.valid_types:
                     try:
-                        type_instance.clean(value=dictified[k], allow_custom=False, strict=True)
-                        clean = True
+                        # ReferenceProperty at least, does check for
+                        # customizations, so we must propagate that
+                        dictified[k], temp_custom = type_.clean(
+                            value=v,
+                            allow_custom=allow_custom,
+                            # Ignore the passed-in value and fix this to True;
+                            # we need strict cleaning to disambiguate value
+                            # types here.
+                            strict=True
+                        )
+                    except CustomContentError:
+                        # Need to propagate these, not treat as a type error
+                        raise
+                    except Exception as e:
+                        # clean failed; value must not conform to type_
+                        # Should be a narrower exception type here, but I don't
+                        # know if it's safe to assume any particular exception
+                        # types...
+                        pass
+                    else:
+                        # clean succeeded; should check the has_custom flag
+                        # just in case.  But if allow_custom is False, I expect
+                        # one of the valid_types property instances would have
+                        # already raised an exception.
+                        has_custom = has_custom or temp_custom
+                        if has_custom and not allow_custom:
+                            raise CustomContentError(f'Custom content detected in key "{k}"')
+
                         break
-                    except ValueError:
-                        continue
-            if not clean:
-                raise ValueError("Dictionary Property does not support this value's type: ", type(dictified[k]))
+
+                else:
+                    # clean failed for all properties!
+                    raise ValueError(
+                        f"Invalid value: {v!r}"
+                    )
+
+            # else: no valid types given, so we skip the validity check
 
         if len(dictified) < 1:
             raise ValueError("must not be empty.")
 
-        return dictified, False
+        return dictified, has_custom
 
 
 class HashesProperty(DictionaryProperty):
@@ -488,7 +550,7 @@ class HashesProperty(DictionaryProperty):
             if alg:
                 self.__alg_to_spec_name[alg] = spec_hash_name
 
-    def clean(self, value, allow_custom, strict=False):
+    def clean(self, value, allow_custom=False, strict=False):
         # ignore the has_custom return value here; there is no customization
         # of DictionaryProperties.
         clean_dict, _ = super().clean(value, allow_custom)
@@ -597,7 +659,7 @@ class ReferenceProperty(Property):
 
         super(ReferenceProperty, self).__init__(**kwargs)
 
-    def clean(self, value, allow_custom):
+    def clean(self, value, allow_custom=False, strict=False):
         if isinstance(value, _STIXBase):
             value = value.id
         value = str(value)
@@ -675,7 +737,7 @@ SELECTOR_REGEX = re.compile(r"^([a-z0-9_-]{3,250}(\.(\[\d+\]|[a-z0-9_-]{1,250}))
 
 class SelectorProperty(Property):
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
         if not SELECTOR_REGEX.match(value):
             raise ValueError("must adhere to selector syntax.")
         return value, False
@@ -696,7 +758,7 @@ class EmbeddedObjectProperty(Property):
         self.type = type
         super(EmbeddedObjectProperty, self).__init__(**kwargs)
 
-    def clean(self, value, allow_custom):
+    def clean(self, value, allow_custom=False, strict=False):
         if isinstance(value, dict):
             value = self.type(allow_custom=allow_custom, **value)
         elif not isinstance(value, self.type):
@@ -724,7 +786,7 @@ class EnumProperty(StringProperty):
         self.allowed = allowed
         super(EnumProperty, self).__init__(**kwargs)
 
-    def clean(self, value, allow_custom, strict=False):
+    def clean(self, value, allow_custom=False, strict=False):
 
         cleaned_value, _ = super(EnumProperty, self).clean(value, allow_custom, strict)
 
@@ -746,26 +808,20 @@ class OpenVocabProperty(StringProperty):
             allowed = [allowed]
         self.allowed = allowed
 
-    def clean(self, value, allow_custom, strict=False):
+    def clean(self, value, allow_custom=False, strict=False):
         cleaned_value, _ = super(OpenVocabProperty, self).clean(
             value, allow_custom, strict,
         )
 
-        # Disabled: it was decided that enforcing this is too strict (might
-        # break too much user code).  Revisit when we have the capability for
-        # more granular config settings when creating objects.
-        #
-        if strict is True:
-            has_custom = cleaned_value not in self.allowed
-
-            if not allow_custom and has_custom:
-                raise CustomContentError(
-                    "custom value in open vocab: '{}'".format(cleaned_value),
-                )
+        # Customization enforcement is disabled: it was decided that enforcing
+        # it is too strict (might break too much user code).  On the other
+        # hand, we need to lock it down in strict mode.  If we are locking it
+        # down in strict mode, we always throw an exception if a value isn't
+        # in the vocab list, and never report anything as "custom".
+        if strict and cleaned_value not in self.allowed:
+            raise ValueError("not in vocab: " + cleaned_value)
 
-        has_custom = False
-
-        return cleaned_value, has_custom
+        return cleaned_value, False
 
 
 class PatternProperty(StringProperty):
@@ -780,7 +836,7 @@ class ObservableProperty(Property):
         self.spec_version = spec_version
         super(ObservableProperty, self).__init__(*args, **kwargs)
 
-    def clean(self, value, allow_custom):
+    def clean(self, value, allow_custom=False, strict=False):
         try:
             dictified = _get_dict(value)
             # get deep copy since we are going modify the dict and might
@@ -828,7 +884,7 @@ class ExtensionsProperty(DictionaryProperty):
     def __init__(self, spec_version=DEFAULT_VERSION, required=False):
         super(ExtensionsProperty, self).__init__(spec_version=spec_version, required=required)
 
-    def clean(self, value, allow_custom):
+    def clean(self, value, allow_custom=False, strict=False):
         try:
             dictified = _get_dict(value)
             # get deep copy since we are going modify the dict and might
@@ -894,7 +950,7 @@ class STIXObjectProperty(Property):
         self.spec_version = spec_version
         super(STIXObjectProperty, self).__init__(*args, **kwargs)
 
-    def clean(self, value, allow_custom):
+    def clean(self, value, allow_custom=False, strict=False):
         # Any STIX Object (SDO, SRO, or Marking Definition) can be added to
         # a bundle with no further checks.
         stix2_classes = {'_DomainObject', '_RelationshipObject', 'MarkingDefinition'}


Test patch:
diff --git a/stix2/test/test_properties.py b/stix2/test/test_properties.py
index 5116a68..4637e39 100644
--- a/stix2/test/test_properties.py
+++ b/stix2/test/test_properties.py
@@ -181,6 +181,12 @@ def test_string_property():
     assert prop.clean(1)
     assert prop.clean([1, 2, 3])
 
+    with pytest.raises(ValueError):
+        prop.clean(1, strict=True)
+
+    result = prop.clean("foo", strict=True)
+    assert result == ("foo", False)
+
 
 def test_type_property():
     prop = TypeProperty('my-type')
@@ -244,6 +250,15 @@ def test_integer_property_invalid(value):
         int_prop.clean(value)
 
 
+def test_integer_property_strict():
+    int_prop = IntegerProperty()
+    with pytest.raises(ValueError):
+        int_prop.clean("123", strict=True)
+
+    result = int_prop.clean(123, strict=True)
+    assert result == (123, False)
+
+
 @pytest.mark.parametrize(
     "value", [
         2,
@@ -253,8 +268,8 @@ def test_integer_property_invalid(value):
     ],
 )
 def test_float_property_valid(value):
-    int_prop = FloatProperty()
-    assert int_prop.clean(value) is not None
+    float_prop = FloatProperty()
+    assert float_prop.clean(value) is not None
 
 
 @pytest.mark.parametrize(
@@ -264,9 +279,18 @@ def test_float_property_valid(value):
     ],
 )
 def test_float_property_invalid(value):
-    int_prop = FloatProperty()
+    float_prop = FloatProperty()
     with pytest.raises(ValueError):
-        int_prop.clean(value)
+        float_prop.clean(value)
+
+
+def test_float_property_strict():
+    float_prop = FloatProperty()
+    with pytest.raises(ValueError):
+        float_prop.clean("1.323", strict=True)
+
+    result = float_prop.clean(1.323, strict=True)
+    assert result == (1.323, False)
 
 
 @pytest.mark.parametrize(
@@ -308,6 +332,15 @@ def test_boolean_property_invalid(value):
         bool_prop.clean(value)
 
 
+def test_boolean_property_strict():
+    bool_prop = BooleanProperty()
+    with pytest.raises(ValueError):
+        bool_prop.clean("true", strict=True)
+
+    result = bool_prop.clean(True, strict=True)
+    assert result == (True, False)
+
+
 @pytest.mark.parametrize(
     "value", [
         '2017-01-01T12:34:56Z',
@@ -368,6 +401,16 @@ def test_enum_property_invalid():
         enum_prop.clean('z', True)
 
 
+def test_enum_property_strict():
+    enum_prop = EnumProperty(['1', '2', '3'])
+    with pytest.raises(ValueError):
+        enum_prop.clean(1, strict=True)
+
+    result = enum_prop.clean(1, strict=False)
+    assert result == ("1", False)
+
+
+
 @pytest.mark.xfail(
     reason="Temporarily disabled custom open vocab enforcement",
     strict=True,
@@ -391,6 +434,27 @@ def test_openvocab_property(vocab):
     assert ov_prop.clean("d", True) == ("d", True)
 
 
+def test_openvocab_property_strict():
+    ov_prop = OpenVocabProperty(["1", "2", "3"])
+    with pytest.raises(ValueError):
+        ov_prop.clean(1, allow_custom=False, strict=True)
+
+    with pytest.raises(ValueError):
+        ov_prop.clean(1, allow_custom=True, strict=True)
+
+    result = ov_prop.clean("1", allow_custom=False, strict=True)
+    assert result == ("1", False)
+
+    result = ov_prop.clean(1, allow_custom=True, strict=False)
+    assert result == ("1", False)
+
+    result = ov_prop.clean(1, allow_custom=False, strict=False)
+    assert result == ("1", False)
+
+    result = ov_prop.clean("foo", allow_custom=False, strict=False)
+    assert result == ("foo", False)
+
+
 @pytest.mark.parametrize(
     "value", [
         {"sha256": "6db12788c37247f2316052e142f42f4b259d6561751e5f401a1ae2a6df9c674b"},
diff --git a/stix2/test/v21/test_properties.py b/stix2/test/v21/test_properties.py
index 0eff3c2..2cd42a1 100644
--- a/stix2/test/v21/test_properties.py
+++ b/stix2/test/v21/test_properties.py
@@ -6,71 +6,16 @@ from stix2.exceptions import (
     ExtraPropertiesError, ParseError,
 )
 from stix2.properties import (
-    DictionaryProperty, EmbeddedObjectProperty, ExtensionsProperty,
-    HashesProperty, IDProperty, IntegerProperty, ListProperty,
-    ObservableProperty, ReferenceProperty, STIXObjectProperty, StringProperty,
+    DictionaryProperty, EmbeddedObjectProperty, EnumProperty,
+    ExtensionsProperty, HashesProperty, IDProperty, IntegerProperty,
+    ListProperty, ObservableProperty, ReferenceProperty, STIXObjectProperty,
+    StringProperty,
 )
 from stix2.v21.common import MarkingProperty
 
 from . import constants
 
 
-def test_dictionary_property():
-    p = DictionaryProperty()
-
-    assert p.clean({'spec_version': '2.1'})
-    with pytest.raises(ValueError):
-        p.clean({}, False)
-
-
-def test_dictionary_property_values_str():
-    p = DictionaryProperty(valid_types=[StringProperty], spec_version='2.1')
-    result = p.clean({'x': '123'}, False)
-    assert result == ({'x': '123'}, False)
-
-    q = DictionaryProperty(valid_types=[StringProperty], spec_version='2.1')
-    with pytest.raises(ValueError):
-        assert q.clean({'x': [123]}, False)
-
-
-def test_dictionary_property_values_int():
-    p = DictionaryProperty(valid_types=[IntegerProperty], spec_version='2.1')
-    result = p.clean({'x': 123}, False)
-    assert result == ({'x': 123}, False)
-
-    q = DictionaryProperty(valid_types=[IntegerProperty], spec_version='2.1')
-    with pytest.raises(ValueError):
-        assert q.clean({'x': [123]}, False)
-
-
-def test_dictionary_property_values_stringlist():
-    p = DictionaryProperty(valid_types=[ListProperty(StringProperty)], spec_version='2.1')
-    result = p.clean({'x': ['abc', 'def']}, False)
-    assert result == ({'x': ['abc', 'def']}, False)
-
-    q = DictionaryProperty(valid_types=[ListProperty(StringProperty)], spec_version='2.1')
-    with pytest.raises(ValueError):
-        assert q.clean({'x': [123]})
-
-    r = DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')
-    with pytest.raises(ValueError):
-        assert r.clean({'x': [123, 456]})
-
-
-def test_dictionary_property_values_list():
-    p = DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')
-    result = p.clean({'x': 123}, False)
-    assert result == ({'x': 123}, False)
-
-    q = DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')
-    result = q.clean({'x': '123'}, False)
-    assert result == ({'x': '123'}, False)
-
-    r = DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')
-    with pytest.raises(ValueError):
-        assert r.clean({'x': ['abc', 'def']}, False)
-
-
 ID_PROP = IDProperty('my-type', spec_version="2.1")
 MY_ID = 'my-type--232c9d3f-49fc-4440-bb01-607f638778e7'
 
@@ -436,6 +381,116 @@ def test_property_list_of_dictionary():
     assert test_obj.property1[0]['foo'] == 'bar'
 
 
+def test_dictionary_property():
+    p = DictionaryProperty()
+
+    result = p.clean({'spec_version': '2.1'})
+    assert result == ({'spec_version': '2.1'}, False)
+
+    with pytest.raises(ValueError):
+        p.clean({}, False)
+
+
+def test_dictionary_property_values_str():
+    p = DictionaryProperty(valid_types=[StringProperty], spec_version='2.1')
+    result = p.clean({'x': '123'}, False)
+    assert result == ({'x': '123'}, False)
+
+    q = DictionaryProperty(valid_types=[StringProperty], spec_version='2.1')
+    with pytest.raises(ValueError):
+        assert q.clean({'x': [123]}, False)
+
+
+def test_dictionary_property_values_str_single():
+    # singles should be treated as length-one lists
+    p = DictionaryProperty(valid_types=StringProperty, spec_version='2.1')
+    result = p.clean({'x': '123'}, False)
+    assert result == ({'x': '123'}, False)
+
+    with pytest.raises(ValueError):
+        assert p.clean({'x': [123]}, False)
+
+
+def test_dictionary_property_values_int():
+    p = DictionaryProperty(valid_types=[IntegerProperty], spec_version='2.1')
+    result = p.clean({'x': 123}, False)
+    assert result == ({'x': 123}, False)
+
+    q = DictionaryProperty(valid_types=[IntegerProperty], spec_version='2.1')
+    with pytest.raises(ValueError):
+        assert q.clean({'x': [123]}, False)
+
+
+def test_dictionary_property_values_stringlist():
+    p = DictionaryProperty(valid_types=[ListProperty(StringProperty)], spec_version='2.1')
+    result = p.clean({'x': ['abc', 'def']}, False)
+    assert result == ({'x': ['abc', 'def']}, False)
+
+    q = DictionaryProperty(valid_types=[ListProperty(StringProperty)], spec_version='2.1')
+    with pytest.raises(ValueError):
+        assert q.clean({'x': [123]})
+
+    r = DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')
+    with pytest.raises(ValueError):
+        assert r.clean({'x': [123, 456]})
+
+
+def test_dictionary_property_values_list():
+    p = DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')
+    result = p.clean({'x': 123}, False)
+    assert result == ({'x': 123}, False)
+
+    q = DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')
+    result = q.clean({'x': '123'}, False)
+    assert result == ({'x': '123'}, False)
+
+    r = DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')
+    with pytest.raises(ValueError):
+        assert r.clean({'x': ['abc', 'def']}, False)
+
+
+def test_dictionary_property_ref_custom():
+    p = DictionaryProperty(
+        valid_types=ReferenceProperty(valid_types="SDO"), spec_version="2.1"
+    )
+
+    result = p.clean({"key": "identity--a2ac7670-f88f-424a-b3be-28f612f943f9"}, allow_custom=False)
+    assert result == ({"key": "identity--a2ac7670-f88f-424a-b3be-28f612f943f9"}, False)
+
+    with pytest.raises(ValueError):
+        p.clean({"key": "software--a2ac7670-f88f-424a-b3be-28f612f943f9"}, allow_custom=False)
+
+    with pytest.raises(ValueError):
+        p.clean({"key": "software--a2ac7670-f88f-424a-b3be-28f612f943f9"}, allow_custom=True)
+
+    pfoo = DictionaryProperty(
+        valid_types=ReferenceProperty(valid_types=["SDO", "foo"]), spec_version="2.1"
+    )
+
+    with pytest.raises(CustomContentError):
+        pfoo.clean({"key": "foo--a2ac7670-f88f-424a-b3be-28f612f943f9"}, allow_custom=False)
+
+    result = pfoo.clean({"key": "foo--a2ac7670-f88f-424a-b3be-28f612f943f9"}, allow_custom=True)
+    assert result == ({"key": "foo--a2ac7670-f88f-424a-b3be-28f612f943f9"}, True)
+
+
+def test_dictionary_property_values_strict_clean():
+    prop = DictionaryProperty(
+        valid_types=[EnumProperty(["value1", "value2"]), IntegerProperty]
+    )
+
+    result = prop.clean({"key": "value1"}, allow_custom=False)
+    assert result == ({"key": "value1"}, False)
+
+    result = prop.clean({"key": 123}, allow_custom=False)
+    assert result == ({"key": 123}, False)
+
+    with pytest.raises(ValueError):
+        # IntegerProperty normally cleans "123" to 123, but can't when used
+        # in a DictionaryProperty.
+        prop.clean({"key": "123"}, allow_custom=False)
+
+
 @pytest.mark.parametrize(
     "key", [
         "a",