Repository: oasis-open/cti-python-stix2
Pull request #589 (2024-03-20 19:29:19)

Dictionary Prop Adjustments
Address #588 


- don't allow 'string_list' as one of the values when a list is given for valid_types
- in clean, check that the value of the dictionary items is the correct type as noted in specifics.
- some Tests to check for both the correct use of the DictionaryProperty definitions and instantiations


Patch:
diff --git a/stix2/properties.py b/stix2/properties.py
index 00f2462..ff500f0 100644
--- a/stix2/properties.py
+++ b/stix2/properties.py
@@ -133,7 +133,7 @@ class Property(object):
 
     Subclasses can also define the following functions:
 
-    - ``def clean(self, value, allow_custom) -> (any, has_custom):``
+    - ``def clean(self, value, allow_custom, strict) -> (any, has_custom):``
         - Return a value that is valid for this property, and enforce and
           detect value customization.  If ``value`` is not valid for this
           property, you may attempt to transform it first.  If ``value`` is not
@@ -148,7 +148,9 @@ class Property(object):
           mean there actually are any).  The method must return an appropriate
           value for has_custom.  Customization may not be applicable/possible
           for a property.  In that case, allow_custom can be ignored, and
-          has_custom must be returned as False.
+          has_custom must be returned as False. strict is a True/False flag
+          that is used in the dictionary property. if strict is True,
+          properties like StringProperty will not be lenient in their clean method.
 
     - ``def default(self):``
         - provide a default value for this property.
@@ -191,7 +193,7 @@ class Property(object):
         if default:
             self.default = default
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
         return value, False
 
 
@@ -224,7 +226,7 @@ class ListProperty(Property):
 
         super(ListProperty, self).__init__(**kwargs)
 
-    def clean(self, value, allow_custom):
+    def clean(self, value, allow_custom, strict_flag=False):
         try:
             iter(value)
         except TypeError:
@@ -237,7 +239,10 @@ class ListProperty(Property):
         has_custom = False
         if isinstance(self.contained, Property):
             for item in value:
-                valid, temp_custom = self.contained.clean(item, allow_custom)
+                try:
+                    valid, temp_custom = self.contained.clean(item, allow_custom, strict=strict_flag)
+                except TypeError:
+                    valid, temp_custom = self.contained.clean(item, allow_custom)
                 result.append(valid)
                 has_custom = has_custom or temp_custom
 
@@ -275,8 +280,11 @@ class StringProperty(Property):
     def __init__(self, **kwargs):
         super(StringProperty, self).__init__(**kwargs)
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
         if not isinstance(value, str):
+            if strict is True:
+                raise ValueError("Must be a string.")
+
             value = str(value)
         return value, False
 
@@ -296,7 +304,7 @@ class IDProperty(Property):
         self.spec_version = spec_version
         super(IDProperty, self).__init__()
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
         _validate_id(value, self.spec_version, self.required_prefix)
         return value, False
 
@@ -311,7 +319,10 @@ class IntegerProperty(Property):
         self.max = max
         super(IntegerProperty, self).__init__(**kwargs)
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
+        if strict is True and not isinstance(value, int):
+            raise ValueError("must be an integer.")
+
         try:
             value = int(value)
         except Exception:
@@ -335,7 +346,10 @@ class FloatProperty(Property):
         self.max = max
         super(FloatProperty, self).__init__(**kwargs)
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
+        if strict is True and not isinstance(value, float):
+            raise ValueError("must be a float.")
+
         try:
             value = float(value)
         except Exception:
@@ -356,7 +370,7 @@ class BooleanProperty(Property):
     _trues = ['true', 't', '1', 1, True]
     _falses = ['false', 'f', '0', 0, False]
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
 
         if isinstance(value, str):
             value = value.lower()
@@ -379,7 +393,7 @@ class TimestampProperty(Property):
 
         super(TimestampProperty, self).__init__(**kwargs)
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
         return parse_into_datetime(
             value, self.precision, self.precision_constraint,
         ), False
@@ -390,16 +404,24 @@ class DictionaryProperty(Property):
     def __init__(self, valid_types=None, spec_version=DEFAULT_VERSION, **kwargs):
         self.spec_version = spec_version
 
+        simple_types = [BinaryProperty, BooleanProperty, FloatProperty, HexProperty, IntegerProperty, StringProperty, TimestampProperty, ReferenceProperty]
         if not valid_types:
-            valid_types = ["string"]
-        elif not isinstance(valid_types, list):
-            valid_types = [valid_types]
-
-        for type_ in valid_types:
-            if type_ not in ("string", "integer", "string_list"):
-                raise ValueError("The value of a dictionary key cannot be ", type_)
+            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_)
 
-        self.specifics = valid_types
+        self.valid_types = valid_types
 
         super(DictionaryProperty, self).__init__(**kwargs)
 
@@ -408,6 +430,7 @@ class DictionaryProperty(Property):
             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:
@@ -425,6 +448,22 @@ class DictionaryProperty(Property):
                 )
                 raise DictionaryKeyError(k, msg)
 
+            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_()
+                    try:
+                        type_instance.clean(value=dictified[k], allow_custom=False, strict=True)
+                        clean = True
+                        break
+                    except ValueError:
+                        continue
+            if not clean:
+                raise ValueError("Dictionary Property does not support this value's type: ", type(dictified[k]))
+
         if len(dictified) < 1:
             raise ValueError("must not be empty.")
 
@@ -446,7 +485,7 @@ class HashesProperty(DictionaryProperty):
             if alg:
                 self.__alg_to_spec_name[alg] = spec_hash_name
 
-    def clean(self, value, allow_custom):
+    def clean(self, value, allow_custom, strict=False):
         # ignore the has_custom return value here; there is no customization
         # of DictionaryProperties.
         clean_dict, _ = super().clean(value, allow_custom)
@@ -494,7 +533,7 @@ class HashesProperty(DictionaryProperty):
 
 class BinaryProperty(Property):
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
         try:
             base64.b64decode(value)
         except (binascii.Error, TypeError):
@@ -504,7 +543,7 @@ class BinaryProperty(Property):
 
 class HexProperty(Property):
 
-    def clean(self, value, allow_custom=False):
+    def clean(self, value, allow_custom=False, strict=False):
         if not re.match(r"^([a-fA-F0-9]{2})+$", value):
             raise ValueError("must contain an even number of hexadecimal characters")
         return value, False
diff --git a/stix2/v21/observables.py b/stix2/v21/observables.py
index 0929fd3..6fa777e 100644
--- a/stix2/v21/observables.py
+++ b/stix2/v21/observables.py
@@ -181,7 +181,7 @@ class EmailMessage(_Observable):
         ('message_id', StringProperty()),
         ('subject', StringProperty()),
         ('received_lines', ListProperty(StringProperty)),
-        ('additional_header_fields', DictionaryProperty(valid_types="string_list", spec_version='2.1')),
+        ('additional_header_fields', DictionaryProperty(valid_types=[ListProperty(StringProperty)], spec_version='2.1')),
         ('body', StringProperty()),
         ('body_multipart', ListProperty(EmbeddedObjectProperty(type=EmailMIMEComponent))),
         ('raw_email_ref', ReferenceProperty(valid_types='artifact', spec_version='2.1')),
@@ -245,7 +245,7 @@ class PDFExt(_Extension):
     _properties = OrderedDict([
         ('version', StringProperty()),
         ('is_optimized', BooleanProperty()),
-        ('document_info_dict', DictionaryProperty(valid_types="string", spec_version='2.1')),
+        ('document_info_dict', DictionaryProperty(valid_types=[StringProperty], spec_version='2.1')),
         ('pdfid0', StringProperty()),
         ('pdfid1', StringProperty()),
     ])
@@ -261,7 +261,7 @@ class RasterImageExt(_Extension):
         ('image_height', IntegerProperty()),
         ('image_width', IntegerProperty()),
         ('bits_per_pixel', IntegerProperty()),
-        ('exif_tags', DictionaryProperty(valid_types=["string", "integer"], spec_version='2.1')),
+        ('exif_tags', DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')),
     ])
 
 
@@ -468,7 +468,7 @@ class HTTPRequestExt(_Extension):
         ('request_method', StringProperty(required=True)),
         ('request_value', StringProperty(required=True)),
         ('request_version', StringProperty()),
-        ('request_header', DictionaryProperty(valid_types="string_list", spec_version='2.1')),
+        ('request_header', DictionaryProperty(valid_types=[ListProperty(StringProperty)], spec_version='2.1')),
         ('message_body_length', IntegerProperty()),
         ('message_body_data_ref', ReferenceProperty(valid_types='artifact', spec_version='2.1')),
     ])
@@ -496,7 +496,7 @@ class SocketExt(_Extension):
         ('address_family', EnumProperty(NETWORK_SOCKET_ADDRESS_FAMILY, required=True)),
         ('is_blocking', BooleanProperty()),
         ('is_listening', BooleanProperty()),
-        ('options', DictionaryProperty(valid_types="integer", spec_version='2.1')),
+        ('options', DictionaryProperty(valid_types=[IntegerProperty], spec_version='2.1')),
         ('socket_type', EnumProperty(NETWORK_SOCKET_TYPE)),
         ('socket_descriptor', IntegerProperty(min=0)),
         ('socket_handle', IntegerProperty()),
@@ -550,7 +550,7 @@ class NetworkTraffic(_Observable):
         ('dst_byte_count', IntegerProperty(min=0)),
         ('src_packets', IntegerProperty(min=0)),
         ('dst_packets', IntegerProperty(min=0)),
-        ('ipfix', DictionaryProperty(valid_types=["string", "integer"], spec_version='2.1')),
+        ('ipfix', DictionaryProperty(valid_types=[StringProperty, IntegerProperty], spec_version='2.1')),
         ('src_payload_ref', ReferenceProperty(valid_types='artifact', spec_version='2.1')),
         ('dst_payload_ref', ReferenceProperty(valid_types='artifact', spec_version='2.1')),
         ('encapsulates_refs', ListProperty(ReferenceProperty(valid_types='network-traffic', spec_version='2.1'))),
@@ -634,7 +634,7 @@ class Process(_Observable):
         ('created_time', TimestampProperty()),
         ('cwd', StringProperty()),
         ('command_line', StringProperty()),
-        ('environment_variables', DictionaryProperty(valid_types="string", spec_version='2.1')),
+        ('environment_variables', DictionaryProperty(valid_types=[StringProperty], spec_version='2.1')),
         ('opened_connection_refs', ListProperty(ReferenceProperty(valid_types='network-traffic', spec_version='2.1'))),
         ('creator_user_ref', ReferenceProperty(valid_types='user-account', spec_version='2.1')),
         ('image_ref', ReferenceProperty(valid_types='file', spec_version='2.1')),


Test patch:
diff --git a/stix2/test/v21/test_observed_data.py b/stix2/test/v21/test_observed_data.py
index d2ccec4..91baa69 100644
--- a/stix2/test/v21/test_observed_data.py
+++ b/stix2/test/v21/test_observed_data.py
@@ -1176,14 +1176,14 @@ def test_incorrect_socket_options():
         )
     assert "Incorrect options key" == str(excinfo.value)
 
-    with pytest.raises(ValueError) as excinfo:
+    with pytest.raises(Exception) as excinfo:
         stix2.v21.SocketExt(
             is_listening=True,
             address_family="AF_INET",
             socket_type="SOCK_STREAM",
             options={"SO_RCVTIMEO": '100'},
         )
-    assert "Options value must be an integer" == str(excinfo.value)
+    assert "Dictionary Property does not support this value's type" in str(excinfo.value)
 
 
 def test_network_traffic_tcp_example():
diff --git a/stix2/test/v21/test_properties.py b/stix2/test/v21/test_properties.py
index b1768e7..fcb73a3 100644
--- a/stix2/test/v21/test_properties.py
+++ b/stix2/test/v21/test_properties.py
@@ -7,8 +7,8 @@ from stix2.exceptions import (
 )
 from stix2.properties import (
     DictionaryProperty, EmbeddedObjectProperty, ExtensionsProperty,
-    HashesProperty, IDProperty, ListProperty, ObservableProperty,
-    ReferenceProperty, STIXObjectProperty,
+    HashesProperty, IDProperty, IntegerProperty, ListProperty,
+    ObservableProperty, ReferenceProperty, STIXObjectProperty, StringProperty,
 )
 from stix2.v21.common import MarkingProperty
 
@@ -20,8 +20,51 @@ def test_dictionary_property():
 
     assert p.clean({'spec_version': '2.1'})
     with pytest.raises(ValueError):
-        p.clean({})
+        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'