Eagerly read IAsyncEnumerable{object} instances before formatting (#11118)

* Eagerly read IAsyncEnumerable{object} instances before formatting

Fixes #4833

Author: pranavkm

src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs

@@ -875,6 +875,7 @@ public MvcOptions() { }
         public Microsoft.AspNetCore.Mvc.Filters.FilterCollection Filters { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
         public Microsoft.AspNetCore.Mvc.Formatters.FormatterMappings FormatterMappings { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
         public Microsoft.AspNetCore.Mvc.Formatters.FormatterCollection<Microsoft.AspNetCore.Mvc.Formatters.IInputFormatter> InputFormatters { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
+        public int MaxIAsyncEnumerableBufferLimit { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public int MaxModelBindingCollectionSize { get { throw null; } set { } }
         public int MaxModelBindingRecursionDepth { get { throw null; } set { } }
         public int MaxModelValidationErrors { get { throw null; } set { } }
@@ -2086,7 +2087,9 @@ public MvcCompatibilityOptions() { }
     }
     public partial class ObjectResultExecutor : Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultExecutor<Microsoft.AspNetCore.Mvc.ObjectResult>
     {
+        [System.ObsoleteAttribute("This constructor is obsolete and will be removed in a future release.")]
         public ObjectResultExecutor(Microsoft.AspNetCore.Mvc.Infrastructure.OutputFormatterSelector formatterSelector, Microsoft.AspNetCore.Mvc.Infrastructure.IHttpResponseStreamWriterFactory writerFactory, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) { }
+        public ObjectResultExecutor(Microsoft.AspNetCore.Mvc.Infrastructure.OutputFormatterSelector formatterSelector, Microsoft.AspNetCore.Mvc.Infrastructure.IHttpResponseStreamWriterFactory writerFactory, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Mvc.MvcOptions> mvcOptions) { }
         protected Microsoft.AspNetCore.Mvc.Infrastructure.OutputFormatterSelector FormatterSelector { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
         protected Microsoft.Extensions.Logging.ILogger Logger { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
         protected System.Func<System.IO.Stream, System.Text.Encoding, System.IO.TextWriter> WriterFactory { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }

src/Mvc/Mvc.Core/src/Infrastructure/AsyncEnumerableReader.cs

@@ -0,0 +1,102 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections;
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Reflection;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Mvc.Core;
+using Microsoft.Extensions.Internal;
+
+#if JSONNET
+namespace Microsoft.AspNetCore.Mvc.NewtonsoftJson
+#else
+namespace Microsoft.AspNetCore.Mvc.Infrastructure
+#endif
+{
+    using ReaderFunc = Func<IAsyncEnumerable<object>, Task<ICollection>>;
+
+    /// <summary>
+    /// Type that reads an <see cref="IAsyncEnumerable{T}"/> instance into a
+    /// generic collection instance.
+    /// </summary>
+    /// <remarks>
+    /// This type is used to create a strongly typed synchronous <see cref="ICollection{T}"/> instance from
+    /// an <see cref="IAsyncEnumerable{T}"/>. An accurate <see cref="ICollection{T}"/> is required for XML formatters to
+    /// correctly serialize.
+    /// </remarks>
+    internal sealed class AsyncEnumerableReader
+    {
+        private readonly MethodInfo Converter = typeof(AsyncEnumerableReader).GetMethod(
+            nameof(ReadInternal),
+            BindingFlags.NonPublic | BindingFlags.Instance);
+
+        private readonly ConcurrentDictionary<Type, ReaderFunc> _asyncEnumerableConverters =
+            new ConcurrentDictionary<Type, ReaderFunc>();
+        private readonly MvcOptions _mvcOptions;
+
+        /// <summary>
+        /// Initializes a new instance of <see cref="AsyncEnumerableReader"/>.
+        /// </summary>
+        /// <param name="mvcOptions">Accessor to <see cref="MvcOptions"/>.</param>
+        public AsyncEnumerableReader(MvcOptions mvcOptions)
+        {
+            _mvcOptions = mvcOptions;
+        }
+
+        /// <summary>
+        /// Reads a <see cref="IAsyncEnumerable{T}"/> into an <see cref="ICollection{T}"/>.
+        /// </summary>
+        /// <param name="value">The <see cref="IAsyncEnumerable{T}"/> to read.</param>
+        /// <returns>The <see cref="ICollection"/>.</returns>
+        public Task<ICollection> ReadAsync(IAsyncEnumerable<object> value)
+        {
+            if (value == null)
+            {
+                throw new ArgumentNullException(nameof(value));
+            }
+
+            var type = value.GetType();
+            if (!_asyncEnumerableConverters.TryGetValue(type, out var result))
+            {
+                var enumerableType = ClosedGenericMatcher.ExtractGenericInterface(type, typeof(IAsyncEnumerable<>));
+                Debug.Assert(enumerableType != null);
+
+                var enumeratedObjectType = enumerableType.GetGenericArguments()[0];
+
+                var converter = (ReaderFunc)Converter
+                    .MakeGenericMethod(enumeratedObjectType)
+                    .CreateDelegate(typeof(ReaderFunc), this);
+
+                _asyncEnumerableConverters.TryAdd(type, converter);
+                result = converter;
+            }
+
+            return result(value);
+        }
+
+        private async Task<ICollection> ReadInternal<T>(IAsyncEnumerable<object> value)
+        {
+            var asyncEnumerable = (IAsyncEnumerable<T>)value;
+            var result = new List<T>();
+            var count = 0;
+
+            await foreach (var item in asyncEnumerable)
+            {
+                if (count++ >= _mvcOptions.MaxIAsyncEnumerableBufferLimit)
+                {
+                    throw new InvalidOperationException(Resources.FormatObjectResultExecutor_MaxEnumerationExceeded(
+                        nameof(AsyncEnumerableReader),
+                        value.GetType()));
+                }
+
+                result.Add(item);
+            }
+
+            return result;
+        }
+    }
+}

src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs

@@ -10,6 +10,7 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.Formatters;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Mvc.Infrastructure
 {
@@ -18,16 +19,35 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
     /// </summary>
     public class ObjectResultExecutor : IActionResultExecutor<ObjectResult>
     {
+        private readonly AsyncEnumerableReader _asyncEnumerableReader;
+
         /// <summary>
         /// Creates a new <see cref="ObjectResultExecutor"/>.
         /// </summary>
         /// <param name="formatterSelector">The <see cref="OutputFormatterSelector"/>.</param>
         /// <param name="writerFactory">The <see cref="IHttpResponseStreamWriterFactory"/>.</param>
         /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        [Obsolete("This constructor is obsolete and will be removed in a future release.")]
         public ObjectResultExecutor(
             OutputFormatterSelector formatterSelector,
             IHttpResponseStreamWriterFactory writerFactory,
             ILoggerFactory loggerFactory)
+            : this(formatterSelector, writerFactory, loggerFactory, mvcOptions: null)
+        {
+        }
+
+        /// <summary>
+        /// Creates a new <see cref="ObjectResultExecutor"/>.
+        /// </summary>
+        /// <param name="formatterSelector">The <see cref="OutputFormatterSelector"/>.</param>
+        /// <param name="writerFactory">The <see cref="IHttpResponseStreamWriterFactory"/>.</param>
+        /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
+        /// <param name="mvcOptions">Accessor to <see cref="MvcOptions"/>.</param>
+        public ObjectResultExecutor(
+            OutputFormatterSelector formatterSelector,
+            IHttpResponseStreamWriterFactory writerFactory,
+            ILoggerFactory loggerFactory,
+            IOptions<MvcOptions> mvcOptions)
         {
             if (formatterSelector == null)
             {
@@ -47,6 +67,8 @@ public ObjectResultExecutor(
             FormatterSelector = formatterSelector;
             WriterFactory = writerFactory.CreateWriter;
             Logger = loggerFactory.CreateLogger<ObjectResultExecutor>();
+            var options = mvcOptions?.Value ?? throw new ArgumentNullException(nameof(mvcOptions));
+            _asyncEnumerableReader = new AsyncEnumerableReader(options);
         }
 
         /// <summary>
@@ -87,16 +109,37 @@ public virtual Task ExecuteAsync(ActionContext context, ObjectResult result)
             InferContentTypes(context, result);
 
             var objectType = result.DeclaredType;
+
             if (objectType == null || objectType == typeof(object))
             {
                 objectType = result.Value?.GetType();
             }
 
+            var value = result.Value;
+
+            if (value is IAsyncEnumerable<object> asyncEnumerable)
+            {
+                return ExecuteAsyncEnumerable(context, result, asyncEnumerable);
+            }
+
+            return ExecuteAsyncCore(context, result, objectType, value);
+        }
+
+        private async Task ExecuteAsyncEnumerable(ActionContext context, ObjectResult result, IAsyncEnumerable<object> asyncEnumerable)
+        {
+            Log.BufferingAsyncEnumerable(Logger, asyncEnumerable);
+
+            var enumerated = await _asyncEnumerableReader.ReadAsync(asyncEnumerable);
+            await ExecuteAsyncCore(context, result, enumerated.GetType(), enumerated);
+        }
+
+        private Task ExecuteAsyncCore(ActionContext context, ObjectResult result, Type objectType, object value)
+        {
             var formatterContext = new OutputFormatterWriteContext(
                 context.HttpContext,
                 WriterFactory,
                 objectType,
-                result.Value);
+                value);
 
             var selectedFormatter = FormatterSelector.SelectFormatter(
                 formatterContext,
@@ -138,5 +181,21 @@ private static void InferContentTypes(ActionContext context, ObjectResult result
                 result.ContentTypes.Add("application/problem+xml");
             }
         }
+
+        private static class Log
+        {
+            private static readonly Action<ILogger, string, Exception> _bufferingAsyncEnumerable;
+
+            static Log()
+            {
+                _bufferingAsyncEnumerable = LoggerMessage.Define<string>(
+                   LogLevel.Debug,
+                   new EventId(1, "BufferingAsyncEnumerable"),
+                   "Buffering IAsyncEnumerable instance of type '{Type}'.");
+            }
+
+            public static void BufferingAsyncEnumerable(ILogger logger, IAsyncEnumerable<object> asyncEnumerable)
+                => _bufferingAsyncEnumerable(logger, asyncEnumerable.GetType().FullName, null);
+        }
     }
 }

src/Mvc/Mvc.Core/src/Infrastructure/SystemTextJsonResultExecutor.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Text;
 using System.Text.Json;
@@ -25,13 +26,16 @@ internal sealed class SystemTextJsonResultExecutor : IActionResultExecutor<JsonR
 
         private readonly JsonOptions _options;
         private readonly ILogger<SystemTextJsonResultExecutor> _logger;
+        private readonly AsyncEnumerableReader _asyncEnumerableReader;
 
         public SystemTextJsonResultExecutor(
             IOptions<JsonOptions> options,
-            ILogger<SystemTextJsonResultExecutor> logger)
+            ILogger<SystemTextJsonResultExecutor> logger,
+            IOptions<MvcOptions> mvcOptions)
         {
             _options = options.Value;
             _logger = logger;
+            _asyncEnumerableReader = new AsyncEnumerableReader(mvcOptions.Value);
         }
 
         public async Task ExecuteAsync(ActionContext context, JsonResult result)
@@ -70,8 +74,15 @@ public async Task ExecuteAsync(ActionContext context, JsonResult result)
             var writeStream = GetWriteStream(context.HttpContext, resolvedContentTypeEncoding);
             try
             {
-                var type = result.Value?.GetType() ?? typeof(object);
-                await JsonSerializer.WriteAsync(writeStream, result.Value, type, jsonSerializerOptions);
+                var value = result.Value;
+                if (value is IAsyncEnumerable<object> asyncEnumerable)
+                {
+                    Log.BufferingAsyncEnumerable(_logger, asyncEnumerable);
+                    value = await _asyncEnumerableReader.ReadAsync(asyncEnumerable);
+                }
+
+                var type = value?.GetType() ?? typeof(object);
+                await JsonSerializer.WriteAsync(writeStream, value, type, jsonSerializerOptions);
                 await writeStream.FlushAsync();
             }
             finally
@@ -123,11 +134,19 @@ private static class Log
                 new EventId(1, "JsonResultExecuting"),
                 "Executing JsonResult, writing value of type '{Type}'.");
 
+            private static readonly Action<ILogger, string, Exception> _bufferingAsyncEnumerable = LoggerMessage.Define<string>(
+               LogLevel.Debug,
+               new EventId(2, "BufferingAsyncEnumerable"),
+               "Buffering IAsyncEnumerable instance of type '{Type}'.");
+
             public static void JsonResultExecuting(ILogger logger, object value)
             {
                 var type = value == null ? "null" : value.GetType().FullName;
                 _jsonResultExecuting(logger, type, null);
             }
+
+            public static void BufferingAsyncEnumerable(ILogger logger, IAsyncEnumerable<object> asyncEnumerable)
+                => _bufferingAsyncEnumerable(logger, asyncEnumerable.GetType().FullName, null);
         }
     }
 }

src/Mvc/Mvc.Core/src/MvcOptions.cs

@@ -359,6 +359,19 @@ public int MaxModelBindingRecursionDepth
             }
         }
 
+        /// <summary>
+        /// Gets or sets the most number of entries of an <see cref="IAsyncEnumerable{T}"/> that
+        /// that <see cref="ObjectResultExecutor"/> will buffer.
+        /// <para>
+        /// When <see cref="ObjectResult.Value" /> is an instance of <see cref="IAsyncEnumerable{T}"/>,
+        /// <see cref="ObjectResultExecutor"/> will eagerly read the enumeration and add to a synchronous collection
+        /// prior to invoking the selected formatter.
+        /// This property determines the most number of entries that the executor is allowed to buffer.
+        /// </para>
+        /// </summary>
+        /// <value>Defaults to <c>8192</c>.</value>
+        public int MaxIAsyncEnumerableBufferLimit { get; set; } = 8192;
+
         IEnumerator<ICompatibilitySwitch> IEnumerable<ICompatibilitySwitch>.GetEnumerator() => _switches.GetEnumerator();
 
         IEnumerator IEnumerable.GetEnumerator() => _switches.GetEnumerator();

src/Mvc/Mvc.Core/src/Properties/Resources.Designer.cs

@@ -503,5 +503,8 @@
   </data>
   <data name="Property_MustBeInstanceOfType" xml:space="preserve">
     <value>Property '{0}.{1}' must be an instance of type '{2}'.</value>
-  </data>  
+  </data>
+  <data name="ObjectResultExecutor_MaxEnumerationExceeded" xml:space="preserve">
+    <value>'{0}' reached the configured maximum size of the buffer when enumerating a value of type '{1}'. This limit is in place to prevent infinite streams of 'IAsyncEnumerable&lt;&gt;' from continuing indefinitely. If this is not a programming mistake, consider ways to reduce the collection size, or consider manually converting '{1}' into a list rather than increasing the limit.</value>
+  </data>
 </root>

src/Mvc/Mvc.Core/src/Resources.resx

@@ -275,7 +275,8 @@ private static IServiceProvider CreateServices(Mock<IOutputFormatter> formatter)
             services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
                 new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
                 new TestHttpResponseStreamWriterFactory(),
-                NullLoggerFactory.Instance));
+                NullLoggerFactory.Instance,
+                options));
 
             return services.BuildServiceProvider();
         }

src/Mvc/Mvc.Core/test/AcceptedAtActionResultTests.cs

@@ -183,7 +183,8 @@ private static IServiceProvider CreateServices(Mock<IOutputFormatter> formatter)
             services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
                 new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
                 new TestHttpResponseStreamWriterFactory(),
-                NullLoggerFactory.Instance));
+                NullLoggerFactory.Instance,
+                options));
 
             return services.BuildServiceProvider();
         }

src/Mvc/Mvc.Core/test/AcceptedAtRouteResultTests.cs

@@ -139,7 +139,8 @@ private static IServiceProvider CreateServices(Mock<IOutputFormatter> formatter)
             services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
                 new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
                 new TestHttpResponseStreamWriterFactory(),
-                NullLoggerFactory.Instance));
+                NullLoggerFactory.Instance,
+                options));
 
             return services.BuildServiceProvider();
         }