From 2cda97cf5e51403c43a96ceb9073afab7706638a Mon Sep 17 00:00:00 2001 From: Michael Chisholm Date: Tue, 6 Jul 2021 20:32:58 -0400 Subject: [PATCH] Changed STIX object initialization to formulate a property order and process properties in that order. This establishes iteration order on object properties, making the object_properties() method unnecessary. So the latter method has been deleted. All uses of that method have been removed. Removed unnecessary deepcopy() in STIXJSONEncoder, to improve efficiency. This uncovered a bug which had been affecting STIXdatetime instances. Not deepcopying doesn't trip the bug, which can change serialization format. This caused a unit test to fail, which was checking serialization format. I fixed the unit test. Fixed a bug in _STIXBase.__repr__ which caused it to omit all properties with falsey values. This caused several unit tests to break, since they were written against the old buggy repr format. Notably, 'revoked=False' was never included in reprs before, but it is now. --- stix2/base.py | 77 ++++++++------------- stix2/serialization.py | 4 +- stix2/test/v20/test_identity.py | 2 +- stix2/test/v20/test_indicator.py | 1 + stix2/test/v20/test_markings.py | 2 +- stix2/test/v21/test_extension_definition.py | 1 - stix2/test/v21/test_identity.py | 1 - stix2/test/v21/test_incident.py | 1 - stix2/test/v21/test_indicator.py | 3 +- stix2/test/v21/test_location.py | 6 +- stix2/test/v21/test_note.py | 1 + stix2/test/v21/test_opinion.py | 3 +- 12 files changed, 41 insertions(+), 61 deletions(-) diff --git a/stix2/base.py b/stix2/base.py index 69d21b3..24176f4 100644 --- a/stix2/base.py +++ b/stix2/base.py @@ -36,34 +36,6 @@ def get_required_properties(properties): class _STIXBase(collections.abc.Mapping): """Base class for STIX object types""" - def object_properties(self): - """ - Get a list of property names in a particular order: spec order for - spec defined properties, followed by toplevel-property-extension - properties (any order), followed by custom properties (any order). - - The returned list doesn't include only defined+extension properties, - nor does it include only assigned properties (i.e. those this object - actually possesses). It's a mix of both: the spec defined property - group and extension group include all of them, regardless of whether - they're present on this object; the custom group include only names of - properties present on this object. - - :return: A list of property names - """ - if self.__property_order is None: - custom_props = sorted( - self.keys() - self._properties.keys() - - self.__ext_property_names - ) - - # Any custom properties to the bottom - self.__property_order = list(self._properties) \ - + list(self.__ext_property_names) \ - + custom_props - - return self.__property_order - def _check_property(self, prop_name, prop, kwargs, allow_custom): if prop_name not in kwargs: if hasattr(prop, 'default'): @@ -206,20 +178,33 @@ class _STIXBase(collections.abc.Mapping): self._properties, registered_toplevel_extension_props ) - # object_properties() needs this; cache it here to avoid needing to - # recompute. - self.__ext_property_names = set(registered_toplevel_extension_props) - # object_properties() will compute this on first call, based on - # __ext_property_names above. Maybe it makes sense to not compute this - # unless really necessary. - self.__property_order = None + assigned_properties = collections.ChainMap(kwargs, custom_props) - # Remove any keyword arguments whose value is None or [] (i.e. empty list) - setting_kwargs = { - k: v - for k, v in itertools.chain(kwargs.items(), custom_props.items()) - if v is not None and v != [] - } + # Establish property order: spec-defined, toplevel extension, custom. + toplevel_extension_props = registered_toplevel_extension_props.keys() \ + | (kwargs.keys() - self._properties.keys() - custom_kwargs) + property_order = itertools.chain( + self._properties, + toplevel_extension_props, + sorted(all_custom_prop_names) + ) + + setting_kwargs = {} + + has_custom = bool(all_custom_prop_names) + for prop_name in property_order: + + prop_val = assigned_properties.get(prop_name) + if prop_val not in (None, []): + setting_kwargs[prop_name] = prop_val + + prop = defined_properties.get(prop_name) + if prop: + temp_custom = self._check_property( + prop_name, prop, setting_kwargs, allow_custom, + ) + + has_custom = has_custom or temp_custom # Detect any missing required properties required_properties = set( @@ -229,14 +214,6 @@ class _STIXBase(collections.abc.Mapping): if missing_kwargs: raise MissingPropertiesError(cls, missing_kwargs) - has_custom = bool(all_custom_prop_names) - for prop_name, prop_metadata in defined_properties.items(): - temp_custom = self._check_property( - prop_name, prop_metadata, setting_kwargs, allow_custom, - ) - - has_custom = has_custom or temp_custom - # Cache defaulted optional properties for serialization defaulted = [] for name, prop in defined_properties.items(): @@ -304,7 +281,7 @@ class _STIXBase(collections.abc.Mapping): return self.serialize() def __repr__(self): - props = ', '.join([f"{k}={self[k]!r}" for k in self.object_properties() if self.get(k)]) + props = ', '.join([f"{k}={self[k]!r}" for k in self]) return f'{self.__class__.__name__}({props})' def __deepcopy__(self, memo): diff --git a/stix2/serialization.py b/stix2/serialization.py index 2784d39..236b987 100644 --- a/stix2/serialization.py +++ b/stix2/serialization.py @@ -24,7 +24,7 @@ class STIXJSONEncoder(json.JSONEncoder): if isinstance(obj, (dt.date, dt.datetime)): return format_datetime(obj) elif isinstance(obj, stix2.base._STIXBase): - tmp_obj = dict(copy.deepcopy(obj)) + tmp_obj = dict(obj) for prop_name in obj._defaulted_optional_properties: del tmp_obj[prop_name] return tmp_obj @@ -177,7 +177,7 @@ def find_property_index(obj, search_key, search_value): if isinstance(obj, stix2.base._STIXBase): if search_key in obj and obj[search_key] == search_value: - idx = _find(obj.object_properties(), search_key) + idx = _find(list(obj), search_key) else: idx = _find_property_in_seq(obj.values(), search_key, search_value) elif isinstance(obj, dict): diff --git a/stix2/test/v20/test_identity.py b/stix2/test/v20/test_identity.py index c62da46..93787b3 100644 --- a/stix2/test/v20/test_identity.py +++ b/stix2/test/v20/test_identity.py @@ -74,6 +74,6 @@ def test_identity_with_custom(): ) assert identity.x_foo == "bar" - assert "x_foo" in identity.object_properties() + assert "x_foo" in identity # TODO: Add other examples diff --git a/stix2/test/v20/test_indicator.py b/stix2/test/v20/test_indicator.py index 47f4812..10a7015 100644 --- a/stix2/test/v20/test_indicator.py +++ b/stix2/test/v20/test_indicator.py @@ -28,6 +28,7 @@ EXPECTED_INDICATOR_REPR = "Indicator(" + " ".join( modified='2017-01-01T00:00:01.000Z', pattern="[file:hashes.MD5 = 'd41d8cd98f00b204e9800998ecf8427e']", valid_from='1970-01-01T00:00:01Z', + revoked=False, labels=['malicious-activity'] """.split(), ) + ")" diff --git a/stix2/test/v20/test_markings.py b/stix2/test/v20/test_markings.py index f7d15fa..70b46a6 100644 --- a/stix2/test/v20/test_markings.py +++ b/stix2/test/v20/test_markings.py @@ -21,7 +21,7 @@ EXPECTED_TLP_MARKING_DEFINITION = """{ EXPECTED_STATEMENT_MARKING_DEFINITION = """{ "type": "marking-definition", "id": "marking-definition--613f2e26-407d-48c7-9eca-b8e91df99dc9", - "created": "2017-01-20T00:00:00Z", + "created": "2017-01-20T00:00:00.000Z", "definition_type": "statement", "definition": { "statement": "Copyright 2016, Example Corp" diff --git a/stix2/test/v21/test_extension_definition.py b/stix2/test/v21/test_extension_definition.py index 97513c4..af13425 100644 --- a/stix2/test/v21/test_extension_definition.py +++ b/stix2/test/v21/test_extension_definition.py @@ -105,4 +105,3 @@ def test_extension_definition_with_custom(): ) assert extension_definition.x_foo == "bar" - assert "x_foo" in extension_definition.object_properties() diff --git a/stix2/test/v21/test_identity.py b/stix2/test/v21/test_identity.py index c235b4d..2f8747c 100644 --- a/stix2/test/v21/test_identity.py +++ b/stix2/test/v21/test_identity.py @@ -77,6 +77,5 @@ def test_identity_with_custom(): ) assert identity.x_foo == "bar" - assert "x_foo" in identity.object_properties() # TODO: Add other examples diff --git a/stix2/test/v21/test_incident.py b/stix2/test/v21/test_incident.py index 27bc254..fac9238 100644 --- a/stix2/test/v21/test_incident.py +++ b/stix2/test/v21/test_incident.py @@ -78,4 +78,3 @@ def test_incident_with_custom(): ) assert incident.x_foo == "bar" - assert "x_foo" in incident.object_properties() diff --git a/stix2/test/v21/test_indicator.py b/stix2/test/v21/test_indicator.py index e42ffba..6175948 100644 --- a/stix2/test/v21/test_indicator.py +++ b/stix2/test/v21/test_indicator.py @@ -30,7 +30,8 @@ EXPECTED_INDICATOR_REPR = "Indicator(" + " ".join( pattern="[file:hashes.MD5 = 'd41d8cd98f00b204e9800998ecf8427e']", pattern_type='stix', pattern_version='2.1', - valid_from='1970-01-01T00:00:01Z' + valid_from='1970-01-01T00:00:01Z', + revoked=False """.split(), ) + ")" diff --git a/stix2/test/v21/test_location.py b/stix2/test/v21/test_location.py index c2df5d3..5d3eab8 100644 --- a/stix2/test/v21/test_location.py +++ b/stix2/test/v21/test_location.py @@ -27,7 +27,8 @@ EXPECTED_LOCATION_1_REPR = "Location(" + " ".join( created='2016-04-06T20:03:00.000Z', modified='2016-04-06T20:03:00.000Z', latitude=48.8566, - longitude=2.3522""".split(), + longitude=2.3522, + revoked=False""".split(), ) + ")" EXPECTED_LOCATION_2 = """{ @@ -47,7 +48,8 @@ EXPECTED_LOCATION_2_REPR = "Location(" + " ".join( id='location--a6e9345f-5a15-4c29-8bb3-7dcc5d168d64', created='2016-04-06T20:03:00.000Z', modified='2016-04-06T20:03:00.000Z', - region='northern-america'""".split(), + region='northern-america', + revoked=False""".split(), ) + ")" diff --git a/stix2/test/v21/test_note.py b/stix2/test/v21/test_note.py index 2f20c8d..ca1fc6d 100644 --- a/stix2/test/v21/test_note.py +++ b/stix2/test/v21/test_note.py @@ -48,6 +48,7 @@ EXPECTED_OPINION_REPR = "Note(" + " ".join(( content='%s', authors=['John Doe'], object_refs=['campaign--8e2e2d2b-17d4-4cbf-938f-98ee46b3cd3f'], + revoked=False, external_references=[ExternalReference(source_name='job-tracker', external_id='job-id-1234')] """ % CONTENT ).split()) + ")" diff --git a/stix2/test/v21/test_opinion.py b/stix2/test/v21/test_opinion.py index 21c96d7..31cd0af 100644 --- a/stix2/test/v21/test_opinion.py +++ b/stix2/test/v21/test_opinion.py @@ -38,7 +38,8 @@ EXPECTED_OPINION_REPR = "Opinion(" + " ".join(( modified='2016-05-12T08:17:27.000Z', explanation="%s", opinion='strongly-disagree', - object_refs=['relationship--16d2358f-3b0d-4c88-b047-0da2f7ed4471'] + object_refs=['relationship--16d2358f-3b0d-4c88-b047-0da2f7ed4471'], + revoked=False """ % EXPLANATION ).split()) + ")"