cloudevents / sdk-php

PHP SDK for CloudEvents
Apache License 2.0
80 stars 21 forks source link

JSON data in an extension attribute causes exceptions when Deserializing #56

Open dannholmes opened 2 months ago

dannholmes commented 2 months ago

I'm trying to use the deserializedStructured function to load a CloudEvents message coming in as a JSON string.

If I leave out the metadata extension field in the below message the function works correctly, but with the metadata extension I get a couple of errors: invalid message exception TypeError: CloudEvents\V1\CloudEventTrait::setExtension(): Argument #2 ($value) must be of type bool|int|string|null, array given in ....src/V1/CloudEventTrait.php:282

and

Next CloudEvents\Exceptions\InvalidAttributeException: Invalid CloudEvent attribute. ... src/Serializers/Normalizers/V1/Denormalizer.php:43

It looks like when the message is passed into deserializedStructured, the json_decode turns any extension field like this into an array. Then when validating data in CloudEventTrait/setExtension it's failing as it's not matching the type.

However, this same message will validate correctly in both the Python SDK and when checking against the CloudEvents validator here: https://www.itb.ec.europa.eu/json/cloudevents/upload

It seems that just adding array to the list of accepted types and changing the null data check to an empty() check instead would work, but I haven't tried everything yet. It does pass the current test suite, though.

Before I make a PR for this, I just wanted to check and see if this was expected behavior from your side.

Example message: { "specversion": "1.0", "type": "testtype", "source": "testsource", "subject": "testsubject", "id": "1234-4567-7890", "time": "2024-09-06T17:15:23Z", "data": {"one": "test"}, "extensionone": "extension", "metadata": { "field": "1", "secondfield": "2" } }

GrahamCampbell commented 2 months ago

Thanks for the report. I believe this is a mistake by the Python SDK, or at least, a decision to delegate the correct following of the spec to the user. Extension values must be one of the predefined available types from the spec. If you'd like to use an object as an extension value, I believe you must JSON encode it yourself, first, before trying to set it as the value.

dannholmes commented 2 months ago

Amusingly, I tried the same message in Go and node.js - Go called it out as invalid while node.js was fine with it.

It seems to be a bit of a grey area where it comes to the documentation as well, unless I'm missing something.