Support non-JSON text in cloud events middleware (#621)

Fixes: #592

This change teaches the cloud events middleware to JSON-decode non-JSON
content when it appears in the `data` attribute in a cloud event.

There are three cases for cloud events that matter:

- data is used, content is a JSON object, datacontentype is JSON
- data is used, content is a JSON string, dataconenttype != JSON
- data_base64 is used, content is base64 bytes

We weren't handling the second of this. If you submitted the content:

`"data": "hello, "\world!\""` with datacontenttype = text/plain

You would end up with `"hello, \"world!\""` in the request body instead
of `hello, "world!"`.

This is a very subtle case, and I'm trying to fix it before users couple
their code to the wrong behavior. Since this is technically a breaking
change, I've added a opt-out so you can restore the buggy behavior.

src/Dapr.AspNetCore/CloudEventsMiddleware.cs

@@ -8,21 +8,24 @@ namespace Dapr
     using System;
     using System.IO;
     using System.Net;
-    using System.Net.Http.Headers;
     using System.Text;
     using System.Text.Json;
     using System.Threading.Tasks;
     using Microsoft.AspNetCore.Http;
     using Microsoft.AspNetCore.WebUtilities;
+    using Microsoft.Extensions.Primitives;
+    using Microsoft.Net.Http.Headers;
 
     internal class CloudEventsMiddleware
     {
         private const string ContentType = "application/cloudevents+json";
         private readonly RequestDelegate next;
+        private readonly CloudEventsMiddlewareOptions options;
 
-        public CloudEventsMiddleware(RequestDelegate next)
+        public CloudEventsMiddleware(RequestDelegate next, CloudEventsMiddlewareOptions options)
         {
             this.next = next;
+            this.options = options;
         }
 
         public Task InvokeAsync(HttpContext httpContext)
@@ -82,11 +85,27 @@ namespace Dapr
             }
             else if (isDataSet)
             {
-                body = new MemoryStream();
-                await JsonSerializer.SerializeAsync<JsonElement>(body, data);
-                body.Seek(0L, SeekOrigin.Begin);
+                contentType = this.GetDataContentType(json, out var isJson);
 
-                contentType = this.GetDataContentType(json);
+                // If the value is anything other than a JSON string, treat it as JSON. Cloud Events requires
+                // non-JSON text to be enclosed in a JSON string.
+                isJson |= data.ValueKind != JsonValueKind.String;
+
+                body = new MemoryStream();
+                if (isJson || options.SuppressJsonDecodingOfTextPayloads)
+                {
+                    // Rehydrate body from JSON payload
+                    await JsonSerializer.SerializeAsync<JsonElement>(body, data);
+                }
+                else
+                {
+                    // Rehydrate body from contents of the string
+                    var text = data.GetString();
+                    using var writer = new HttpResponseStreamWriter(body, Encoding.UTF8);
+                    writer.Write(text);
+                }
+
+                body.Seek(0L, SeekOrigin.Begin);
             }
             else if (isBinaryDataSet)
             {
@@ -96,7 +115,7 @@ namespace Dapr
                 var decodedBody = binaryData.GetBytesFromBase64();
                 body = new MemoryStream(decodedBody);
                 body.Seek(0L, SeekOrigin.Begin);
-                contentType = this.GetDataContentType(json);
+                contentType = this.GetDataContentType(json, out _);
             }
             else
             {
@@ -121,20 +140,24 @@ namespace Dapr
             }
         }
 
-        private string GetDataContentType(JsonElement json)
+        private string GetDataContentType(JsonElement json, out bool isJson)
         {
             string contentType;
             if (json.TryGetProperty("datacontenttype", out var dataContentType) &&
-                    dataContentType.ValueKind == JsonValueKind.String)
+                dataContentType.ValueKind == JsonValueKind.String && 
+                MediaTypeHeaderValue.TryParse(dataContentType.GetString(), out var parsed))
             {
                 contentType = dataContentType.GetString();
+                isJson = 
+                    parsed.MediaType.Equals( "application/json", StringComparison.Ordinal) ||
+                    parsed.Suffix.EndsWith("+json", StringComparison.Ordinal);
 
                 // Since S.T.Json always outputs utf-8, we may need to normalize the data content type
                 // to remove any charset information. We generally just assume utf-8 everywhere, so omitting
                 // a charset is a safe bet.
-                if (contentType.Contains("charset") && MediaTypeHeaderValue.TryParse(contentType, out var parsed))
+                if (contentType.Contains("charset"))
                 {
-                    parsed.CharSet = null;
+                    parsed.Charset = StringSegment.Empty;
                     contentType = parsed.ToString();
                 }
             }
@@ -142,6 +165,7 @@ namespace Dapr
             {
                 // assume JSON is not specified.
                 contentType = "application/json";
+                isJson = true;
             }
 
             return contentType;
@@ -175,8 +199,8 @@ namespace Dapr
                 return false;
             }
 
-            charSet = parsed.CharSet ?? "UTF-8";
+            charSet = parsed.Charset.Length > 0 ? parsed.Charset.Value : "UTF-8";
             return true;
         }
     }
-}
+}

src/Dapr.AspNetCore/CloudEventsMiddlewareOptions.cs

@@ -0,0 +1,29 @@
+// ------------------------------------------------------------
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+// ------------------------------------------------------------
+
+namespace Dapr
+{
+    /// <summary>
+    /// Provides optional settings to the cloud events middleware.
+    /// </summary>
+    public class CloudEventsMiddlewareOptions
+    {
+        /// <summary>
+        /// Gets or sets a value that will determine whether non-JSON textual payloads are decoded.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// In the 1.0 release of the Dapr .NET SDK the cloud events middleware would not JSON-decode
+        /// a textual cloud events payload. A cloud event payload containing <c>text/plain</c> data 
+        /// of <c>"data": "Hello, \"world!\""</c> would result in a request body containing <c>"Hello, \"world!\""</c>
+        /// instead of the expected JSON-decoded value of <c>Hello, "world!"</c>.
+        /// </para>
+        /// <para>
+        /// Setting this property to <c>true</c> restores the previous invalid behavior for compatiblity.
+        /// </para>
+        /// </remarks>
+        public bool SuppressJsonDecodingOfTextPayloads { get; set; }
+    }
+}

src/Dapr.AspNetCore/DaprApplicationBuilderExtensions.cs

@@ -26,8 +26,25 @@ namespace Microsoft.AspNetCore.Builder
                 throw new ArgumentNullException(nameof(builder));
             }
 
-            builder.UseMiddleware<CloudEventsMiddleware>();
+            return UseCloudEvents(builder, new CloudEventsMiddlewareOptions());
+        }
+
+        /// <summary>
+        /// Adds the cloud events middleware to the middleware pipeline. The cloud events middleware will unwrap
+        /// requests that use the cloud events structured format, allowing the event payload to be read directly.
+        /// </summary>
+        /// <param name="builder">An <see cref="IApplicationBuilder" />.</param>
+        /// <param name="options">The <see cref="CloudEventsMiddlewareOptions" /> to configure optional settings.</param>
+        /// <returns>The <see cref="IApplicationBuilder" />.</returns>
+        public static IApplicationBuilder UseCloudEvents(this IApplicationBuilder builder, CloudEventsMiddlewareOptions options)
+        {
+            if (builder is null)
+            {
+                throw new ArgumentNullException(nameof(builder));
+            }
+
+            builder.UseMiddleware<CloudEventsMiddleware>(options);
             return builder;
         }
     }
-}
+}

test/Dapr.AspNetCore.IntegrationTest.App/DaprController.cs

@@ -6,10 +6,12 @@
 namespace Dapr.AspNetCore.IntegrationTest.App
 {
     using System;
+    using System.Text;
     using System.Threading.Tasks;
     using Dapr;
     using Dapr.Client;
     using Microsoft.AspNetCore.Mvc;
+    using Microsoft.AspNetCore.WebUtilities;
 
     [ApiController]
     public class DaprController : ControllerBase
@@ -27,6 +29,15 @@ namespace Dapr.AspNetCore.IntegrationTest.App
             return user; // echo back the user for testing
         }
 
+        [Topic("pubsub", "register-user-plaintext")]
+        [HttpPost("/register-user-plaintext")]
+        public async Task<ActionResult> RegisterUserPlaintext()
+        {
+            using var reader = new HttpRequestStreamReader(Request.Body, Encoding.UTF8);
+            var user = await reader.ReadToEndAsync();
+            return Content(user, "text/plain"); // echo back the user for testing
+        }
+
         [HttpPost("/controllerwithoutstateentry/{widget}")]
         public async Task AddOneWithoutStateEntry([FromServices] DaprClient state, [FromState("testStore")] Widget widget)
         {

test/Dapr.AspNetCore.IntegrationTest/CloudEventsIntegrationTest.cs

@@ -101,6 +101,34 @@ namespace Dapr.AspNetCore.IntegrationTest
             }
         }
 
+        [Fact]
+        public async Task CanSendStructuredCloudEvent_WithNonJsonContentType()
+        {
+            using (var factory = new AppWebApplicationFactory())
+            {
+                var httpClient = factory.CreateClient();
+
+                var request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/register-user-plaintext")
+                {
+                    Content = new StringContent(
+                    JsonSerializer.Serialize(
+                        new
+                        {
+                            data = "jimmy \"the cool guy\" smith",
+                            datacontenttype = "text/plain",
+                        }),
+                    Encoding.UTF8)
+                };
+                request.Content.Headers.ContentType = new MediaTypeHeaderValue("application/cloudevents+json");
+
+                var response = await httpClient.SendAsync(request);
+                response.EnsureSuccessStatusCode();
+
+                var user = await response.Content.ReadAsStringAsync();
+                user.Should().Be("jimmy \"the cool guy\" smith");
+            }
+        }
+
         // Yeah, I know, binary isn't a great term for this, it's what the cloudevents spec uses.
         // Basically this is here to test that an endpoint can handle requests with and without
         // an envelope.

test/Dapr.AspNetCore.IntegrationTest/SubscribeEndpointTest.cs

@@ -30,7 +30,7 @@ namespace Dapr.AspNetCore.IntegrationTest
                     var json = await JsonSerializer.DeserializeAsync<JsonElement>(stream);
 
                     json.ValueKind.Should().Be(JsonValueKind.Array);
-                    json.GetArrayLength().Should().Be(3);
+                    json.GetArrayLength().Should().Be(4);
                     var topics = new List<string>();
                     var routes = new List<string>();
                     foreach (var element in json.EnumerateArray())
@@ -42,10 +42,12 @@ namespace Dapr.AspNetCore.IntegrationTest
                     topics.Should().Contain("A");
                     topics.Should().Contain("B");
                     topics.Should().Contain("register-user");
+                    topics.Should().Contain("register-user-plaintext");
 
                     routes.Should().Contain("B");
                     routes.Should().Contain("topic-a");
                     routes.Should().Contain("register-user");
+                    routes.Should().Contain("register-user-plaintext");
                 }
             }
         }

test/Dapr.AspNetCore.Test/CloudEventsMiddlewareTest.cs

@@ -8,6 +8,7 @@ namespace Dapr.AspNetCore.Test
     using System.IO;
     using System.Net;
     using System.Text;
+    using System.Text.Json;
     using System.Threading.Tasks;
     using FluentAssertions;
     using Microsoft.AspNetCore.Builder;
@@ -76,6 +77,64 @@ namespace Dapr.AspNetCore.Test
             await pipeline.Invoke(context);
         }
 
+        [Fact]
+        public async Task InvokeAsync_ReplacesBodyNonJsonData()
+        {
+            // Our logic is based on the content-type, not the content.
+            // Since this is for text-plain content, we're going to encode it as a JSON string
+            // and store it in the data attribute - the middleware should JSON-decode it.
+            var input = "{ \"message\": \"hello, world\"}";
+            var expected = input;
+
+            var app = new ApplicationBuilder(null);
+            app.UseCloudEvents();
+
+            // Do verification in the scope of the middleware
+            app.Run(httpContext =>
+            {
+                httpContext.Request.ContentType.Should().Be("text/plain");
+                ReadBody(httpContext.Request.Body).Should().Be(expected);
+                return Task.CompletedTask;
+            });
+
+            
+            var pipeline = app.Build();
+
+            var context = new DefaultHttpContext();
+            context.Request.ContentType = "application/cloudevents+json";
+            context.Request.Body = MakeBody($"{{ \"datacontenttype\": \"text/plain\", \"data\": {JsonSerializer.Serialize(input)} }}");
+
+            await pipeline.Invoke(context);
+        }
+
+        [Fact]
+        public async Task InvokeAsync_ReplacesBodyNonJsonData_ExceptWhenSuppressed()
+        {
+            // Our logic is based on the content-type, not the content. This test tests the old bad behavior.
+            var input = "{ \"message\": \"hello, world\"}";
+            var expected = JsonSerializer.Serialize(input);
+
+            var app = new ApplicationBuilder(null);
+            app.UseCloudEvents(new CloudEventsMiddlewareOptions() { SuppressJsonDecodingOfTextPayloads = true, });
+
+            // Do verification in the scope of the middleware
+            app.Run(httpContext =>
+            {
+                httpContext.Request.ContentType.Should().Be("text/plain");
+                ReadBody(httpContext.Request.Body).Should().Be(expected);
+                return Task.CompletedTask;
+            });
+
+            
+            var pipeline = app.Build();
+
+            var context = new DefaultHttpContext();
+            context.Request.ContentType = "application/cloudevents+json";
+            context.Request.Body = MakeBody($"{{ \"datacontenttype\": \"text/plain\", \"data\": {JsonSerializer.Serialize(input)} }}");
+
+            await pipeline.Invoke(context);
+        }
+
         // This is a special case. S.T.Json will always output utf8, so we have to reinterpret the charset
         // of the datacontenttype.
         [Fact]