Skip to content

Commit f593965

Browse files
committed
Eagerly read IAsyncEnumerable{object} instances before formatting
Fixes #4833
1 parent b0be780 commit f593965

20 files changed

+430
-22
lines changed

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

+3
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,7 @@ public MvcOptions() { }
874874
public Microsoft.AspNetCore.Mvc.Filters.FilterCollection Filters { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
875875
public Microsoft.AspNetCore.Mvc.Formatters.FormatterMappings FormatterMappings { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
876876
public Microsoft.AspNetCore.Mvc.Formatters.FormatterCollection<Microsoft.AspNetCore.Mvc.Formatters.IInputFormatter> InputFormatters { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
877+
public int MaxIAsyncEnumerableBufferLimit { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
877878
public int MaxModelBindingCollectionSize { get { throw null; } set { } }
878879
public int MaxModelBindingRecursionDepth { get { throw null; } set { } }
879880
public int MaxModelValidationErrors { get { throw null; } set { } }
@@ -2084,7 +2085,9 @@ public MvcCompatibilityOptions() { }
20842085
}
20852086
public partial class ObjectResultExecutor : Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultExecutor<Microsoft.AspNetCore.Mvc.ObjectResult>
20862087
{
2088+
[System.ObsoleteAttribute("This constructor is obsolete and will be removed in a future release.")]
20872089
public ObjectResultExecutor(Microsoft.AspNetCore.Mvc.Infrastructure.OutputFormatterSelector formatterSelector, Microsoft.AspNetCore.Mvc.Infrastructure.IHttpResponseStreamWriterFactory writerFactory, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) { }
2090+
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) { }
20882091
protected Microsoft.AspNetCore.Mvc.Infrastructure.OutputFormatterSelector FormatterSelector { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
20892092
protected Microsoft.Extensions.Logging.ILogger Logger { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
20902093
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/ObjectResultExecutor.cs

+98-1
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,19 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5+
using System.Collections.Concurrent;
56
using System.Collections.Generic;
67
using System.Diagnostics;
78
using System.IO;
9+
using System.Reflection;
810
using System.Text;
911
using System.Threading.Tasks;
1012
using Microsoft.AspNetCore.Http;
13+
using Microsoft.AspNetCore.Mvc.Core;
1114
using Microsoft.AspNetCore.Mvc.Formatters;
15+
using Microsoft.Extensions.Internal;
1216
using Microsoft.Extensions.Logging;
17+
using Microsoft.Extensions.Options;
1318

1419
namespace Microsoft.AspNetCore.Mvc.Infrastructure
1520
{
@@ -18,16 +23,43 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
1823
/// </summary>
1924
public class ObjectResultExecutor : IActionResultExecutor<ObjectResult>
2025
{
26+
private delegate Task<object> ReadAsyncEnumerableDelegate(object value);
27+
28+
private readonly MethodInfo Converter = typeof(ObjectResultExecutor).GetMethod(
29+
nameof(ReadAsyncEnumerable),
30+
BindingFlags.NonPublic | BindingFlags.Instance);
31+
32+
private readonly ConcurrentDictionary<Type, ReadAsyncEnumerableDelegate> _asyncEnumerableConverters =
33+
new ConcurrentDictionary<Type, ReadAsyncEnumerableDelegate>();
34+
private readonly MvcOptions _mvcOptions;
35+
2136
/// <summary>
2237
/// Creates a new <see cref="ObjectResultExecutor"/>.
2338
/// </summary>
2439
/// <param name="formatterSelector">The <see cref="OutputFormatterSelector"/>.</param>
2540
/// <param name="writerFactory">The <see cref="IHttpResponseStreamWriterFactory"/>.</param>
2641
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
42+
[Obsolete("This constructor is obsolete and will be removed in a future release.")]
2743
public ObjectResultExecutor(
2844
OutputFormatterSelector formatterSelector,
2945
IHttpResponseStreamWriterFactory writerFactory,
3046
ILoggerFactory loggerFactory)
47+
: this(formatterSelector, writerFactory, loggerFactory, mvcOptions: null)
48+
{
49+
}
50+
51+
/// <summary>
52+
/// Creates a new <see cref="ObjectResultExecutor"/>.
53+
/// </summary>
54+
/// <param name="formatterSelector">The <see cref="OutputFormatterSelector"/>.</param>
55+
/// <param name="writerFactory">The <see cref="IHttpResponseStreamWriterFactory"/>.</param>
56+
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
57+
/// <param name="mvcOptions">Accessor to <see cref="MvcOptions"/>.</param>
58+
public ObjectResultExecutor(
59+
OutputFormatterSelector formatterSelector,
60+
IHttpResponseStreamWriterFactory writerFactory,
61+
ILoggerFactory loggerFactory,
62+
IOptions<MvcOptions> mvcOptions)
3163
{
3264
if (formatterSelector == null)
3365
{
@@ -47,6 +79,7 @@ public ObjectResultExecutor(
4779
FormatterSelector = formatterSelector;
4880
WriterFactory = writerFactory.CreateWriter;
4981
Logger = loggerFactory.CreateLogger<ObjectResultExecutor>();
82+
_mvcOptions = mvcOptions?.Value ?? throw new ArgumentNullException(nameof(mvcOptions));
5083
}
5184

5285
/// <summary>
@@ -87,16 +120,35 @@ public virtual Task ExecuteAsync(ActionContext context, ObjectResult result)
87120
InferContentTypes(context, result);
88121

89122
var objectType = result.DeclaredType;
123+
90124
if (objectType == null || objectType == typeof(object))
91125
{
92126
objectType = result.Value?.GetType();
93127
}
94128

129+
var value = result.Value;
130+
131+
if (value is IAsyncEnumerable<object> asyncEnumerable)
132+
{
133+
return ExecuteAsyncEnumerable(context, result, asyncEnumerable);
134+
}
135+
136+
return ExecuteAsyncCore(context, result, objectType, value);
137+
}
138+
139+
private async Task ExecuteAsyncEnumerable(ActionContext context, ObjectResult result, IAsyncEnumerable<object> asyncEnumerable)
140+
{
141+
var enumerated = await EnumerateAsyncEnumerable(asyncEnumerable);
142+
await ExecuteAsyncCore(context, result, enumerated.GetType(), enumerated);
143+
}
144+
145+
private Task ExecuteAsyncCore(ActionContext context, ObjectResult result, Type objectType, object value)
146+
{
95147
var formatterContext = new OutputFormatterWriteContext(
96148
context.HttpContext,
97149
WriterFactory,
98150
objectType,
99-
result.Value);
151+
value);
100152

101153
var selectedFormatter = FormatterSelector.SelectFormatter(
102154
formatterContext,
@@ -138,5 +190,50 @@ private static void InferContentTypes(ActionContext context, ObjectResult result
138190
result.ContentTypes.Add("application/problem+xml");
139191
}
140192
}
193+
194+
private Task<object> EnumerateAsyncEnumerable(IAsyncEnumerable<object> value)
195+
{
196+
var type = value.GetType();
197+
if (!_asyncEnumerableConverters.TryGetValue(type, out var result))
198+
{
199+
var enumerableType = ClosedGenericMatcher.ExtractGenericInterface(type, typeof(IAsyncEnumerable<>));
200+
result = null;
201+
if (enumerableType != null)
202+
{
203+
var enumeratedObjectType = enumerableType.GetGenericArguments()[0];
204+
205+
var converter = (ReadAsyncEnumerableDelegate)Converter
206+
.MakeGenericMethod(enumeratedObjectType)
207+
.CreateDelegate(typeof(ReadAsyncEnumerableDelegate), this);
208+
209+
_asyncEnumerableConverters.TryAdd(type, converter);
210+
result = converter;
211+
}
212+
}
213+
214+
return result(value);
215+
}
216+
217+
private async Task<object> ReadAsyncEnumerable<T>(object value)
218+
{
219+
var asyncEnumerable = (IAsyncEnumerable<T>)value;
220+
var result = new List<T>();
221+
var count = 0;
222+
223+
await foreach (var item in asyncEnumerable)
224+
{
225+
if (count++ >= _mvcOptions.MaxIAsyncEnumerableBufferLimit)
226+
{
227+
throw new InvalidOperationException(Resources.FormatObjectResultExecutor_MaxEnumerationExceeded(
228+
nameof(ObjectResultExecutor),
229+
_mvcOptions.MaxIAsyncEnumerableBufferLimit,
230+
value.GetType()));
231+
}
232+
233+
result.Add(item);
234+
}
235+
236+
return result;
237+
}
141238
}
142239
}

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

+13
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,19 @@ public int MaxModelBindingRecursionDepth
359359
}
360360
}
361361

362+
/// <summary>
363+
/// Gets or sets the most number of entries of an <see cref="IAsyncEnumerable{T}"/> that
364+
/// that <see cref="ObjectResultExecutor"/> will buffer.
365+
/// <para>
366+
/// When <see cref="ObjectResult.Value" /> is an instance of <see cref="IAsyncEnumerable{T}"/>,
367+
/// <see cref="ObjectResultExecutor"/> will eagerly read the enumeration and add to a synchronous collection
368+
/// prior to invoking the selected formatter.
369+
/// This property determines the most number of entries that the executor is allowed to buffer.
370+
/// </para>
371+
/// </summary>
372+
/// <value>Defaults to <c>8192</c>.</value>
373+
public int MaxIAsyncEnumerableBufferLimit { get; set; } = 8192;
374+
362375
IEnumerator<ICompatibilitySwitch> IEnumerable<ICompatibilitySwitch>.GetEnumerator() => _switches.GetEnumerator();
363376

364377
IEnumerator IEnumerable.GetEnumerator() => _switches.GetEnumerator();

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

+14
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

+5-2
Original file line numberDiff line numberDiff line change
@@ -503,5 +503,8 @@
503503
</data>
504504
<data name="Property_MustBeInstanceOfType" xml:space="preserve">
505505
<value>Property '{0}.{1}' must be an instance of type '{2}'.</value>
506-
</data>
507-
</root>
506+
</data>
507+
<data name="ObjectResultExecutor_MaxEnumerationExceeded" xml:space="preserve">
508+
<value>{0} execeeded the maximum configured enumeration limit '{1}' when enumerating async enumerable type '{2}'.</value>
509+
</data>
510+
</root>

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,8 @@ private static IServiceProvider CreateServices(Mock<IOutputFormatter> formatter)
275275
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
276276
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
277277
new TestHttpResponseStreamWriterFactory(),
278-
NullLoggerFactory.Instance));
278+
NullLoggerFactory.Instance,
279+
Options.Create(new MvcOptions())));
279280

280281
return services.BuildServiceProvider();
281282
}

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ private static IServiceProvider CreateServices(Mock<IOutputFormatter> formatter)
183183
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
184184
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
185185
new TestHttpResponseStreamWriterFactory(),
186-
NullLoggerFactory.Instance));
186+
NullLoggerFactory.Instance,
187+
Options.Create(new MvcOptions())));
187188

188189
return services.BuildServiceProvider();
189190
}

src/Mvc/Mvc.Core/test/AcceptedResultTests.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ private static IServiceProvider CreateServices(Mock<IOutputFormatter> formatter)
139139
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
140140
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
141141
new TestHttpResponseStreamWriterFactory(),
142-
NullLoggerFactory.Instance));
142+
NullLoggerFactory.Instance,
143+
Options.Create(new MvcOptions())));
143144

144145
return services.BuildServiceProvider();
145146
}

src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ private static IServiceProvider CreateServices()
9797
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
9898
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
9999
new TestHttpResponseStreamWriterFactory(),
100-
NullLoggerFactory.Instance));
100+
NullLoggerFactory.Instance,
101+
Options.Create(new MvcOptions())));
101102

102103
return services.BuildServiceProvider();
103104
}

src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ private static IServiceProvider CreateServices()
110110
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
111111
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
112112
new TestHttpResponseStreamWriterFactory(),
113-
NullLoggerFactory.Instance));
113+
NullLoggerFactory.Instance,
114+
Options.Create(new MvcOptions())));
114115

115116
return services.BuildServiceProvider();
116117
}

src/Mvc/Mvc.Core/test/CreatedResultTests.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ private static IServiceProvider CreateServices()
9898
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
9999
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
100100
new TestHttpResponseStreamWriterFactory(),
101-
NullLoggerFactory.Instance));
101+
NullLoggerFactory.Instance,
102+
Options.Create(new MvcOptions())));
102103

103104
return services.BuildServiceProvider();
104105
}

src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ private static IServiceProvider CreateServices()
7575
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
7676
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
7777
new TestHttpResponseStreamWriterFactory(),
78-
NullLoggerFactory.Instance));
78+
NullLoggerFactory.Instance,
79+
Options.Create(new MvcOptions())));
7980

8081
return services.BuildServiceProvider();
8182
}

src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ private static IServiceProvider CreateServices()
7676
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
7777
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
7878
new TestHttpResponseStreamWriterFactory(),
79-
NullLoggerFactory.Instance));
79+
NullLoggerFactory.Instance,
80+
Options.Create(new MvcOptions())));
8081

8182
return services.BuildServiceProvider();
8283
}

src/Mvc/Mvc.Core/test/Infrastructure/ControllerActionInvokerTest.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1567,7 +1567,8 @@ private ControllerActionInvoker CreateInvoker(
15671567
services.AddSingleton<IActionResultExecutor<ObjectResult>>(new ObjectResultExecutor(
15681568
new DefaultOutputFormatterSelector(options, NullLoggerFactory.Instance),
15691569
new TestHttpResponseStreamWriterFactory(),
1570-
NullLoggerFactory.Instance));
1570+
NullLoggerFactory.Instance,
1571+
Options.Create(new MvcOptions())));
15711572

15721573
httpContext.Response.Body = new MemoryStream();
15731574
httpContext.RequestServices = services.BuildServiceProvider();

0 commit comments

Comments
 (0)