From f0f6d8fdd1925944d2ef0b6c6cda8f52a7b67b61 Mon Sep 17 00:00:00 2001
From: Michael Chisholm <chisholm@mitre.org>
Date: Sun, 15 Sep 2024 17:25:26 -0400
Subject: [PATCH] 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.
---
 stix2/properties.py               | 224 +++++++++++++++++++-----------
 stix2/test/test_properties.py     |  72 +++++++++-
 stix2/test/v21/test_properties.py | 173 +++++++++++++++--------
 3 files changed, 322 insertions(+), 147 deletions(-)

diff --git a/stix2/properties.py b/stix2/properties.py
index 59a5da1a..424912ec 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 @@ def __init__(self, contained, **kwargs):
 
         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 @@ def clean(self, value, allow_custom, strict_flag=False):
         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 @@ def __init__(self, **kwargs):
         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 @@ def __init__(self, min=None, max=None, **kwargs):
         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 @@ def __init__(self, min=None, max=None, **kwargs):
         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 @@ def __init__(self, spec_hash_names, spec_version=DEFAULT_VERSION, **kwargs):
             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 @@ def __init__(self, valid_types=None, invalid_types=None, spec_version=DEFAULT_VE
 
         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 @@ def clean(self, value, allow_custom):
 
 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 @@ def __init__(self, type, **kwargs):
         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 @@ def __init__(self, allowed, **kwargs):
         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 @@ def __init__(self, allowed, **kwargs):
             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 @@ def __init__(self, spec_version=DEFAULT_VERSION, *args, **kwargs):
         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 @@ def __init__(self, spec_version=DEFAULT_VERSION, *args, **kwargs):
         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'}
diff --git a/stix2/test/test_properties.py b/stix2/test/test_properties.py
index 5116a685..4637e39f 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 0eff3c29..2cd42a12 100644
--- a/stix2/test/v21/test_properties.py
+++ b/stix2/test/v21/test_properties.py
@@ -6,71 +6,16 @@
     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 @@ class NewObj():
     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",