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

Make datacontenttype value check case-insensitive #282

Open
batrived opened this issue Mar 5, 2024 · 6 comments
Open

Make datacontenttype value check case-insensitive #282

batrived opened this issue Mar 5, 2024 · 6 comments

Comments

@batrived
Copy link

batrived commented Mar 5, 2024

SDK is using case-sensitive checks when formatting CloudEvents to Json based on "datacontenttype" value. MIME types type and subtype are case insensitive according to the spec.

For example, this code snippet throws exception:
System.ArgumentException: 'JsonEventFormatter cannot serialize data of type System.String with content type 'application/JSON''

CloudEvent cloudEvent = new CloudEvent
{
Id = "event-id",
Type = "event-type",
Source = new Uri("https://cloudevents.io/"),
Time = DateTimeOffset.UtcNow,
DataContentType = "application/JSON",
Data = "text"
};

CloudEventFormatter formatter = new JsonEventFormatter();
var httpContent = cloudEvent.ToHttpContent(ContentMode.Structured, formatter);

@jskeet
Copy link
Contributor

jskeet commented Mar 6, 2024

Could you clarify which spec (ideally with a link to the specific part) you mean? Is this a MIME types spec, or the CloudEvents spec? https://www.rfc-editor.org/rfc/rfc2046.html states that the charset parameter isn't case-sensitive, and that subtype names of image are not case-sensitive, but I don't see anything similar for "application".

@batrived
Copy link
Author

batrived commented Mar 6, 2024

@jskeet https://www.rfc-editor.org/rfc/rfc2045#section-2

From the above rfc:

All media type values, subtype values, and
parameter names as defined are case-insensitive. However, parameter
values are case-sensitive unless otherwise specified for the specific
parameter.

@jskeet
Copy link
Contributor

jskeet commented Mar 6, 2024

Thanks, that's helpful. Will look at what's required here - and also raise it with other CE SDK maintainers.

@captainsafia
Copy link
Contributor

We may want to take a look at the code ASP.NET uses for header comparisons (ref) here. It's used in ASP.NET to validate Accepts headers and handle the case-sensitivity rules correctly, AFAIK. It also has some capabilities to filter out invalid subtypes like th application/cloudeventsfoobar that is mentioned in this TODO.

@jskeet
Copy link
Contributor

jskeet commented Mar 8, 2024

We may want to take a look at the code ASP.NET uses for header comparisons (ref) here.

Yes, it's a shame that's not all in System.Net.Http.Headers.MediaTypeHeaderValue :( I suspect we'll need a really careful sweep through the code to find everything. I'll start keeping a log here...

@captainsafia
Copy link
Contributor

Yes, it's a shame that's not all in System.Net.Http.Headers.MediaTypeHeaderValue :(

Yes, the Microsoft.Net.Headers stuff was specifically optimized for server/web scenarios as opposed to the "general purpose" types in System.Net.Http.Headers. We can get away with doing source-based code sharing to bring in the functionality from Microsoft.Net.Headers for this.

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

No branches or pull requests

3 participants