From da5978d31735124fd05373840ecba3318f1d0969 Mon Sep 17 00:00:00 2001 From: Michael Chisholm Date: Thu, 13 Jun 2019 18:37:21 -0400 Subject: [PATCH] Factored out more of the STIX identifier validity checking, partly inspired by PR #263. This resulted in some error message format changes (an improvement, I think), which caused some unit test breakage. Removed those asserts from the unit tests, since tests shouldn't be testing human-targeted error messages. --- stix2/properties.py | 67 ++++++++++++++++---------------- stix2/test/v20/test_indicator.py | 2 - stix2/test/v20/test_report.py | 2 - stix2/test/v20/test_sighting.py | 4 -- stix2/test/v21/test_indicator.py | 2 - stix2/test/v21/test_report.py | 2 - stix2/test/v21/test_sighting.py | 2 - 7 files changed, 34 insertions(+), 47 deletions(-) diff --git a/stix2/properties.py b/stix2/properties.py index d0349c3..71138e7 100644 --- a/stix2/properties.py +++ b/stix2/properties.py @@ -41,6 +41,38 @@ def _check_uuid(uuid_str, spec_version): return ok +def _validate_id(id_, spec_version, required_prefix): + """ + Check the STIX identifier for correctness, raise an exception if there are + errors. + + :param id_: The STIX identifier + :param spec_version: The STIX specification version to use + :param required_prefix: The required prefix on the identifier, if any. + This function doesn't add a "--" suffix to the prefix, so callers must + add it if it is important. Pass None to skip the prefix check. + :raises ValueError: If there are any errors with the identifier + """ + if required_prefix: + if not id_.startswith(required_prefix): + raise ValueError("must start with '{}'.".format(required_prefix)) + + try: + if required_prefix: + uuid_part = id_[len(required_prefix):] + else: + idx = id_.index("--") + uuid_part = id_[idx+2:] + + result = _check_uuid(uuid_part, spec_version) + except ValueError: + # replace their ValueError with ours + raise ValueError(ERROR_INVALID_ID.format(id_)) + + if not result: + raise ValueError(ERROR_INVALID_ID.format(id_)) + + class Property(object): """Represent a property of STIX data type. @@ -198,19 +230,7 @@ class IDProperty(Property): super(IDProperty, self).__init__() def clean(self, value): - if not value.startswith(self.required_prefix): - raise ValueError("must start with '{}'.".format(self.required_prefix)) - - uuid_part = value[len(self.required_prefix):] - try: - result = _check_uuid(uuid_part, self.spec_version) - except ValueError: - # replace their ValueError with ours - raise ValueError(ERROR_INVALID_ID.format(value)) - - if not result: - raise ValueError(ERROR_INVALID_ID.format(value)) - + _validate_id(value, self.spec_version, self.required_prefix) return value def default(self): @@ -396,26 +416,7 @@ class ReferenceProperty(Property): value = value.id value = str(value) - if self.required_prefix: - if not value.startswith(self.required_prefix): - raise ValueError( - "must start with '{}'.".format(self.required_prefix), - ) - - try: - if self.required_prefix: - uuid_part = value[len(self.required_prefix):] - else: - idx = value.index("--") - uuid_part = value[idx+2:] - - result = _check_uuid(uuid_part, self.spec_version) - except ValueError: - # replace their ValueError with ours - raise ValueError(ERROR_INVALID_ID.format(value)) - - if not result: - raise ValueError(ERROR_INVALID_ID.format(value)) + _validate_id(value, self.spec_version, self.required_prefix) return value diff --git a/stix2/test/v20/test_indicator.py b/stix2/test/v20/test_indicator.py index f8c3a91..5194142 100644 --- a/stix2/test/v20/test_indicator.py +++ b/stix2/test/v20/test_indicator.py @@ -112,8 +112,6 @@ def test_indicator_created_ref_invalid_format(): assert excinfo.value.cls == stix2.v20.Indicator assert excinfo.value.prop_name == "created_by_ref" - assert excinfo.value.reason == "must start with 'identity'." - assert str(excinfo.value) == "Invalid value for Indicator 'created_by_ref': must start with 'identity'." def test_indicator_revoked_invalid(): diff --git a/stix2/test/v20/test_report.py b/stix2/test/v20/test_report.py index 072fc95..fc4f288 100644 --- a/stix2/test/v20/test_report.py +++ b/stix2/test/v20/test_report.py @@ -87,8 +87,6 @@ def test_report_example_objects_in_object_refs_with_bad_id(): assert excinfo.value.cls == stix2.v20.Report assert excinfo.value.prop_name == "object_refs" - assert excinfo.value.reason == stix2.properties.ERROR_INVALID_ID - assert str(excinfo.value) == "Invalid value for Report 'object_refs': " + stix2.properties.ERROR_INVALID_ID @pytest.mark.parametrize( diff --git a/stix2/test/v20/test_sighting.py b/stix2/test/v20/test_sighting.py index e93ca7e..f1d40c5 100644 --- a/stix2/test/v20/test_sighting.py +++ b/stix2/test/v20/test_sighting.py @@ -59,8 +59,6 @@ def test_sighting_bad_where_sighted_refs(): assert excinfo.value.cls == stix2.v20.Sighting assert excinfo.value.prop_name == "where_sighted_refs" - assert excinfo.value.reason == "must start with 'identity'." - assert str(excinfo.value) == "Invalid value for Sighting 'where_sighted_refs': must start with 'identity'." def test_sighting_type_must_be_sightings(): @@ -69,8 +67,6 @@ def test_sighting_type_must_be_sightings(): assert excinfo.value.cls == stix2.v20.Sighting assert excinfo.value.prop_name == "type" - assert excinfo.value.reason == "must equal 'sighting'." - assert str(excinfo.value) == "Invalid value for Sighting 'type': must equal 'sighting'." def test_invalid_kwarg_to_sighting(): diff --git a/stix2/test/v21/test_indicator.py b/stix2/test/v21/test_indicator.py index 628bdff..a54ea62 100644 --- a/stix2/test/v21/test_indicator.py +++ b/stix2/test/v21/test_indicator.py @@ -116,8 +116,6 @@ def test_indicator_created_ref_invalid_format(): assert excinfo.value.cls == stix2.v21.Indicator assert excinfo.value.prop_name == "created_by_ref" - assert excinfo.value.reason == "must start with 'identity'." - assert str(excinfo.value) == "Invalid value for Indicator 'created_by_ref': must start with 'identity'." def test_indicator_revoked_invalid(): diff --git a/stix2/test/v21/test_report.py b/stix2/test/v21/test_report.py index c9d790e..8b7ee78 100644 --- a/stix2/test/v21/test_report.py +++ b/stix2/test/v21/test_report.py @@ -88,8 +88,6 @@ def test_report_example_objects_in_object_refs_with_bad_id(): assert excinfo.value.cls == stix2.v21.Report assert excinfo.value.prop_name == "object_refs" - assert excinfo.value.reason == stix2.properties.ERROR_INVALID_ID - assert str(excinfo.value) == "Invalid value for Report 'object_refs': " + stix2.properties.ERROR_INVALID_ID @pytest.mark.parametrize( diff --git a/stix2/test/v21/test_sighting.py b/stix2/test/v21/test_sighting.py index 8fcbb6d..cfdc7ea 100644 --- a/stix2/test/v21/test_sighting.py +++ b/stix2/test/v21/test_sighting.py @@ -61,8 +61,6 @@ def test_sighting_bad_where_sighted_refs(): assert excinfo.value.cls == stix2.v21.Sighting assert excinfo.value.prop_name == "where_sighted_refs" - assert excinfo.value.reason == "must start with 'identity'." - assert str(excinfo.value) == "Invalid value for Sighting 'where_sighted_refs': must start with 'identity'." def test_sighting_type_must_be_sightings():