From a0e0df87dea4d0cff4ac80373afb22f01bd17a99 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Tue, 1 Sep 2015 09:33:29 -0700 Subject: [PATCH] [Fixes #3016] Disable response buffering in places where the content is already built/available --- .../ActionResults/ContentResult.cs | 8 +++- .../ActionResults/FileContentResult.cs | 4 ++ .../ActionResults/FileStreamResult.cs | 4 ++ .../Formatters/StreamOutputFormatter.cs | 4 ++ .../ActionResults/ContentResultTest.cs | 31 +++++++++++++ .../ActionResults/FileContentResultTest.cs | 44 +++++++++++++++++++ .../ActionResults/FileStreamResultTest.cs | 30 +++++++++++++ .../ActionResults/ObjectResultTests.cs | 16 ++++--- .../Formatters/StreamOutputFormatterTest.cs | 27 ++++++++++++ .../TestBufferingFeature.cs | 24 ++++++++++ 10 files changed, 183 insertions(+), 9 deletions(-) create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/TestBufferingFeature.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ContentResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ContentResult.cs index bd9c0904c3..92f0ce461d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/ContentResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/ContentResult.cs @@ -4,7 +4,7 @@ using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http; -using Microsoft.AspNet.Mvc.Actions; +using Microsoft.AspNet.Http.Features; using Microsoft.AspNet.Mvc.Internal; using Microsoft.Framework.Internal; using Microsoft.Net.Http.Headers; @@ -44,7 +44,7 @@ namespace Microsoft.AspNet.Mvc.ActionResults contentTypeHeader = contentTypeHeader.Copy(); contentTypeHeader.Encoding = Encoding.UTF8; } - + response.ContentType = contentTypeHeader?.ToString() ?? response.ContentType ?? DefaultContentType.ToString(); @@ -56,8 +56,12 @@ namespace Microsoft.AspNet.Mvc.ActionResults if (Content != null) { + var bufferingFeature = response.HttpContext.Features.Get(); + bufferingFeature?.DisableResponseBuffering(); + return response.WriteAsync(Content, contentTypeHeader?.Encoding ?? DefaultContentType.Encoding); } + return TaskCache.CompletedTask; } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileContentResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileContentResult.cs index 2e9199adad..c92c77911f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileContentResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileContentResult.cs @@ -5,6 +5,7 @@ using System; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Features; using Microsoft.Framework.Internal; using Microsoft.Net.Http.Headers; @@ -66,6 +67,9 @@ namespace Microsoft.AspNet.Mvc.ActionResults /// protected override Task WriteFileAsync(HttpResponse response, CancellationToken cancellation) { + var bufferingFeature = response.HttpContext.Features.Get(); + bufferingFeature?.DisableResponseBuffering(); + return response.Body.WriteAsync(FileContents, 0, FileContents.Length, cancellation); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileStreamResult.cs b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileStreamResult.cs index 5057329345..b093a90021 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileStreamResult.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionResults/FileStreamResult.cs @@ -6,6 +6,7 @@ using System.IO; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Features; using Microsoft.Framework.Internal; using Microsoft.Net.Http.Headers; @@ -74,6 +75,9 @@ namespace Microsoft.AspNet.Mvc.ActionResults using (FileStream) { + var bufferingFeature = response.HttpContext.Features.Get(); + bufferingFeature?.DisableResponseBuffering(); + await FileStream.CopyToAsync(outputStream, BufferSize, cancellation); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Formatters/StreamOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.Core/Formatters/StreamOutputFormatter.cs index a500943796..7e5b52a8b2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Formatters/StreamOutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Formatters/StreamOutputFormatter.cs @@ -3,6 +3,7 @@ using System.IO; using System.Threading.Tasks; +using Microsoft.AspNet.Http.Features; using Microsoft.Framework.Internal; using Microsoft.Net.Http.Headers; @@ -38,6 +39,9 @@ namespace Microsoft.AspNet.Mvc.Formatters response.ContentType = context.SelectedContentType.ToString(); } + var bufferingFeature = context.HttpContext.Features.Get(); + bufferingFeature?.DisableResponseBuffering(); + await valueAsStream.CopyToAsync(response.Body); } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ContentResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ContentResultTest.cs index bb02604ba3..39e0fa0522 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ContentResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ContentResultTest.cs @@ -1,10 +1,12 @@ // 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.IO; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Features; using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc.Actions; using Microsoft.AspNet.Routing; @@ -79,6 +81,35 @@ namespace Microsoft.AspNet.Mvc.ActionResults Assert.Equal("text/plain; charset=utf-7", httpContext.Response.ContentType); } + [Fact] + public async Task ContentResult_DisablesResponseBuffering_IfBufferingFeatureAvailable() + { + // Arrange + var data = "Test Content"; + var contentResult = new ContentResult + { + Content = data, + ContentType = new MediaTypeHeaderValue("text/plain") + { + Encoding = Encoding.ASCII + } + }; + var httpContext = new DefaultHttpContext(); + httpContext.Features.Set(new TestBufferingFeature()); + var memoryStream = new MemoryStream(); + httpContext.Response.Body = memoryStream; + var actionContext = GetActionContext(httpContext); + + // Act + await contentResult.ExecuteResultAsync(actionContext); + + // Assert + Assert.Equal("text/plain; charset=us-ascii", httpContext.Response.ContentType); + Assert.Equal(Encoding.ASCII.GetString(memoryStream.ToArray()), data); + var bufferingFeature = (TestBufferingFeature)httpContext.Features.Get(); + Assert.True(bufferingFeature.DisableResponseBufferingInvoked); + } + public static TheoryData ContentResultContentTypeData { get diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileContentResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileContentResultTest.cs index eb4af340bb..bab042a258 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileContentResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileContentResultTest.cs @@ -3,6 +3,7 @@ using System.IO; using System.Threading.Tasks; +using Microsoft.AspNet.Http.Features; using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc.Actions; using Microsoft.AspNet.Routing; @@ -71,5 +72,48 @@ namespace Microsoft.AspNet.Mvc.ActionResults Assert.Equal(buffer, outStream.ToArray()); Assert.Equal(expectedContentType, httpContext.Response.ContentType); } + + [Fact] + public async Task DisablesResponseBuffering_IfBufferingFeatureAvailable() + { + // Arrange + var expectedContentType = "text/foo; charset=us-ascii"; + var buffer = new byte[] { 1, 2, 3, 4, 5 }; + + var httpContext = new DefaultHttpContext(); + var bufferingFeature = new TestBufferingFeature(); + httpContext.Features.Set(bufferingFeature); + var outStream = new MemoryStream(); + httpContext.Response.Body = outStream; + + var context = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + + var result = new FileContentResult(buffer, MediaTypeHeaderValue.Parse(expectedContentType)); + + // Act + await result.ExecuteResultAsync(context); + + // Assert + Assert.Equal(buffer, outStream.ToArray()); + Assert.Equal(expectedContentType, httpContext.Response.ContentType); + Assert.True(bufferingFeature.DisableResponseBufferingInvoked); + } + + private class TestBufferingFeature : IHttpBufferingFeature + { + public bool DisableResponseBufferingInvoked { get; private set; } + + public bool DisableRequestBufferingInvoked { get; private set; } + + public void DisableRequestBuffering() + { + DisableRequestBufferingInvoked = true; + } + + public void DisableResponseBuffering() + { + DisableResponseBufferingInvoked = true; + } + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileStreamResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileStreamResultTest.cs index 77540f17ce..b9c6f007f0 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileStreamResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/FileStreamResultTest.cs @@ -3,8 +3,10 @@ using System.IO; using System.Linq; +using System.Text; using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNet.Http.Features; using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc.Actions; using Microsoft.AspNet.Routing; @@ -118,5 +120,33 @@ namespace Microsoft.AspNet.Mvc.ActionResults Assert.True(originalBytes.SequenceEqual(outBytes)); Assert.Equal(expectedContentType, httpContext.Response.ContentType); } + + [Fact] + public async Task DisablesResponseBuffering_IfBufferingFeatureAvailable() + { + // Arrange + var expectedContentType = "text/foo; charset=us-ascii"; + var expected = Encoding.ASCII.GetBytes("Test data"); + + var originalStream = new MemoryStream(expected); + + var httpContext = new DefaultHttpContext(); + var bufferingFeature = new TestBufferingFeature(); + httpContext.Features.Set(bufferingFeature); + var outStream = new MemoryStream(); + httpContext.Response.Body = outStream; + + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var result = new FileStreamResult(originalStream, MediaTypeHeaderValue.Parse(expectedContentType)); + + // Act + await result.ExecuteResultAsync(actionContext); + + // Assert + var outBytes = outStream.ToArray(); + Assert.Equal(expected, outBytes); + Assert.Equal(expectedContentType, httpContext.Response.ContentType); + Assert.True(bufferingFeature.DisableResponseBufferingInvoked); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectResultTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectResultTests.cs index 0fc326f668..c8a5675f7c 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectResultTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/ObjectResultTests.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Features; using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc.Actions; using Microsoft.AspNet.Mvc.Formatters; @@ -119,7 +120,7 @@ namespace Microsoft.AspNet.Mvc.ActionResults var httpResponse = new DefaultHttpContext().Response; httpResponse.Body = new MemoryStream(); var actionContext = CreateMockActionContext( - outputFormatters: new IOutputFormatter[] + outputFormatters: new IOutputFormatter[] { new HttpNotAcceptableOutputFormatter(), new JsonOutputFormatter() @@ -172,7 +173,7 @@ namespace Microsoft.AspNet.Mvc.ActionResults var actionContext = CreateMockActionContext( new[] { formatter.Object }, setupActionBindingContext: false); - + // Set the content type property explicitly to a single value. var result = new ObjectResult("someValue"); @@ -383,7 +384,7 @@ namespace Microsoft.AspNet.Mvc.ActionResults // Assert Assert.Equal(mockCountingFormatter.Object, formatter); mockCountingFormatter.Verify(v => v.CanWriteResult(context, null), Times.Once()); - + // CanWriteResult is invoked for the following cases: // 1. For each accept header present // 2. Request Content-Type @@ -695,7 +696,7 @@ namespace Microsoft.AspNet.Mvc.ActionResults // Arrange var expectedData = "Hello World!"; var objectResult = new ObjectResult(expectedData); - var outputFormatters = new IOutputFormatter[] + var outputFormatters = new IOutputFormatter[] { new HttpNotAcceptableOutputFormatter(), new StringOutputFormatter(), @@ -727,7 +728,7 @@ namespace Microsoft.AspNet.Mvc.ActionResults // Arrange var objectResult = new ObjectResult(new Person() { Name = "John" }); objectResult.ContentTypes.Add(new MediaTypeHeaderValue("application/json")); - var outputFormatters = new IOutputFormatter[] + var outputFormatters = new IOutputFormatter[] { new HttpNotAcceptableOutputFormatter(), new JsonOutputFormatter() @@ -760,7 +761,7 @@ namespace Microsoft.AspNet.Mvc.ActionResults var objectResult = new ObjectResult(new Person() { Name = "John" }); objectResult.ContentTypes.Add(new MediaTypeHeaderValue("application/foo")); objectResult.ContentTypes.Add(new MediaTypeHeaderValue("application/json")); - var outputFormatters = new IOutputFormatter[] + var outputFormatters = new IOutputFormatter[] { new HttpNotAcceptableOutputFormatter(), new JsonOutputFormatter() @@ -891,6 +892,7 @@ namespace Microsoft.AspNet.Mvc.ActionResults request.ContentType = requestContentType; request.Body = new MemoryStream(contentBytes); + httpContext.Setup(o => o.Features).Returns(new FeatureCollection()); httpContext.Setup(o => o.Request).Returns(request); httpContext.Setup(o => o.RequestServices).Returns(GetServiceProvider()); @@ -911,7 +913,7 @@ namespace Microsoft.AspNet.Mvc.ActionResults { actionBindingContext = new ActionBindingContext { OutputFormatters = outputFormatters.ToList() }; } - + httpContext.Setup(o => o.RequestServices.GetService(typeof(IActionBindingContextAccessor))) .Returns(new ActionBindingContextAccessor() { ActionBindingContext = actionBindingContext }); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/StreamOutputFormatterTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/StreamOutputFormatterTest.cs index 37fa0dff68..9be7cb276f 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/StreamOutputFormatterTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Formatters/StreamOutputFormatterTest.cs @@ -3,6 +3,10 @@ using System; using System.IO; +using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNet.Http.Features; +using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc.ActionResults; using Microsoft.Net.Http.Headers; using Xunit; @@ -97,6 +101,29 @@ namespace Microsoft.AspNet.Mvc.Formatters Assert.Same(contentType, context.SelectedContentType); } + [Fact] + public async Task DisablesResponseBuffering_IfBufferingFeatureAvailable() + { + // Arrange + var expected = Encoding.UTF8.GetBytes("Test data"); + var formatter = new StreamOutputFormatter(); + var httpContext = new DefaultHttpContext(); + var ms = new MemoryStream(); + httpContext.Response.Body = ms; + var bufferingFeature = new TestBufferingFeature(); + httpContext.Features.Set(bufferingFeature); + var context = new OutputFormatterContext(); + context.Object = new MemoryStream(expected); + context.HttpContext = httpContext; + + // Act + await formatter.WriteAsync(context); + + // Assert + Assert.Equal(expected, ms.ToArray()); + Assert.True(bufferingFeature.DisableResponseBufferingInvoked); + } + private class SimplePOCO { public int Id { get; set; } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/TestBufferingFeature.cs b/test/Microsoft.AspNet.Mvc.Core.Test/TestBufferingFeature.cs new file mode 100644 index 0000000000..2cc756ba8b --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/TestBufferingFeature.cs @@ -0,0 +1,24 @@ +// 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 Microsoft.AspNet.Http.Features; + +namespace Microsoft.AspNet.Mvc +{ + internal class TestBufferingFeature : IHttpBufferingFeature + { + public bool DisableResponseBufferingInvoked { get; private set; } + + public bool DisableRequestBufferingInvoked { get; private set; } + + public void DisableRequestBuffering() + { + DisableRequestBufferingInvoked = true; + } + + public void DisableResponseBuffering() + { + DisableResponseBufferingInvoked = true; + } + } +}