cloudevents / sdk-csharp

CSharp SDK for CloudEvents
Apache License 2.0
277 stars 79 forks source link

Make datacontenttype value check case-insensitive #282

Open batrived opened 6 months ago

batrived commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

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

captainsafia commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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.