Should throw if specversion attribute is missing

Describe the Bug

Does not throw if event doesn't have a specversion attribute, and fallbacks to 1.0. The spec says that the specversion attribute is mandatory, so I think this library should fail when it's missing, instead of "magically" falling back.

Steps to Reproduce

Receiver.accept({ 'Content-Type': 'application/cloudevents+json' }, { id: 'foo', source: 'foo', type: 'foo' });
Prints console.error saying it will fallback to 1.0
Validation passes

Expected Behavior
It should throw, since the specversion attribute is missing

https://runkit.com/embed/chsthvg529ba

@satazor

@satazor thanks for pointing this out. I will look into dealing with this in #328

@satazor

@satazor in this PR, there was discussion around loose validation on receipt of a CloudEvent. Based on input from the community, noted in that comment, receipt of a CloudEvent should have loose validation, since it should be possible to receive and recover from poorly formed/invalid events. The example you provide is when receiving events on the wire - just such a scenario. If you would like to check whether the received event is valid, you can call CloudEvent#validate().

That method returns a boolean. I think it might be nice to see what the validation errors actually were, and we don't really have a convenience function for that. In any case, this is expected behavior.

Oh - but I see the problem here. Since it's defaulting the specversion, property, by the time it's unmarshalled as a CloudEvent, it is valid, so calling ce.validate() won't help.

The other required attribute that we supply a default for if it's not provided is id. The same would happen for this.

This issue is stale because it has been open 30 days with no activity.

I know this has been open for a while, but the idea is that we shouldn't be defaulting the specversion and id fields?

This would be a good one to get into the 4.0 release. i can assign myself to take of it

I know this has been open for a while, but the idea is that we shouldn't be defaulting the specversion and id fields?

I think defaulting them on creation with new CloudEvent() is fine. But when receiving events, we should not just assume that they are v1.0. That's what we are doing here.

sdk-javascript/src/message/http/index.ts

Lines 58 to 60 in f6be285

    
           if (version !== Version.V03 && version !== Version.V1) { 
        
             console.error(`Unknown spec version ${version}. Default to ${Version.V1}`); 
        
             version = Version.V1;

Throwing here would also remove that console.error() and resolve #333

@lance

@lance that sounds good. i'll take of it

src(receiver)!: don't default specversion on receiving.

I think defaulting them on creation with new CloudEvent() is fine. But when receiving events, we should not just assume that they are v1.0. That's what we are doing here.

I think this should also be the same for the id attribute, right?

As of now, HTTP.toEvent() will also assign a new uuidv4 to an event if passed in structured mode with no id present. This time even implicitly without a log message that it was defaulted.

parseStructured will call the CloudEvent constructor with eventObj missing the id attribute and generate it.

sdk-javascript/src/message/http/index.ts

Line 231 in 6be3b27

return new CloudEvent(eventObj as CloudEventV1 | CloudEventV03, false);

sdk-javascript/src/event/cloudevent.ts

Line 61 in 6be3b27

this.id = (properties.id as string) || uuidv4();

PS: the same would also happen with the time attribute. Maybe less important though because not a required attribute.

FYI, there is another effect that when calling HTTP.toEvent with a structured mode event and a specversion value different from 1.0 or 0.3 (e.g. 2), than the defaulting here is applied:

sdk-javascript/src/message/http/index.ts

Line 58 in 6be3b27

if (version !== Version.V03 && version !== Version.V1) {

but the parsed event is actually getting the original value (e.g. 2) assigned by this block:

sdk-javascript/src/message/http/index.ts

Line 208 in 6be3b27

for (const key in parserMap) {

in binary mode this is prevented by this block:

sdk-javascript/src/message/http/index.ts

Line 129 in 6be3b27

if (

no sure if therefore this block for structured should use the body attributes instead (I think sending ce-header even though structured mode is used is valid as long as the context attributes are present in the body)?

sdk-javascript/src/message/http/index.ts

Line 189 in 6be3b27

if (

This issue is stale because it has been open 30 days with no activity.

@lholmquist

@lholmquist ping - were you still thinking you'd take this? If not, I'll pick it up. Running into this as an issue now with function runtime.

feat: add TypeScript type definitions

@lance

@lance it isn't on my radar atm, so feel free to pick it up

fix: do not modify incoming event's specversion

fix: do not modify incoming event's specversion (#419)

fix: do not modify incoming event's specversion (cloudevents#419)

Navigation Menu

Repository navigation

Should throw if specversion attribute is missing #332

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (version !== Version.V03 && version !== Version.V1) {
	console.error(`Unknown spec version ${version}. Default to ${Version.V1}`);
	version = Version.V1;

Should throw if specversion attribute is missing #332

Description

Activity

satazor commented on Sep 2, 2020

lance commented on Sep 2, 2020

lance commented on Sep 3, 2020

lance commented on Sep 3, 2020

lance commented on Sep 3, 2020

github-actions commented on Oct 4, 2020

lholmquist commented on Oct 7, 2020

lance commented on Oct 7, 2020

lholmquist commented on Oct 7, 2020

1 remaining item

c-pius commented on Feb 4, 2021

c-pius commented on Feb 11, 2021

github-actions commented on Mar 14, 2021

lance commented on May 17, 2021

lholmquist commented on May 18, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions