Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CloudEvent extensions should be restricted to types allowed in spec #229

Closed
lance opened this issue Jun 20, 2020 · 2 comments · Fixed by #251
Closed

CloudEvent extensions should be restricted to types allowed in spec #229

lance opened this issue Jun 20, 2020 · 2 comments · Fixed by #251
Labels
spec/1.0 Issues related to the CE specification type/bug Something isn't working version/3.x Issues related to the 3.0 release of this library

Comments

@lance
Copy link
Member

lance commented Jun 20, 2020

Currently an extension can have any value - e.g. it could be an object. However, the spec limits what value types an extension can have. See https://github.com/cloudevents/spec/blob/master/spec.md#type-system

@lance lance added type/bug Something isn't working spec/1.0 Issues related to the CE specification labels Jun 20, 2020
@lholmquist lholmquist added the version/3.x Issues related to the 3.0 release of this library label Jul 8, 2020
@lholmquist
Copy link
Contributor

lholmquist commented Jul 8, 2020

Since the spec doesn't allow an Object, should we be throwing an error if someone passes in this:

new CloudEvent({
...
...
extension1: { key: value }
})

Or is this ok since it will get converted to a JSON before transmission. basically, should this test pass or fail: https://github.com/cloudevents/sdk-javascript/blob/master/test/integration/spec_1_tests.ts#L99

@lance
Copy link
Member Author

lance commented Jul 13, 2020

That test should probably fail. We can't assume that the receiver will know that the value needs to be converted to an object. Since strings are valid value types, the user should use JSON.stringify before setting the extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec/1.0 Issues related to the CE specification type/bug Something isn't working version/3.x Issues related to the 3.0 release of this library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants