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",