From 52a7c112e8c0369b4b1626d750fb3d5b43db65bd Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Mon, 11 Jul 2016 15:11:31 -0700 Subject: [PATCH] [Fixes #4876] ContentResult forcing chunked encoding --- .../ContentResult.cs | 44 +------ .../MvcCoreServiceCollectionExtensions.cs | 1 + .../Internal/ContentResultExecutor.cs | 72 ++++++++++ .../ContentResultTest.cs | 123 +++++++++++++++++- .../Internal/ControllerActionInvokerTest.cs | 14 +- 5 files changed, 207 insertions(+), 47 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Internal/ContentResultExecutor.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ContentResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/ContentResult.cs index 55315f695c..9051496640 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ContentResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ContentResult.cs @@ -2,24 +2,14 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Text; using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Mvc { public class ContentResult : ActionResult { - private readonly string DefaultContentType = new MediaTypeHeaderValue("text/plain") - { - Encoding = Encoding.UTF8 - }.ToString(); - /// /// Gets or set the content representing the body of the response. /// @@ -42,38 +32,8 @@ namespace Microsoft.AspNetCore.Mvc throw new ArgumentNullException(nameof(context)); } - var loggerFactory = context.HttpContext.RequestServices.GetRequiredService(); - var logger = loggerFactory.CreateLogger(); - - var response = context.HttpContext.Response; - - string resolvedContentType; - Encoding resolvedContentTypeEncoding; - ResponseContentTypeHelper.ResolveContentTypeAndEncoding( - ContentType, - response.ContentType, - DefaultContentType, - out resolvedContentType, - out resolvedContentTypeEncoding); - - response.ContentType = resolvedContentType; - - if (StatusCode != null) - { - response.StatusCode = StatusCode.Value; - } - - logger.ContentResultExecuting(resolvedContentType); - - if (Content != null) - { - var bufferingFeature = response.HttpContext.Features.Get(); - bufferingFeature?.DisableResponseBuffering(); - - return response.WriteAsync(Content, resolvedContentTypeEncoding); - } - - return TaskCache.CompletedTask; + var executor = context.HttpContext.RequestServices.GetRequiredService(); + return executor.ExecuteAsync(context, this); } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 961d8301b6..80f72cf352 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -215,6 +215,7 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); + services.TryAddSingleton(); // // Route Handlers diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ContentResultExecutor.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ContentResultExecutor.cs new file mode 100644 index 0000000000..c7f149f983 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ContentResultExecutor.cs @@ -0,0 +1,72 @@ +// 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.Text; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class ContentResultExecutor + { + private const string DefaultContentType = "text/plain; charset=utf-8"; + private readonly ILogger _logger; + private readonly IHttpResponseStreamWriterFactory _httpResponseStreamWriterFactory; + + public ContentResultExecutor(ILogger logger, IHttpResponseStreamWriterFactory httpResponseStreamWriterFactory) + { + _logger = logger; + _httpResponseStreamWriterFactory = httpResponseStreamWriterFactory; + } + + public async Task ExecuteAsync(ActionContext context, ContentResult result) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (result == null) + { + throw new ArgumentNullException(nameof(result)); + } + + var response = context.HttpContext.Response; + + string resolvedContentType; + Encoding resolvedContentTypeEncoding; + ResponseContentTypeHelper.ResolveContentTypeAndEncoding( + result.ContentType, + response.ContentType, + DefaultContentType, + out resolvedContentType, + out resolvedContentTypeEncoding); + + response.ContentType = resolvedContentType; + + if (result.StatusCode != null) + { + response.StatusCode = result.StatusCode.Value; + } + + _logger.ContentResultExecuting(resolvedContentType); + + if (result.Content != null) + { + response.ContentLength = resolvedContentTypeEncoding.GetByteCount(result.Content); + + using (var textWriter = _httpResponseStreamWriterFactory.CreateWriter(response.Body, resolvedContentTypeEncoding)) + { + await textWriter.WriteAsync(result.Content); + + // Flushing the HttpResponseStreamWriter does not flush the underlying stream. This just flushes + // the buffered text in the writer. + // We do this rather than letting dispose handle it because dispose would call Write and we want + // to call WriteAsync. + await textWriter.FlushAsync(); + } + } + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ContentResultTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ContentResultTest.cs index 09a29560f5..0525590994 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ContentResultTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ContentResultTest.cs @@ -1,14 +1,17 @@ // 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.Buffers; using System.IO; using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.TestCommon; using Microsoft.AspNetCore.Mvc.ViewComponents; using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; @@ -20,6 +23,8 @@ namespace Microsoft.AspNetCore.Mvc { public class ContentResultTest { + private const int DefaultCharacterChunkSize = HttpResponseStreamWriter.DefaultBufferSize; + [Fact] public async Task ContentResult_Response_NullContent_SetsContentTypeAndEncoding() { @@ -137,6 +142,110 @@ namespace Microsoft.AspNetCore.Mvc var finalResponseContentType = httpContext.Response.ContentType; Assert.Equal(expectedContentType, finalResponseContentType); Assert.Equal(expectedContentData, memoryStream.ToArray()); + Assert.Equal(expectedContentData.Length, httpContext.Response.ContentLength); + } + + public static TheoryData ContentResult_WritesDataCorrectly_ForDifferentContentSizesData + { + get + { + // content, contentType + return new TheoryData + { + { string.Empty, "text/plain; charset=utf-8" }, + { new string('a', DefaultCharacterChunkSize), "text/plain; charset=utf-8" }, + { new string('a', DefaultCharacterChunkSize - 1), "text/plain; charset=utf-8" }, + { new string('a', DefaultCharacterChunkSize + 1), "text/plain; charset=utf-8" }, + { new string('a', DefaultCharacterChunkSize - 2), "text/plain; charset=utf-8" }, + { new string('a', DefaultCharacterChunkSize + 2), "text/plain; charset=utf-8" }, + { new string('a', DefaultCharacterChunkSize - 3), "text/plain; charset=utf-8" }, + { new string('a', DefaultCharacterChunkSize + 3), "text/plain; charset=utf-8" }, + { new string('a', DefaultCharacterChunkSize * 2), "text/plain; charset=utf-8" }, + { new string('a', DefaultCharacterChunkSize * 3), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 2) - 1), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 2) - 2), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 2) - 3), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 2) + 1), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 2) + 2), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 2) + 3), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 3) - 1), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 3) - 2), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 3) - 3), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 3) + 1), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 3) + 2), "text/plain; charset=utf-8" }, + { new string('a', (DefaultCharacterChunkSize * 3) + 3), "text/plain; charset=utf-8" }, + + { new string('色', DefaultCharacterChunkSize), "text/plain; charset=utf-16" }, + { new string('色', DefaultCharacterChunkSize - 1), "text/plain; charset=utf-16" }, + { new string('色', DefaultCharacterChunkSize + 1), "text/plain; charset=utf-16" }, + { new string('色', DefaultCharacterChunkSize - 2), "text/plain; charset=utf-16" }, + { new string('色', DefaultCharacterChunkSize + 2), "text/plain; charset=utf-16" }, + { new string('色', DefaultCharacterChunkSize - 3), "text/plain; charset=utf-16" }, + { new string('色', DefaultCharacterChunkSize + 3), "text/plain; charset=utf-16" }, + { new string('色', DefaultCharacterChunkSize * 2), "text/plain; charset=utf-16" }, + { new string('色', DefaultCharacterChunkSize * 3), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 2) - 1), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 2) - 2), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 2) - 3), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 2) + 1), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 2) + 2), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 2) + 3), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 3) - 1), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 3) - 2), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 3) - 3), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 3) + 1), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 3) + 2), "text/plain; charset=utf-16" }, + { new string('色', (DefaultCharacterChunkSize * 3) + 3), "text/plain; charset=utf-16" }, + + { new string('色', DefaultCharacterChunkSize), "text/plain; charset=utf-32" }, + { new string('色', DefaultCharacterChunkSize - 1), "text/plain; charset=utf-32" }, + { new string('色', DefaultCharacterChunkSize + 1), "text/plain; charset=utf-32" }, + { new string('色', DefaultCharacterChunkSize - 2), "text/plain; charset=utf-32" }, + { new string('色', DefaultCharacterChunkSize + 2), "text/plain; charset=utf-32" }, + { new string('色', DefaultCharacterChunkSize - 3), "text/plain; charset=utf-32" }, + { new string('色', DefaultCharacterChunkSize + 3), "text/plain; charset=utf-32" }, + { new string('色', DefaultCharacterChunkSize * 2), "text/plain; charset=utf-32" }, + { new string('色', DefaultCharacterChunkSize * 3), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 2) - 1), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 2) - 2), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 2) - 3), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 2) + 1), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 2) + 2), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 2) + 3), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 3) - 1), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 3) - 2), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 3) - 3), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 3) + 1), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 3) + 2), "text/plain; charset=utf-32" }, + { new string('色', (DefaultCharacterChunkSize * 3) + 3), "text/plain; charset=utf-32" }, + }; + } + } + + [Theory] + [MemberData(nameof(ContentResult_WritesDataCorrectly_ForDifferentContentSizesData))] + public async Task ContentResult_WritesDataCorrectly_ForDifferentContentSizes(string content, string contentType) + { + // Arrange + var contentResult = new ContentResult + { + Content = content, + ContentType = contentType + }; + var httpContext = GetHttpContext(); + var memoryStream = new MemoryStream(); + httpContext.Response.Body = memoryStream; + var actionContext = GetActionContext(httpContext); + var encoding = MediaTypeHeaderValue.Parse(contentType).Encoding; + + // Act + await contentResult.ExecuteResultAsync(actionContext); + + // Assert + memoryStream.Seek(0, SeekOrigin.Begin); + var streamReader = new StreamReader(memoryStream, encoding); + var actualContent = await streamReader.ReadToEndAsync(); + Assert.Equal(content, actualContent); } private static ActionContext GetActionContext(HttpContext httpContext) @@ -151,8 +260,18 @@ namespace Microsoft.AspNetCore.Mvc private static IServiceCollection CreateServices(params ViewComponentDescriptor[] descriptors) { + // An array pool could return a buffer which is greater or equal to the size of the default character + // chunk size. Since the tests here depend on a specifc character buffer size to test boundary conditions, + // make sure to only return a buffer of that size. + var charArrayPool = new Mock>(); + charArrayPool + .Setup(ap => ap.Rent(DefaultCharacterChunkSize)) + .Returns(new char[DefaultCharacterChunkSize]); + var services = new ServiceCollection(); - services.AddSingleton(NullLoggerFactory.Instance); + services.AddSingleton(new ContentResultExecutor( + new Logger(NullLoggerFactory.Instance), + new MemoryPoolHttpResponseStreamWriterFactory(ArrayPool.Shared, charArrayPool.Object))); return services; } @@ -166,4 +285,4 @@ namespace Microsoft.AspNetCore.Mvc return httpContext; } } -} \ No newline at end of file +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs index f54da94908..dce6b44fc1 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerTest.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.Buffers; using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics; @@ -2726,9 +2727,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal httpContext.SetupGet(c => c.Request).Returns(httpRequest); httpContext.SetupGet(c => c.Response).Returns(httpResponse); - httpContext - .Setup(o => o.RequestServices.GetService(typeof(ILoggerFactory))) - .Returns(NullLoggerFactory.Instance); httpResponse.Body = new MemoryStream(); @@ -2752,6 +2750,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal .SetupGet(o => o.Value) .Returns(options); + httpContext + .Setup(o => o.RequestServices.GetService(typeof(ILoggerFactory))) + .Returns(NullLoggerFactory.Instance); + httpContext .Setup(o => o.RequestServices.GetService(typeof(IOptions))) .Returns(optionsAccessor.Object); @@ -2765,6 +2767,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal new TestHttpResponseStreamWriterFactory(), NullLoggerFactory.Instance)); + httpContext + .Setup(o => o.RequestServices.GetService(typeof(ContentResultExecutor))) + .Returns(new ContentResultExecutor( + new Logger(NullLoggerFactory.Instance), + new MemoryPoolHttpResponseStreamWriterFactory(ArrayPool.Shared, ArrayPool.Shared))); + if (routeData == null) { routeData = new RouteData();