Skip to content

Commit

Permalink
fix: Minor tweaks to the Json.NET JsonEventFormatter for compliance
Browse files Browse the repository at this point in the history 
- Make "JSON media type detection" configurable, and default to */json and */*+json
- Default to JSON content type (including making it present in the serialized event)
- Make a "binary vs non-binary" distinction as the first part of serialization (instead of as a fallback)
- Deserialize string data values regardless of content type
- Prohibit non-string data values for non-JSON content types

Signed-off-by: Jon Skeet <jonskeet@google.com>
  • Loading branch information
@jskeet
jskeet committed Jan 19, 2022
1 parent 4f13307 commit 9cf04dc
Show file tree
Hide file tree
Showing 2 changed files with 302 additions and 51 deletions.
135 changes: 91 additions & 44 deletions src/CloudNative.CloudEvents.NewtonsoftJson/JsonEventFormatter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ namespace CloudNative.CloudEvents.NewtonsoftJson
/// nor "data_base64" property is populated in a structured mode message.
/// </description></item>
/// <item><description>
/// If the data content type is absent or has a media type of "application/json", the data is encoded as JSON.
/// If the data value is a byte array, it is serialized either directly as binary data
/// (for binary mode messages) or as base64 data (for structured mode messages).
/// </description></item>
/// <item><description>
/// Otherwise, if the data content type is absent or has a media type indicating JSON, the data is encoded as JSON.
/// If the data is already a <see cref="JToken"/>, that is serialized directly as JSON. Otherwise, the data
/// is converted using the <see cref="JsonSerializer"/> passed into the constructor, or a
/// default serializer.
Expand All @@ -40,29 +44,35 @@ namespace CloudNative.CloudEvents.NewtonsoftJson
/// the data is serialized as a string.
/// </description></item>
/// <item><description>
/// Otherwise, if the data value is a byte array, it is serialized either directly as binary data
/// (for binary mode messages) or as base64 data (for structured mode messages).
/// </description></item>
/// <item><description>
/// Otherwise, the encoding operation fails.
/// </description></item>
/// </list>
/// <para>
/// When decoding CloudEvent data, this implementation uses the following rules:
/// </para>
/// <para>
/// In a structured mode message, any data is either binary data within the "data_base64" property value,
/// or is a JSON token as the "data" property value. Binary data is represented as a byte array.
/// A JSON token is decoded as a string if is just a string value and the data content type is specified
/// and has a media type beginning with "text/". A JSON token representing the null value always
/// leads to a null data result. In any other situation, the JSON token is preserved as a <see cref="JToken"/>
/// that can be used for further deserialization (e.g. to a specific CLR type). This behavior can be modified
/// by overriding <see cref="DecodeStructuredModeDataBase64Property(JToken, CloudEvent)"/> and
/// <see cref="DecodeStructuredModeDataProperty(JToken, CloudEvent)"/>.
/// When decoding structured mode CloudEvent data, this implementation uses the following rules,
/// which can be modified by overriding <see cref="DecodeStructuredModeDataBase64Property(JToken, CloudEvent)"/>
/// and <see cref="DecodeStructuredModeDataProperty(JToken, CloudEvent)"/>.
/// </para>
/// <list type="bullet">
/// <item><description>
/// If the "data_base64" property is present, its value is decoded as a byte array.
/// </description></item>
/// <item><description>
/// If the "data" property is present (and non-null) and the content type is absent or indicates a JSON media type,
/// the JSON token present in the property is preserved as a <see cref="JToken"/> that can be used for further
/// deserialization (e.g. to a specific CLR type).
/// </description></item>
/// <item><description>
/// If the "data" property has a string value and a non-JSON content type has been specified, the data is
/// deserialized as a string.
/// </description></item>
/// <item><description>
/// If the "data" property has a non-null, non-string value and a non-JSON content type has been specified,
/// the deserialization operation fails.
/// </description></item>
/// </list>
/// <para>
/// In a binary mode message, the data is parsed based on the content type of the message. When the content
/// type is absent or has a media type of "application/json", the data is parsed as JSON, with the result as
/// type is absent or has a JSON media type, the data is parsed as JSON, with the result as
/// a <see cref="JToken"/> (or null if the data is empty). When the content type has a media type beginning
/// with "text/", the data is parsed as a string. In all other cases, the data is left as a byte array.
/// This behavior can be specialized by overriding <see cref="DecodeBinaryModeEventData(ReadOnlyMemory{byte}, CloudEvent)"/>.
Expand Down Expand Up @@ -296,6 +306,9 @@ private void PopulateDataFromStructuredEvent(CloudEvent cloudEvent, JObject jObj
}
else
{
// If no content type has been specified, default to application/json
cloudEvent.DataContentType ??= JsonMediaType;

// We know that dataToken must be non-null here, due to the above conditions.
DecodeStructuredModeDataProperty(dataToken!, cloudEvent);
}
Expand Down Expand Up @@ -334,8 +347,9 @@ protected virtual void DecodeStructuredModeDataBase64Property(JToken dataBase64T
/// </summary>
/// <remarks>
/// <para>
/// This implementation converts JSON string tokens to strings when the content type suggests
/// that's appropriate, but otherwise returns the token directly.
/// This implementation will populate the Data property with the verbatim <see cref="JToken"/> if
/// the content type is deemed to be JSON according to <see cref="IsJsonMediaType(string)"/>. Otherwise,
/// it validates that the token is a string, and the Data property is populated with that string.
/// </para>
/// <para>
/// Override this method to provide more specialized conversions.
Expand All @@ -345,12 +359,24 @@ protected virtual void DecodeStructuredModeDataBase64Property(JToken dataBase64T
/// not have a null token type.</param>
/// <param name="cloudEvent">The event being decoded. This should not be modified except to
/// populate the <see cref="CloudEvent.Data"/> property, but may be used to provide extra
/// information such as the data content type. Will not be null.</param>
/// information such as the data content type. Will not be null, and the <see cref="CloudEvent.DataContentType"/>
/// property will be non-null.</param>
/// <returns>The data to populate in the <see cref="CloudEvent.Data"/> property.</returns>
protected virtual void DecodeStructuredModeDataProperty(JToken dataToken, CloudEvent cloudEvent) =>
cloudEvent.Data = dataToken.Type == JTokenType.String && cloudEvent.DataContentType?.StartsWith("text/") == true
? (string?) dataToken
: (object) dataToken; // Deliberately cast to object to avoid any implicit conversions
protected virtual void DecodeStructuredModeDataProperty(JToken dataToken, CloudEvent cloudEvent)
{
if (IsJsonMediaType(cloudEvent.DataContentType!))
{
cloudEvent.Data = dataToken;
}
else
{
if (dataToken.Type != JTokenType.String)
{
throw new ArgumentException("CloudEvents with a non-JSON datacontenttype can only have string data values.");
}
cloudEvent.Data = (string?) dataToken;
}
}

/// <inheritdoc />
public override ReadOnlyMemory<byte> EncodeStructuredModeMessage(CloudEvent cloudEvent, out ContentType contentType)
Expand Down Expand Up @@ -420,6 +446,11 @@ private void WriteCloudEventForBatchOrStructuredMode(JsonWriter writer, CloudEve

if (cloudEvent.Data is object)
{
if (cloudEvent.DataContentType is null)
{
writer.WritePropertyName(cloudEvent.SpecVersion.DataContentTypeAttribute.Name);
writer.WriteValue(JsonMediaType);
}
EncodeStructuredModeData(cloudEvent, writer);
}
writer.WriteEndObject();
Expand All @@ -440,26 +471,31 @@ private void WriteCloudEventForBatchOrStructuredMode(JsonWriter writer, CloudEve
/// <param name="writer"/>The writer to serialize the data to. Will not be null.</param>
protected virtual void EncodeStructuredModeData(CloudEvent cloudEvent, JsonWriter writer)
{
ContentType dataContentType = new ContentType(cloudEvent.DataContentType ?? JsonMediaType);
if (dataContentType.MediaType == JsonMediaType)
{
writer.WritePropertyName(DataPropertyName);
Serializer.Serialize(writer, cloudEvent.Data);
}
else if (cloudEvent.Data is string text && dataContentType.MediaType.StartsWith("text/"))
{
writer.WritePropertyName(DataPropertyName);
writer.WriteValue(text);
}
else if (cloudEvent.Data is byte[] binary)
// Binary data is encoded using the data_base64 property, regardless of content type.
// TODO: Support other forms of binary data, e.g. ReadOnlyMemory<byte>
if (cloudEvent.Data is byte[] binary)
{
writer.WritePropertyName(DataBase64PropertyName);
writer.WriteValue(Convert.ToBase64String(binary));
}
else
{
// We assume CloudEvent.Data is not null due to the way this is called.
throw new ArgumentException($"{nameof(JsonEventFormatter)} cannot serialize data of type {cloudEvent.Data!.GetType()} with content type '{cloudEvent.DataContentType}'");
ContentType dataContentType = new ContentType(cloudEvent.DataContentType ?? JsonMediaType);
if (IsJsonMediaType(dataContentType.MediaType))
{
writer.WritePropertyName(DataPropertyName);
Serializer.Serialize(writer, cloudEvent.Data);
}
else if (cloudEvent.Data is string text && dataContentType.MediaType.StartsWith("text/"))
{
writer.WritePropertyName(DataPropertyName);
writer.WriteValue(text);
}
else
{
// We assume CloudEvent.Data is not null due to the way this is called.
throw new ArgumentException($"{nameof(JsonEventFormatter)} cannot serialize data of type {cloudEvent.Data!.GetType()} with content type '{cloudEvent.DataContentType}'");
}
}
}

Expand All @@ -472,8 +508,14 @@ public override ReadOnlyMemory<byte> EncodeBinaryModeEventData(CloudEvent cloudE
{
return Array.Empty<byte>();
}
// Binary data is left alone, regardless of the content type.
// TODO: Support other forms of binary data, e.g. ReadOnlyMemory<byte>
if (cloudEvent.Data is byte[] bytes)
{
return bytes;
}
ContentType contentType = new ContentType(cloudEvent.DataContentType ?? JsonMediaType);
if (contentType.MediaType == JsonMediaType)
if (IsJsonMediaType(contentType.MediaType))
{
// TODO: Make this more efficient. We could write to a StreamWriter with a MemoryStream,
// but then we end up with a BOM in most cases, which I suspect we don't want.
Expand All @@ -487,10 +529,6 @@ public override ReadOnlyMemory<byte> EncodeBinaryModeEventData(CloudEvent cloudE
{
return MimeUtilities.GetEncoding(contentType).GetBytes(text);
}
if (cloudEvent.Data is byte[] bytes)
{
return bytes;
}
throw new ArgumentException($"{nameof(JsonEventFormatter)} cannot serialize data of type {cloudEvent.Data.GetType()} with content type '{cloudEvent.DataContentType}'");
}

Expand All @@ -503,7 +541,7 @@ public override void DecodeBinaryModeEventData(ReadOnlyMemory<byte> body, CloudE

Encoding encoding = MimeUtilities.GetEncoding(contentType);

if (contentType.MediaType == JsonMediaType)
if (IsJsonMediaType(contentType.MediaType))
{
if (body.Length > 0)
{
Expand Down Expand Up @@ -547,6 +585,15 @@ protected virtual JsonReader CreateJsonReader(Stream stream, Encoding? encoding)
{
DateParseHandling = DateParseHandling.None
};

/// <summary>
/// Determines whether the given media type should be handled as JSON.
/// The default implementation treats anything ending with "/json" or "+json"
/// as JSON.
/// </summary>
/// <param name="mediaType">The media type to check for JSON. Will not be null.</param>
/// <returns>Whether or not <paramref name="mediaType"/> indicates JSON data.</returns>
protected virtual bool IsJsonMediaType(string mediaType) => mediaType.EndsWith("/json") || mediaType.EndsWith("+json");
}

/// <summary>
Expand Down
Loading

0 comments on commit 9cf04dc

Please sign in to comment.