search

cloudevents / sdk-go

Go SDK for CloudEvents
https://cloudevents.github.io/sdk-go/
Apache License 2.0
837 stars 223 forks source link

bugfix_value_type_of_dataschema #985

Closed timmyb32r closed 1 year ago

timmyb32r commented 1 year ago

Accordingly to spec, attribute 'dataschema' has type URI: https://github.com/cloudevents/spec/blob/main/cloudevents/spec.md#dataschema

but in trunk now, function ToProto do:

container.Attributes[dataschema], _ = attributeFor(e.DataSchema())

DataSchema() returns string attributeFor() got string as an input and pack it into pb.CloudEventAttributeValue_CeString - what violates the spec.

Existing tests TestProtobufFormatWithoutProtobufCodec & TestProtobufFormatWithProtobufCodec work only bcs marshal converts it into string, and unmarshal takes this value from string. But if add into test TestFromProto string

"dataschema": {Attr: &pb.CloudEventAttributeValue_CeUri{CeUri: "link"}}, and out.SetDataSchema("link")

it will be broken, bcs unmarshal silently ignores an error here: https://github.com/cloudevents/sdk-go/blob/main/binding/format/protobuf/v2/protobuf.go#L254

I occurred this problem when our users made correct cloudevents message with some non-golang sdk, and send it to our code.

So, I've made bugfix & changed test TestFromProto - now it fails on current trunk, and works with my fix. So, that is proof of problem in trunk & proof of correctness of bugfix.

duglin commented 1 year ago

please sign your commit(s)

duglin commented 1 year ago

@timmyb32r can you please sign your commits?

timmyb32r commented 1 year ago

@duglin made it in #986 (it this branch I've made typo and couldn't fix it)