diff --git a/CHANGELOG.md b/CHANGELOG.md index c24d9ec..1a089c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,18 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.2.0] +### Added +- Added GenericException, DataMarshallingError and DataUnmarshallingError ([#120]) + ## [1.1.0] ### Changed - Changed from_http to now expect headers argument before data ([#110]) - Renamed exception names ([#111]) +### Fixed +- Fixed from_http bugs with data of type None, or not dict-like ([#119]) + ### Deprecated - Renamed to_binary_http and to_structured_http. ([#108]) @@ -105,3 +112,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#108]: https://github.com/cloudevents/sdk-python/pull/108 [#110]: https://github.com/cloudevents/sdk-python/pull/110 [#111]: https://github.com/cloudevents/sdk-python/pull/111 +[#119]: https://github.com/cloudevents/sdk-python/pull/119 +[#120]: https://github.com/cloudevents/sdk-python/pull/120 \ No newline at end of file diff --git a/cloudevents/__init__.py b/cloudevents/__init__.py index 6849410..c68196d 100644 --- a/cloudevents/__init__.py +++ b/cloudevents/__init__.py @@ -1 +1 @@ -__version__ = "1.1.0" +__version__ = "1.2.0" diff --git a/cloudevents/exceptions.py b/cloudevents/exceptions.py index 776e58a..e33b320 100644 --- a/cloudevents/exceptions.py +++ b/cloudevents/exceptions.py @@ -11,17 +11,29 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -class MissingRequiredFields(Exception): +class GenericException(Exception): pass -class InvalidRequiredFields(Exception): +class MissingRequiredFields(GenericException): pass -class InvalidStructuredJSON(Exception): +class InvalidRequiredFields(GenericException): pass -class InvalidHeadersFormat(Exception): +class InvalidStructuredJSON(GenericException): + pass + + +class InvalidHeadersFormat(GenericException): + pass + + +class DataMarshallerError(GenericException): + pass + + +class DataUnmarshallerError(GenericException): pass diff --git a/cloudevents/http/event.py b/cloudevents/http/event.py index 5cb2efb..7cf10fa 100644 --- a/cloudevents/http/event.py +++ b/cloudevents/http/event.py @@ -59,14 +59,14 @@ class CloudEvent: if self._attributes["specversion"] not in _required_by_version: raise cloud_exceptions.MissingRequiredFields( - f"Invalid specversion: {self._attributes['specversion']}. " + f"Invalid specversion: {self._attributes['specversion']}" ) # There is no good way to default 'source' and 'type', so this # checks for those (or any new required attributes). required_set = _required_by_version[self._attributes["specversion"]] if not required_set <= self._attributes.keys(): raise cloud_exceptions.MissingRequiredFields( - f"Missing required keys: {required_set - self._attributes.keys()}. " + f"Missing required keys: {required_set - self._attributes.keys()}" ) def __eq__(self, other): diff --git a/cloudevents/http/http_methods.py b/cloudevents/http/http_methods.py index ef186de..086e388 100644 --- a/cloudevents/http/http_methods.py +++ b/cloudevents/http/http_methods.py @@ -20,19 +20,21 @@ def from_http( Unwrap a CloudEvent (binary or structured) from an HTTP request. :param headers: the HTTP headers :type headers: typing.Dict[str, str] - :param data: the HTTP request body + :param data: the HTTP request body. If set to None, "" or b'', the returned + event's data field will be set to None :type data: typing.IO :param data_unmarshaller: Callable function to map data to a python object e.g. lambda x: x or lambda x: json.loads(x) :type data_unmarshaller: types.UnmarshallerType """ - if data is None: + if data is None or data == b"": + # Empty string will cause data to be marshalled into None data = "" if not isinstance(data, (str, bytes, bytearray)): raise cloud_exceptions.InvalidStructuredJSON( "Expected json of type (str, bytes, bytearray), " - f"but instead found {type(data)}. " + f"but instead found type {type(data)}" ) headers = {key.lower(): value for key, value in headers.items()} @@ -47,22 +49,28 @@ def from_http( try: raw_ce = json.loads(data) except json.decoder.JSONDecodeError: - raise cloud_exceptions.InvalidStructuredJSON( - "Failed to read fields from structured event. " - f"The following can not be parsed as json: {data}. " + raise cloud_exceptions.MissingRequiredFields( + "Failed to read specversion from both headers and data. " + f"The following can not be parsed as json: {data}" + ) + if hasattr(raw_ce, "get"): + specversion = raw_ce.get("specversion", None) + else: + raise cloud_exceptions.MissingRequiredFields( + "Failed to read specversion from both headers and data. " + f"The following deserialized data has no 'get' method: {raw_ce}" ) - specversion = raw_ce.get("specversion", None) if specversion is None: raise cloud_exceptions.MissingRequiredFields( - "Failed to find specversion in HTTP request. " + "Failed to find specversion in HTTP request" ) event_handler = _obj_by_version.get(specversion, None) if event_handler is None: raise cloud_exceptions.InvalidRequiredFields( - f"Found invalid specversion {specversion}. " + f"Found invalid specversion {specversion}" ) event = marshall.FromRequest( @@ -73,7 +81,13 @@ def from_http( attrs.pop("extensions", None) attrs.update(**event.extensions) - return CloudEvent(attrs, event.data) + if event.data == "" or event.data == b"": + # TODO: Check binary unmarshallers to debug why setting data to "" + # returns an event with data set to None, but structured will return "" + data = None + else: + data = event.data + return CloudEvent(attrs, data) def _to_http( @@ -96,7 +110,7 @@ def _to_http( if event._attributes["specversion"] not in _obj_by_version: raise cloud_exceptions.InvalidRequiredFields( - f"Unsupported specversion: {event._attributes['specversion']}. " + f"Unsupported specversion: {event._attributes['specversion']}" ) event_handler = _obj_by_version[event._attributes["specversion"]]() diff --git a/cloudevents/http/util.py b/cloudevents/http/util.py index 2dfb3bb..816b2d0 100644 --- a/cloudevents/http/util.py +++ b/cloudevents/http/util.py @@ -3,7 +3,7 @@ import typing def default_marshaller(content: any): - if content is None or len(content) == 0: + if content is None: return None try: return json.dumps(content) @@ -12,7 +12,7 @@ def default_marshaller(content: any): def _json_or_string(content: typing.Union[str, bytes]): - if content is None or len(content) == 0: + if content is None: return None try: return json.loads(content) diff --git a/cloudevents/sdk/event/base.py b/cloudevents/sdk/event/base.py index 9903e40..7dc5d72 100644 --- a/cloudevents/sdk/event/base.py +++ b/cloudevents/sdk/event/base.py @@ -201,7 +201,14 @@ class BaseEvent(EventGetterSetter): data_marshaller = lambda x: x # noqa: E731 props = self.Properties() if "data" in props: - data = data_marshaller(props.pop("data")) + data = props.pop("data") + try: + data = data_marshaller(data) + except Exception as e: + raise cloud_exceptions.DataMarshallerError( + "Failed to marshall data with error: " + f"{type(e).__name__}('{e}')" + ) if isinstance(data, (bytes, bytes, memoryview)): props["data_base64"] = base64.b64encode(data).decode("ascii") else: @@ -225,14 +232,23 @@ class BaseEvent(EventGetterSetter): ) for name, value in raw_ce.items(): + decoder = lambda x: x if name == "data": # Use the user-provided serializer, which may have customized # JSON decoding - value = data_unmarshaller(json.dumps(value)) + decoder = lambda v: data_unmarshaller(json.dumps(v)) if name == "data_base64": - value = data_unmarshaller(base64.b64decode(value)) + decoder = lambda v: data_unmarshaller(base64.b64decode(v)) name = "data" - self.Set(name, value) + + try: + set_value = decoder(value) + except Exception as e: + raise cloud_exceptions.DataUnmarshallerError( + "Failed to unmarshall data with error: " + f"{type(e).__name__}('{e}')" + ) + self.Set(name, set_value) def UnmarshalBinary( self, @@ -256,7 +272,15 @@ class BaseEvent(EventGetterSetter): self.SetContentType(value) elif header.startswith("ce-"): self.Set(header[3:], value) - self.Set("data", data_unmarshaller(body)) + + try: + raw_ce = data_unmarshaller(body) + except Exception as e: + raise cloud_exceptions.DataUnmarshallerError( + "Failed to unmarshall data with error: " + f"{type(e).__name__}('{e}')" + ) + self.Set("data", raw_ce) def MarshalBinary( self, data_marshaller: types.MarshallerType @@ -276,7 +300,13 @@ class BaseEvent(EventGetterSetter): headers["ce-{0}".format(key)] = value data, _ = self.Get("data") - data = data_marshaller(data) + try: + data = data_marshaller(data) + except Exception as e: + raise cloud_exceptions.DataMarshallerError( + "Failed to marshall data with error: " + f"{type(e).__name__}('{e}')" + ) if isinstance(data, str): # Convenience method for json.dumps data = data.encode("utf-8") return headers, data diff --git a/cloudevents/tests/test_http_cloudevent.py b/cloudevents/tests/test_http_cloudevent.py index de9331c..0568aa9 100644 --- a/cloudevents/tests/test_http_cloudevent.py +++ b/cloudevents/tests/test_http_cloudevent.py @@ -2,6 +2,7 @@ import pytest import cloudevents.exceptions as cloud_exceptions from cloudevents.http import CloudEvent +from cloudevents.http.util import _json_or_string @pytest.mark.parametrize("specversion", ["0.3", "1.0"]) @@ -75,19 +76,19 @@ def test_http_cloudevent_mutates_equality(specversion): def test_cloudevent_missing_specversion(): attributes = {"specversion": "0.2", "source": "s", "type": "t"} with pytest.raises(cloud_exceptions.MissingRequiredFields) as e: - event = CloudEvent(attributes, None) + _ = CloudEvent(attributes, None) assert "Invalid specversion: 0.2" in str(e.value) def test_cloudevent_missing_minimal_required_fields(): attributes = {"type": "t"} with pytest.raises(cloud_exceptions.MissingRequiredFields) as e: - event = CloudEvent(attributes, None) + _ = CloudEvent(attributes, None) assert f"Missing required keys: {set(['source'])}" in str(e.value) attributes = {"source": "s"} with pytest.raises(cloud_exceptions.MissingRequiredFields) as e: - event = CloudEvent(attributes, None) + _ = CloudEvent(attributes, None) assert f"Missing required keys: {set(['type'])}" in str(e.value) @@ -114,3 +115,7 @@ def test_cloudevent_general_overrides(): assert attribute in event del event[attribute] assert len(event) == 0 + + +def test_none_json_or_string(): + assert _json_or_string(None) is None diff --git a/cloudevents/tests/test_http_events.py b/cloudevents/tests/test_http_events.py index 6a9e692..01307d7 100644 --- a/cloudevents/tests/test_http_events.py +++ b/cloudevents/tests/test_http_events.py @@ -90,11 +90,8 @@ async def echo(request): @pytest.mark.parametrize("body", invalid_cloudevent_request_body) def test_missing_required_fields_structured(body): - with pytest.raises(cloud_exceptions.MissingRequiredFields): - # CloudEvent constructor throws TypeError if missing required field - # and NotImplementedError because structured calls aren't - # implemented. In this instance one of the required keys should have - # prefix e-id instead of ce-id therefore it should throw + with pytest.raises(cloud_exceptions.MissingRequiredFields) as e: + _ = from_http( {"Content-Type": "application/cloudevents+json"}, json.dumps(body), ) @@ -103,13 +100,16 @@ def test_missing_required_fields_structured(body): @pytest.mark.parametrize("headers", invalid_test_headers) def test_missing_required_fields_binary(headers): with pytest.raises(cloud_exceptions.MissingRequiredFields): - # CloudEvent constructor throws TypeError if missing required field - # and NotImplementedError because structured calls aren't - # implemented. In this instance one of the required keys should have - # prefix e-id instead of ce-id therefore it should throw _ = from_http(headers, json.dumps(test_data)) +@pytest.mark.parametrize("headers", invalid_test_headers) +def test_missing_required_fields_empty_data_binary(headers): + # Test for issue #115 + with pytest.raises(cloud_exceptions.MissingRequiredFields): + _ = from_http(headers, None) + + @pytest.mark.parametrize("specversion", ["1.0", "0.3"]) def test_emit_binary_event(specversion): headers = { @@ -286,9 +286,17 @@ def test_empty_data_structured_event(specversion): "source": "", } - _ = from_http( + event = from_http( {"content-type": "application/cloudevents+json"}, json.dumps(attributes) ) + assert event.data == None + + attributes["data"] = "" + # Data of empty string will be marshalled into None + event = from_http( + {"content-type": "application/cloudevents+json"}, json.dumps(attributes) + ) + assert event.data == None @pytest.mark.parametrize("specversion", ["1.0", "0.3"]) @@ -302,7 +310,13 @@ def test_empty_data_binary_event(specversion): "ce-time": "2018-10-23T12:28:22.4579346Z", "ce-source": "", } - _ = from_http(headers, "") + event = from_http(headers, None) + assert event.data == None + + data = "" + # Data of empty string will be marshalled into None + event = from_http(headers, data) + assert event.data == None @pytest.mark.parametrize("specversion", ["1.0", "0.3"]) @@ -450,11 +464,13 @@ def test_is_structured(): def test_empty_json_structured(): headers = {"Content-Type": "application/cloudevents+json"} data = "" - with pytest.raises(cloud_exceptions.InvalidStructuredJSON) as e: + with pytest.raises(cloud_exceptions.MissingRequiredFields) as e: from_http( headers, data, ) - assert "Failed to read fields from structured event. " in str(e.value) + assert "Failed to read specversion from both headers and data" in str( + e.value + ) def test_uppercase_headers_with_none_data_binary(): @@ -472,3 +488,46 @@ def test_uppercase_headers_with_none_data_binary(): _, new_data = to_binary(event) assert new_data == None + + +def test_generic_exception(): + headers = {"Content-Type": "application/cloudevents+json"} + data = json.dumps( + { + "specversion": "1.0", + "source": "s", + "type": "t", + "id": "1234-1234-1234", + "data": "", + } + ) + with pytest.raises(cloud_exceptions.GenericException) as e: + from_http({}, None) + e.errisinstance(cloud_exceptions.MissingRequiredFields) + + with pytest.raises(cloud_exceptions.GenericException) as e: + from_http({}, 123) + e.errisinstance(cloud_exceptions.InvalidStructuredJSON) + + with pytest.raises(cloud_exceptions.GenericException) as e: + from_http(headers, data, data_unmarshaller=lambda x: 1 / 0) + e.errisinstance(cloud_exceptions.DataUnmarshallerError) + + with pytest.raises(cloud_exceptions.GenericException) as e: + event = from_http(headers, data) + to_binary(event, data_marshaller=lambda x: 1 / 0) + e.errisinstance(cloud_exceptions.DataMarshallerError) + + +def test_non_dict_data_no_headers_bug(): + # Test for issue #116 + headers = {"Content-Type": "application/cloudevents+json"} + data = "123" + with pytest.raises(cloud_exceptions.MissingRequiredFields) as e: + from_http( + headers, data, + ) + assert "Failed to read specversion from both headers and data" in str( + e.value + ) + assert "The following deserialized data has no 'get' method" in str(e.value) diff --git a/cloudevents/tests/test_marshaller.py b/cloudevents/tests/test_marshaller.py index 2bb0e37..17e7e48 100644 --- a/cloudevents/tests/test_marshaller.py +++ b/cloudevents/tests/test_marshaller.py @@ -12,15 +12,19 @@ # License for the specific language governing permissions and limitations # under the License. +import json + import pytest +import cloudevents.exceptions as cloud_exceptions +from cloudevents.http import CloudEvent, from_http, to_binary, to_structured from cloudevents.sdk import converters, exceptions, marshaller from cloudevents.sdk.converters import binary, structured from cloudevents.sdk.event import v1 @pytest.fixture -def headers(): +def binary_headers(): return { "ce-specversion": "1.0", "ce-source": "1.0", @@ -29,6 +33,19 @@ def headers(): } +@pytest.fixture +def structured_data(): + return json.dumps( + { + "specversion": "1.0", + "source": "pytest", + "type": "com.pytest.test", + "id": "1234-1234-1234", + "data": "test", + } + ) + + def test_from_request_wrong_unmarshaller(): with pytest.raises(exceptions.InvalidDataUnmarshaller): m = marshaller.NewDefaultHTTPMarshaller() @@ -41,7 +58,7 @@ def test_to_request_wrong_marshaller(): _ = m.ToRequest(v1.Event(), data_marshaller="") -def test_from_request_cannot_read(headers): +def test_from_request_cannot_read(binary_headers): with pytest.raises(exceptions.UnsupportedEventConverter): m = marshaller.HTTPMarshaller( [binary.NewBinaryHTTPCloudEventConverter(),] @@ -52,7 +69,7 @@ def test_from_request_cannot_read(headers): m = marshaller.HTTPMarshaller( [structured.NewJSONHTTPCloudEventConverter()] ) - m.FromRequest(v1.Event(), headers, "") + m.FromRequest(v1.Event(), binary_headers, "") def test_to_request_invalid_converter(): @@ -61,3 +78,65 @@ def test_to_request_invalid_converter(): [structured.NewJSONHTTPCloudEventConverter()] ) m.ToRequest(v1.Event(), "") + + +def test_http_data_unmarshaller_exceptions(binary_headers, structured_data): + # binary + with pytest.raises(cloud_exceptions.DataUnmarshallerError) as e: + from_http(binary_headers, None, data_unmarshaller=lambda x: 1 / 0) + assert ( + "Failed to unmarshall data with error: " + "ZeroDivisionError('division by zero')" in str(e.value) + ) + + # structured + headers = {"Content-Type": "application/cloudevents+json"} + with pytest.raises(cloud_exceptions.DataUnmarshallerError) as e: + from_http(headers, structured_data, data_unmarshaller=lambda x: 1 / 0) + assert ( + "Failed to unmarshall data with error: " + "ZeroDivisionError('division by zero')" in str(e.value) + ) + + +def test_http_data_marshaller_exception(binary_headers, structured_data): + # binary + event = from_http(binary_headers, None) + with pytest.raises(cloud_exceptions.DataMarshallerError) as e: + to_binary(event, data_marshaller=lambda x: 1 / 0) + assert ( + "Failed to marshall data with error: " + "ZeroDivisionError('division by zero')" in str(e.value) + ) + + # structured + headers = {"Content-Type": "application/cloudevents+json"} + + event = from_http(headers, structured_data) + with pytest.raises(cloud_exceptions.DataMarshallerError) as e: + to_structured(event, data_marshaller=lambda x: 1 / 0) + assert ( + "Failed to marshall data with error: " + "ZeroDivisionError('division by zero')" in str(e.value) + ) + + +@pytest.mark.parametrize("test_data", [[], {}, (), "", b"", None]) +def test_known_empty_edge_cases(binary_headers, test_data): + expect_data = test_data + if test_data in ["", b""]: + expect_data = None + elif test_data == (): + # json.dumps(()) outputs '[]' hence list not tuple check + expect_data = [] + + # Remove ce- prefix + headers = {key[3:]: value for key, value in binary_headers.items()} + + # binary + event = from_http(*to_binary(CloudEvent(headers, test_data))) + assert event.data == expect_data + + # structured + event = from_http(*to_structured(CloudEvent(headers, test_data))) + assert event.data == expect_data