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

SHOULD throw if ce-datacontenttype is present in binary mode? #381

Open
c-pius opened this issue Feb 8, 2021 · 2 comments
Open

SHOULD throw if ce-datacontenttype is present in binary mode? #381

c-pius opened this issue Feb 8, 2021 · 2 comments

Comments

@c-pius
Copy link

c-pius commented Feb 8, 2021

Describe the Bug
Spec states that the ce-datacontenttype header MUST NOT be present in a binary mode message. If it is passed anyway, no error is thrown and the given ce-datacontenttype is treated as extension attribute overwriting the value from the Content-Type header.

Steps to Reproduce
Use the following snippet:

const event = HTTP.toEvent({
  headers: {
    'Content-Type': 'application/json',
    'ce-id': '0815',
    'ce-specversion': '1.0',
    'ce-type': 'my.event.type',
    'ce-source': 'my.event.source',
    'ce-datacontenttype': 'text/xml',
  },
  body: {}
})

Check the resulting event:

{
  //...
  id: '0815',
  specversion: '1.0',
  type: 'my.event.type',
  source: 'my.event.source',
  datacontenttype: 'text/xml'
}

Expected Behavior
HTTP.toEvent should throw to prevent this.

Additional context
Not quite sure why the datacontenttype (also without ce- prefix) is part of the binary parsers so maybe it is intentional though?

[CONSTANTS.CE_ATTRIBUTES.CONTENT_TYPE]: parser(CONSTANTS.CE_ATTRIBUTES.CONTENT_TYPE),

And I assume this is also a bug (where it should use v03binaryParsers on the else path instead):

const parserMap: Record<string, MappedParser> = version === Version.V1 ? v1binaryParsers : v1binaryParsers;

@lance
Copy link
Member

lance commented Mar 5, 2021

HTTP.toEvent should throw to prevent this.

I don't think it should, to be honest. This is similar to other scenarios that have been pointed out with regard to receiving malformed events. The HTTP.toEvent() function is meant to be used when receiving events via incoming HTTP requests, and we should be as forgiving as possible. To me the debate is really, what should the datacontenttype value be in this scenario, text/xml or application/json?

Thanks for pointing out those two bugs. You're right that they are incorrect.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants