diff --git a/src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs b/src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs index 4070311a3b95..82c72e693ba5 100644 --- a/src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs +++ b/src/Http/Http/ref/Microsoft.AspNetCore.Http.netcoreapp3.0.cs @@ -306,7 +306,6 @@ public BindingAddress() { } } public static partial class BufferingHelper { - public static string TempDirectory { get { throw null; } } public static Microsoft.AspNetCore.Http.HttpRequest EnableRewind(this Microsoft.AspNetCore.Http.HttpRequest request, int bufferThreshold = 30720, long? bufferLimit = default(long?)) { throw null; } public static Microsoft.AspNetCore.WebUtilities.MultipartSection EnableRewind(this Microsoft.AspNetCore.WebUtilities.MultipartSection section, System.Action registerForDispose, int bufferThreshold = 30720, long? bufferLimit = default(long?)) { throw null; } } diff --git a/src/Http/Http/src/Internal/BufferingHelper.cs b/src/Http/Http/src/Internal/BufferingHelper.cs index b912f37116b2..d3ae6e42c83b 100644 --- a/src/Http/Http/src/Internal/BufferingHelper.cs +++ b/src/Http/Http/src/Internal/BufferingHelper.cs @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.IO; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.WebUtilities; namespace Microsoft.AspNetCore.Http.Internal @@ -11,33 +11,6 @@ public static class BufferingHelper { internal const int DefaultBufferThreshold = 1024 * 30; - private readonly static Func _getTempDirectory = () => TempDirectory; - - private static string _tempDirectory; - - public static string TempDirectory - { - get - { - if (_tempDirectory == null) - { - // Look for folders in the following order. - var temp = Environment.GetEnvironmentVariable("ASPNETCORE_TEMP") ?? // ASPNETCORE_TEMP - User set temporary location. - Path.GetTempPath(); // Fall back. - - if (!Directory.Exists(temp)) - { - // TODO: ??? - throw new DirectoryNotFoundException(temp); - } - - _tempDirectory = temp; - } - - return _tempDirectory; - } - } - public static HttpRequest EnableRewind(this HttpRequest request, int bufferThreshold = DefaultBufferThreshold, long? bufferLimit = null) { if (request == null) @@ -48,7 +21,7 @@ public static HttpRequest EnableRewind(this HttpRequest request, int bufferThres var body = request.Body; if (!body.CanSeek) { - var fileStream = new FileBufferingReadStream(body, bufferThreshold, bufferLimit, _getTempDirectory); + var fileStream = new FileBufferingReadStream(body, bufferThreshold, bufferLimit, AspNetCoreTempDirectory.TempDirectoryFactory); request.Body = fileStream; request.HttpContext.Response.RegisterForDispose(fileStream); } @@ -70,11 +43,11 @@ public static MultipartSection EnableRewind(this MultipartSection section, Actio var body = section.Body; if (!body.CanSeek) { - var fileStream = new FileBufferingReadStream(body, bufferThreshold, bufferLimit, _getTempDirectory); + var fileStream = new FileBufferingReadStream(body, bufferThreshold, bufferLimit, AspNetCoreTempDirectory.TempDirectoryFactory); section.Body = fileStream; registerForDispose(fileStream); } return section; } } -} \ No newline at end of file +} diff --git a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj index fd9d84712a62..50d2ff7281a9 100644 --- a/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj +++ b/src/Http/Http/src/Microsoft.AspNetCore.Http.csproj @@ -13,6 +13,7 @@ + diff --git a/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs b/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs index 921c5056231f..45d806e29783 100644 --- a/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs +++ b/src/Http/WebUtilities/ref/Microsoft.AspNetCore.WebUtilities.netcoreapp3.0.cs @@ -62,6 +62,29 @@ public override void SetLength(long value) { } public override void Write(byte[] buffer, int offset, int count) { } public override System.Threading.Tasks.Task WriteAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } } + public sealed partial class FileBufferingWriteStream : System.IO.Stream + { + public FileBufferingWriteStream(int memoryThreshold = 32768, long? bufferLimit = default(long?), System.Func tempFileDirectoryAccessor = null) { } + public override bool CanRead { get { throw null; } } + public override bool CanSeek { get { throw null; } } + public override bool CanWrite { get { throw null; } } + public override long Length { get { throw null; } } + public override long Position { get { throw null; } set { } } + protected override void Dispose(bool disposing) { } + [System.Diagnostics.DebuggerStepThroughAttribute] + public override System.Threading.Tasks.ValueTask DisposeAsync() { throw null; } + [System.Diagnostics.DebuggerStepThroughAttribute] + public System.Threading.Tasks.Task DrainBufferAsync(System.IO.Stream destination, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + public override void Flush() { } + public override System.Threading.Tasks.Task FlushAsync(System.Threading.CancellationToken cancellationToken) { throw null; } + public override int Read(byte[] buffer, int offset, int count) { throw null; } + public override System.Threading.Tasks.Task ReadAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } + public override long Seek(long offset, System.IO.SeekOrigin origin) { throw null; } + public override void SetLength(long value) { } + public override void Write(byte[] buffer, int offset, int count) { } + [System.Diagnostics.DebuggerStepThroughAttribute] + public override System.Threading.Tasks.Task WriteAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) { throw null; } + } public partial class FileMultipartSection { public FileMultipartSection(Microsoft.AspNetCore.WebUtilities.MultipartSection section) { } @@ -129,6 +152,8 @@ public HttpResponseStreamWriter(System.IO.Stream stream, System.Text.Encoding en public HttpResponseStreamWriter(System.IO.Stream stream, System.Text.Encoding encoding, int bufferSize, System.Buffers.ArrayPool bytePool, System.Buffers.ArrayPool charPool) { } public override System.Text.Encoding Encoding { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } protected override void Dispose(bool disposing) { } + [System.Diagnostics.DebuggerStepThroughAttribute] + public override System.Threading.Tasks.ValueTask DisposeAsync() { throw null; } public override void Flush() { } public override System.Threading.Tasks.Task FlushAsync() { throw null; } public override void Write(char value) { } diff --git a/src/Http/WebUtilities/src/AspNetCoreTempDirectory.cs b/src/Http/WebUtilities/src/AspNetCoreTempDirectory.cs new file mode 100644 index 000000000000..868d4efd05e9 --- /dev/null +++ b/src/Http/WebUtilities/src/AspNetCoreTempDirectory.cs @@ -0,0 +1,37 @@ +// 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; + +namespace Microsoft.AspNetCore.Internal +{ + internal static class AspNetCoreTempDirectory + { + private static string _tempDirectory; + + public static string TempDirectory + { + get + { + if (_tempDirectory == null) + { + // Look for folders in the following order. + var temp = Environment.GetEnvironmentVariable("ASPNETCORE_TEMP") ?? // ASPNETCORE_TEMP - User set temporary location. + Path.GetTempPath(); // Fall back. + + if (!Directory.Exists(temp)) + { + throw new DirectoryNotFoundException(temp); + } + + _tempDirectory = temp; + } + + return _tempDirectory; + } + } + + public static Func TempDirectoryFactory => () => TempDirectory; + } +} diff --git a/src/Http/WebUtilities/src/FileBufferingWriteStream.cs b/src/Http/WebUtilities/src/FileBufferingWriteStream.cs new file mode 100644 index 000000000000..ee03685312cf --- /dev/null +++ b/src/Http/WebUtilities/src/FileBufferingWriteStream.cs @@ -0,0 +1,270 @@ +// 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.Buffers; +using System.Diagnostics; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Internal; + +namespace Microsoft.AspNetCore.WebUtilities +{ + /// + /// A that buffers content to be written to disk. Use + /// to write buffered content to a target . + /// + public sealed class FileBufferingWriteStream : Stream + { + private const int DefaultMemoryThreshold = 32 * 1024; // 32k + + private readonly int _memoryThreshold; + private readonly long? _bufferLimit; + private readonly Func _tempFileDirectoryAccessor; + + /// + /// Initializes a new instance of . + /// + /// + /// The maximum amount of memory in bytes to allocate before switching to a file on disk. + /// Defaults to 32kb. + /// + /// + /// The maximum amouont of bytes that the is allowed to buffer. + /// + /// Provides the location of the directory to write buffered contents to. + /// When unspecified, uses the value specified by the environment variable ASPNETCORE_TEMP if available, otherwise + /// uses the value returned by . + /// + public FileBufferingWriteStream( + int memoryThreshold = DefaultMemoryThreshold, + long? bufferLimit = null, + Func tempFileDirectoryAccessor = null) + { + if (memoryThreshold < 0) + { + throw new ArgumentOutOfRangeException(nameof(memoryThreshold)); + } + + if (bufferLimit != null && bufferLimit < memoryThreshold) + { + // We would expect a limit at least as much as memoryThreshold + throw new ArgumentOutOfRangeException(nameof(bufferLimit), $"{nameof(bufferLimit)} must be larger than {nameof(memoryThreshold)}."); + } + + _memoryThreshold = memoryThreshold; + _bufferLimit = bufferLimit; + _tempFileDirectoryAccessor = tempFileDirectoryAccessor ?? AspNetCoreTempDirectory.TempDirectoryFactory; + PagedByteBuffer = new PagedByteBuffer(ArrayPool.Shared); + } + + /// + public override bool CanRead => false; + + /// + public override bool CanSeek => false; + + /// + public override bool CanWrite => true; + + /// + public override long Length => throw new NotSupportedException(); + + /// + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + + internal long BufferedLength => PagedByteBuffer.Length + (FileStream?.Length ?? 0); + + internal PagedByteBuffer PagedByteBuffer { get; } + + internal FileStream FileStream { get; private set; } + + internal bool Disposed { get; private set; } + + /// + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + + /// + public override int Read(byte[] buffer, int offset, int count) + => throw new NotSupportedException(); + + /// + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + => throw new NotSupportedException(); + + /// + public override void Write(byte[] buffer, int offset, int count) + { + ThrowArgumentException(buffer, offset, count); + ThrowIfDisposed(); + + if (_bufferLimit.HasValue && _bufferLimit - BufferedLength < count) + { + Dispose(); + throw new IOException("Buffer limit exceeded."); + } + + // Allow buffering in memory if we're below the memory threshold once the current buffer is written. + var allowMemoryBuffer = (_memoryThreshold - count) >= PagedByteBuffer.Length; + if (allowMemoryBuffer) + { + // Buffer content in the MemoryStream if it has capacity. + PagedByteBuffer.Add(buffer, offset, count); + Debug.Assert(PagedByteBuffer.Length <= _memoryThreshold); + } + else + { + // If the MemoryStream is incapable of accomodating the content to be written + // spool to disk. + EnsureFileStream(); + + // Spool memory content to disk. + PagedByteBuffer.MoveTo(FileStream); + + FileStream.Write(buffer, offset, count); + } + } + + /// + public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + ThrowArgumentException(buffer, offset, count); + ThrowIfDisposed(); + + if (_bufferLimit.HasValue && _bufferLimit - BufferedLength < count) + { + Dispose(); + throw new IOException("Buffer limit exceeded."); + } + + // Allow buffering in memory if we're below the memory threshold once the current buffer is written. + var allowMemoryBuffer = (_memoryThreshold - count) >= PagedByteBuffer.Length; + if (allowMemoryBuffer) + { + // Buffer content in the MemoryStream if it has capacity. + PagedByteBuffer.Add(buffer, offset, count); + Debug.Assert(PagedByteBuffer.Length <= _memoryThreshold); + } + else + { + // If the MemoryStream is incapable of accomodating the content to be written + // spool to disk. + EnsureFileStream(); + + // Spool memory content to disk. + await PagedByteBuffer.MoveToAsync(FileStream, cancellationToken); + await FileStream.WriteAsync(buffer, offset, count, cancellationToken); + } + } + + /// + public override void Flush() + { + // Do nothing. + } + + /// + public override Task FlushAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + /// + public override void SetLength(long value) => throw new NotSupportedException(); + + /// + /// Drains buffered content to . + /// + /// The to drain buffered contents to. + /// The . + /// A that represents the asynchronous drain operation. + public async Task DrainBufferAsync(Stream destination, CancellationToken cancellationToken = default) + { + // When not null, FileStream always has "older" spooled content. The PagedByteBuffer always has "newer" + // unspooled content. Copy the FileStream content first when available. + if (FileStream != null) + { + FileStream.Position = 0; + await FileStream.CopyToAsync(destination, cancellationToken); + + await FileStream.DisposeAsync(); + FileStream = null; + } + + await PagedByteBuffer.MoveToAsync(destination, cancellationToken); + } + + /// + protected override void Dispose(bool disposing) + { + if (!Disposed) + { + Disposed = true; + + PagedByteBuffer.Dispose(); + FileStream?.Dispose(); + } + } + + /// + public override async ValueTask DisposeAsync() + { + if (!Disposed) + { + Disposed = true; + + PagedByteBuffer.Dispose(); + await (FileStream?.DisposeAsync() ?? default); + } + } + + private void EnsureFileStream() + { + if (FileStream == null) + { + var tempFileDirectory = _tempFileDirectoryAccessor(); + var tempFileName = Path.Combine(tempFileDirectory, "ASPNETCORE_" + Guid.NewGuid() + ".tmp"); + FileStream = new FileStream( + tempFileName, + FileMode.Create, + FileAccess.ReadWrite, + FileShare.Delete, + bufferSize: 1, + FileOptions.Asynchronous | FileOptions.SequentialScan | FileOptions.DeleteOnClose); + } + } + + private void ThrowIfDisposed() + { + if (Disposed) + { + throw new ObjectDisposedException(nameof(FileBufferingReadStream)); + } + } + + private static void ThrowArgumentException(byte[] buffer, int offset, int count) + { + if (buffer == null) + { + throw new ArgumentNullException(nameof(buffer)); + } + + if (offset < 0) + { + throw new ArgumentOutOfRangeException(nameof(offset)); + } + + if (count < 0) + { + throw new ArgumentOutOfRangeException(nameof(count)); + } + + if (buffer.Length - offset < count) + { + throw new ArgumentOutOfRangeException(nameof(offset)); + } + } + } +} diff --git a/src/Http/WebUtilities/src/HttpResponseStreamWriter.cs b/src/Http/WebUtilities/src/HttpResponseStreamWriter.cs index c5bcdd0aa3c0..a17116ae3e38 100644 --- a/src/Http/WebUtilities/src/HttpResponseStreamWriter.cs +++ b/src/Http/WebUtilities/src/HttpResponseStreamWriter.cs @@ -310,6 +310,25 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + public override async ValueTask DisposeAsync() + { + if (!_disposed) + { + _disposed = true; + try + { + await FlushInternalAsync(flushEncoder: true); + } + finally + { + _bytePool.Return(_byteBuffer); + _charPool.Return(_charBuffer); + } + } + + await base.DisposeAsync(); + } + // Note: our FlushInternal method does NOT flush the underlying stream. This would result in // chunking. private void FlushInternal(bool flushEncoder) diff --git a/src/Http/WebUtilities/src/PagedByteBuffer.cs b/src/Http/WebUtilities/src/PagedByteBuffer.cs new file mode 100644 index 000000000000..cd9c19682d36 --- /dev/null +++ b/src/Http/WebUtilities/src/PagedByteBuffer.cs @@ -0,0 +1,134 @@ +// 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.Buffers; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.WebUtilities +{ + internal sealed class PagedByteBuffer : IDisposable + { + internal const int PageSize = 1024; + private readonly ArrayPool _arrayPool; + private byte[] _currentPage; + private int _currentPageIndex; + + public PagedByteBuffer(ArrayPool arrayPool) + { + _arrayPool = arrayPool; + Pages = new List(); + } + + public int Length { get; private set; } + + internal bool Disposed { get; private set; } + + internal List Pages { get; } + + private byte[] CurrentPage + { + get + { + if (_currentPage == null || _currentPageIndex == _currentPage.Length) + { + _currentPage = _arrayPool.Rent(PageSize); + Pages.Add(_currentPage); + _currentPageIndex = 0; + } + + return _currentPage; + } + } + + public void Add(byte[] buffer, int offset, int count) + { + ThrowIfDisposed(); + + while (count > 0) + { + var currentPage = CurrentPage; + var copyLength = Math.Min(count, currentPage.Length - _currentPageIndex); + + Buffer.BlockCopy( + buffer, + offset, + currentPage, + _currentPageIndex, + copyLength); + + Length += copyLength; + _currentPageIndex += copyLength; + offset += copyLength; + count -= copyLength; + } + } + + public void MoveTo(Stream stream) + { + ThrowIfDisposed(); + + for (var i = 0; i < Pages.Count; i++) + { + var page = Pages[i]; + var length = (i == Pages.Count - 1) ? + _currentPageIndex : + page.Length; + + stream.Write(page, 0, length); + } + + ClearBuffers(); + } + + public async Task MoveToAsync(Stream stream, CancellationToken cancellationToken) + { + ThrowIfDisposed(); + + for (var i = 0; i < Pages.Count; i++) + { + var page = Pages[i]; + var length = (i == Pages.Count - 1) ? + _currentPageIndex : + page.Length; + + await stream.WriteAsync(page, 0, length, cancellationToken); + } + + ClearBuffers(); + } + + public void Dispose() + { + if (!Disposed) + { + Disposed = true; + ClearBuffers(); + } + } + + private void ClearBuffers() + { + for (var i = 0; i < Pages.Count; i++) + { + _arrayPool.Return(Pages[i]); + } + + Pages.Clear(); + _currentPage = null; + Length = 0; + _currentPageIndex = 0; + } + + private void ThrowIfDisposed() + { + if (Disposed) + { + throw new ObjectDisposedException(nameof(PagedByteBuffer)); + } + } + } +} diff --git a/src/Http/Http/test/Internal/BufferingHelperTests.cs b/src/Http/WebUtilities/test/AspNetCoreTempDirectoryTests.cs similarity index 72% rename from src/Http/Http/test/Internal/BufferingHelperTests.cs rename to src/Http/WebUtilities/test/AspNetCoreTempDirectoryTests.cs index 9ad48986f501..4e0621ca8567 100644 --- a/src/Http/Http/test/Internal/BufferingHelperTests.cs +++ b/src/Http/WebUtilities/test/AspNetCoreTempDirectoryTests.cs @@ -4,16 +4,16 @@ using System.IO; using Xunit; -namespace Microsoft.AspNetCore.Http.Internal +namespace Microsoft.AspNetCore.Internal { - public class BufferingHelperTests + public class AspNetCoreTempDirectoryTests { [Fact] public void GetTempDirectory_Returns_Valid_Location() { - var tempDirectory = BufferingHelper.TempDirectory; + var tempDirectory = AspNetCoreTempDirectory.TempDirectory; Assert.NotNull(tempDirectory); Assert.True(Directory.Exists(tempDirectory)); } } -} \ No newline at end of file +} diff --git a/src/Http/WebUtilities/test/FileBufferingWriteStreamTests.cs b/src/Http/WebUtilities/test/FileBufferingWriteStreamTests.cs new file mode 100644 index 000000000000..14f0c081b5f0 --- /dev/null +++ b/src/Http/WebUtilities/test/FileBufferingWriteStreamTests.cs @@ -0,0 +1,393 @@ +// 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.Buffers; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.AspNetCore.WebUtilities +{ + public class FileBufferingWriteStreamTests : IDisposable + { + private readonly string TempDirectory = Path.Combine(Path.GetTempPath(), "FileBufferingWriteTests", Path.GetRandomFileName()); + + public FileBufferingWriteStreamTests() + { + Directory.CreateDirectory(TempDirectory); + } + + [Fact] + public void Write_BuffersContentToMemory() + { + // Arrange + using var bufferingStream = new FileBufferingWriteStream(tempFileDirectoryAccessor: () => TempDirectory); + var input = Encoding.UTF8.GetBytes("Hello world"); + + // Act + bufferingStream.Write(input, 0, input.Length); + + // Assert + // We should have written content to memory + var pagedByteBuffer = bufferingStream.PagedByteBuffer; + Assert.Equal(input, ReadBufferedContent(pagedByteBuffer)); + + // No files should not have been created. + Assert.Null(bufferingStream.FileStream); + } + + [Fact] + public void Write_BeforeMemoryThresholdIsReached_WritesToMemory() + { + // Arrange + var input = new byte[] { 1, 2, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + bufferingStream.Write(input, 0, 2); + + // Assert + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.Null(fileStream); + + // No content should be in the memory stream + Assert.Equal(2, pageBuffer.Length); + Assert.Equal(input, ReadBufferedContent(pageBuffer)); + } + + [Fact] + public void Write_BuffersContentToDisk_WhenMemoryThresholdIsReached() + { + // Arrange + var input = new byte[] { 1, 2, 3, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, 2); + + // Act + bufferingStream.Write(input, 2, 1); + + // Assert + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(input, fileBytes); + + // No content should be in the memory stream + Assert.Equal(0, pageBuffer.Length); + } + + [Fact] + public void Write_BuffersContentToDisk_WhenWriteWillOverflowMemoryThreshold() + { + // Arrange + var input = new byte[] { 1, 2, 3, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + bufferingStream.Write(input, 0, input.Length); + + // Assert + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(input, fileBytes); + + // No content should be in the memory stream + Assert.Equal(0, pageBuffer.Length); + } + + [Fact] + public void Write_AfterMemoryThresholdIsReached_BuffersToMemory() + { + // Arrange + var input = new byte[] { 1, 2, 3, 4, 5, 6, 7 }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 4, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + bufferingStream.Write(input, 0, 5); + bufferingStream.Write(input, 5, 2); + + // Assert + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(new byte[] { 1, 2, 3, 4, 5, }, fileBytes); + + Assert.Equal(new byte[] { 6, 7 }, ReadBufferedContent(pageBuffer)); + } + + [Fact] + public async Task WriteAsync_BuffersContentToMemory() + { + // Arrange + using var bufferingStream = new FileBufferingWriteStream(tempFileDirectoryAccessor: () => TempDirectory); + var input = Encoding.UTF8.GetBytes("Hello world"); + + // Act + await bufferingStream.WriteAsync(input, 0, input.Length); + + // Assert + // We should have written content to memory + var pagedByteBuffer = bufferingStream.PagedByteBuffer; + Assert.Equal(input, ReadBufferedContent(pagedByteBuffer)); + + // No files should not have been created. + Assert.Null(bufferingStream.FileStream); + } + + [Fact] + public async Task WriteAsync_BeforeMemoryThresholdIsReached_WritesToMemory() + { + // Arrange + var input = new byte[] { 1, 2, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + await bufferingStream.WriteAsync(input, 0, 2); + + // Assert + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.Null(fileStream); + + // No content should be in the memory stream + Assert.Equal(2, pageBuffer.Length); + Assert.Equal(input, ReadBufferedContent(pageBuffer)); + } + + [Fact] + public async Task WriteAsync_BuffersContentToDisk_WhenMemoryThresholdIsReached() + { + // Arrange + var input = new byte[] { 1, 2, 3, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, 2); + + // Act + await bufferingStream.WriteAsync(input, 2, 1); + + // Assert + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(input, fileBytes); + + // No content should be in the memory stream + Assert.Equal(0, pageBuffer.Length); + } + + [Fact] + public async Task WriteAsync_BuffersContentToDisk_WhenWriteWillOverflowMemoryThreshold() + { + // Arrange + var input = new byte[] { 1, 2, 3, }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + await bufferingStream.WriteAsync(input, 0, input.Length); + + // Assert + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + Assert.Equal(input, fileBytes); + + // No content should be in the memory stream + Assert.Equal(0, pageBuffer.Length); + } + + [Fact] + public async Task WriteAsync_AfterMemoryThresholdIsReached_BuffersToMemory() + { + // Arrange + var input = new byte[] { 1, 2, 3, 4, 5, 6, 7 }; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 4, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + await bufferingStream.WriteAsync(input, 0, 5); + await bufferingStream.WriteAsync(input, 5, 2); + + // Assert + var pageBuffer = bufferingStream.PagedByteBuffer; + var fileStream = bufferingStream.FileStream; + + // File should have been created. + Assert.NotNull(fileStream); + var fileBytes = ReadFileContent(fileStream); + + Assert.Equal(new byte[] { 1, 2, 3, 4, 5, }, fileBytes); + Assert.Equal(new byte[] { 6, 7 }, ReadBufferedContent(pageBuffer)); + } + + [Fact] + public void Write_Throws_IfSingleWriteExceedsBufferLimit() + { + // Arrange + var input = new byte[20]; + var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + var exception = Assert.Throws(() => bufferingStream.Write(input, 0, input.Length)); + Assert.Equal("Buffer limit exceeded.", exception.Message); + + Assert.True(bufferingStream.Disposed); + } + + [Fact] + public void Write_Throws_IfWriteCumulativeWritesExceedsBuffersLimit() + { + // Arrange + var input = new byte[6]; + var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + bufferingStream.Write(input, 0, input.Length); + var exception = Assert.Throws(() => bufferingStream.Write(input, 0, input.Length)); + Assert.Equal("Buffer limit exceeded.", exception.Message); + + // Verify we return the buffer. + Assert.True(bufferingStream.Disposed); + } + + [Fact] + public void Write_DoesNotThrow_IfBufferLimitIsReached() + { + // Arrange + var input = new byte[5]; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + bufferingStream.Write(input, 0, input.Length); + bufferingStream.Write(input, 0, input.Length); // Should get to exactly the buffer limit, which is fine + + // If we got here, the test succeeded. + } + + [Fact] + public async Task WriteAsync_Throws_IfSingleWriteExceedsBufferLimit() + { + // Arrange + var input = new byte[20]; + var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + var exception = await Assert.ThrowsAsync(() => bufferingStream.WriteAsync(input, 0, input.Length)); + Assert.Equal("Buffer limit exceeded.", exception.Message); + + Assert.True(bufferingStream.Disposed); + } + + [Fact] + public async Task WriteAsync_Throws_IfWriteCumulativeWritesExceedsBuffersLimit() + { + // Arrange + var input = new byte[6]; + var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + await bufferingStream.WriteAsync(input, 0, input.Length); + var exception = await Assert.ThrowsAsync(() => bufferingStream.WriteAsync(input, 0, input.Length)); + Assert.Equal("Buffer limit exceeded.", exception.Message); + + // Verify we return the buffer. + Assert.True(bufferingStream.Disposed); + } + + [Fact] + public async Task WriteAsync_DoesNotThrow_IfBufferLimitIsReached() + { + // Arrange + var input = new byte[5]; + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 2, bufferLimit: 10, tempFileDirectoryAccessor: () => TempDirectory); + + // Act + await bufferingStream.WriteAsync(input, 0, input.Length); + await bufferingStream.WriteAsync(input, 0, input.Length); // Should get to exactly the buffer limit, which is fine + + // If we got here, the test succeeded. + } + + [Fact] + public async Task DrainBufferAsync_CopiesContentFromMemoryStream() + { + // Arrange + var input = new byte[] { 1, 2, 3, 4, 5 }; + using var bufferingStream = new FileBufferingWriteStream(tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, input.Length); + var memoryStream = new MemoryStream(); + + // Act + await bufferingStream.DrainBufferAsync(memoryStream, default); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + [Fact] + public async Task DrainBufferAsync_WithContentInDisk_CopiesContentFromMemoryStream() + { + // Arrange + var input = Enumerable.Repeat((byte)0xca, 30).ToArray(); + using var bufferingStream = new FileBufferingWriteStream(memoryThreshold: 21, tempFileDirectoryAccessor: () => TempDirectory); + bufferingStream.Write(input, 0, input.Length); + var memoryStream = new MemoryStream(); + + // Act + await bufferingStream.DrainBufferAsync(memoryStream, default); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + public void Dispose() + { + try + { + Directory.Delete(TempDirectory, recursive: true); + } + catch + { + } + } + + private static byte[] ReadFileContent(FileStream fileStream) + { + fileStream.Position = 0; + using var memoryStream = new MemoryStream(); + fileStream.CopyTo(memoryStream); + + return memoryStream.ToArray(); + } + + private static byte[] ReadBufferedContent(PagedByteBuffer buffer) + { + using var memoryStream = new MemoryStream(); + buffer.MoveTo(memoryStream); + + return memoryStream.ToArray(); + } + } +} diff --git a/src/Http/WebUtilities/test/FormPipeReaderTests.cs b/src/Http/WebUtilities/test/FormPipeReaderTests.cs index a95a23db6344..23c3be73fd4f 100644 --- a/src/Http/WebUtilities/test/FormPipeReaderTests.cs +++ b/src/Http/WebUtilities/test/FormPipeReaderTests.cs @@ -10,7 +10,7 @@ using Microsoft.Extensions.Primitives; using Xunit; -namespace Microsoft.AspNetCore.WebUtilities.Test +namespace Microsoft.AspNetCore.WebUtilities { public class FormPipeReaderTests { diff --git a/src/Http/WebUtilities/test/HttpRequestStreamReaderTest.cs b/src/Http/WebUtilities/test/HttpRequestStreamReaderTest.cs index 062342fa4cda..6cab20bccc7f 100644 --- a/src/Http/WebUtilities/test/HttpRequestStreamReaderTest.cs +++ b/src/Http/WebUtilities/test/HttpRequestStreamReaderTest.cs @@ -11,7 +11,7 @@ using Xunit; -namespace Microsoft.AspNetCore.WebUtilities.Test +namespace Microsoft.AspNetCore.WebUtilities { public class HttpResponseStreamReaderTest { diff --git a/src/Http/WebUtilities/test/HttpResponseStreamWriterTest.cs b/src/Http/WebUtilities/test/HttpResponseStreamWriterTest.cs index 7847e1384e24..30402a5bbc71 100644 --- a/src/Http/WebUtilities/test/HttpResponseStreamWriterTest.cs +++ b/src/Http/WebUtilities/test/HttpResponseStreamWriterTest.cs @@ -11,7 +11,7 @@ using System.Threading.Tasks; using Xunit; -namespace Microsoft.AspNetCore.WebUtilities.Test +namespace Microsoft.AspNetCore.WebUtilities { public class HttpResponseStreamWriterTest { diff --git a/src/Http/WebUtilities/test/PagedByteBufferTest.cs b/src/Http/WebUtilities/test/PagedByteBufferTest.cs new file mode 100644 index 000000000000..1494584da4c6 --- /dev/null +++ b/src/Http/WebUtilities/test/PagedByteBufferTest.cs @@ -0,0 +1,248 @@ +// 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.Linq; +using System.Text; +using System.Threading.Tasks; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.WebUtilities +{ + public class PagedByteBufferTest + { + [Fact] + public void Add_CreatesNewPage() + { + // Arrange + var input = Encoding.UTF8.GetBytes("Hello world"); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + + // Act + buffer.Add(input, 0, input.Length); + + // Assert + Assert.Single(buffer.Pages); + Assert.Equal(input.Length, buffer.Length); + Assert.Equal(input, ReadBufferedContent(buffer)); + } + + [Fact] + public void Add_AppendsToExistingPage() + { + // Arrange + var input1 = Encoding.UTF8.GetBytes("Hello"); + var input2 = Encoding.UTF8.GetBytes("world"); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input1, 0, input1.Length); + + // Act + buffer.Add(input2, 0, input2.Length); + + // Assert + Assert.Single(buffer.Pages); + Assert.Equal(10, buffer.Length); + Assert.Equal(Enumerable.Concat(input1, input2).ToArray(), ReadBufferedContent(buffer)); + } + + [Fact] + public void Add_WithOffsets() + { + // Arrange + var input = new byte[] { 1, 2, 3, 4, 5 }; + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + + // Act + buffer.Add(input, 1, 3); + + // Assert + Assert.Single(buffer.Pages); + Assert.Equal(3, buffer.Length); + Assert.Equal(new byte[] { 2, 3, 4 }, ReadBufferedContent(buffer)); + } + + [Fact] + public void Add_FillsUpBuffer() + { + // Arrange + var input1 = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize - 1).ToArray(); + var input2 = new byte[] { 0xca }; + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input1, 0, input1.Length); + + // Act + buffer.Add(input2, 0, 1); + + // Assert + Assert.Single(buffer.Pages); + Assert.Equal(PagedByteBuffer.PageSize, buffer.Length); + Assert.Equal(Enumerable.Concat(input1, input2).ToArray(), ReadBufferedContent(buffer)); + } + + [Fact] + public void Add_AppendsToMultiplePages() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize + 10).ToArray(); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + + // Act + buffer.Add(input, 0, input.Length); + + // Assert + Assert.Equal(2, buffer.Pages.Count); + Assert.Equal(PagedByteBuffer.PageSize + 10, buffer.Length); + Assert.Equal(input.ToArray(), ReadBufferedContent(buffer)); + } + + [Fact] + public void MoveTo_CopiesContentToStream() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input, 0, input.Length); + var stream = new MemoryStream(); + + // Act + buffer.MoveTo(stream); + + // Assert + Assert.Equal(input, stream.ToArray()); + + // Verify moving new content works. + var newInput = Enumerable.Repeat((byte)0xcb, PagedByteBuffer.PageSize * 2 + 13).ToArray(); + buffer.Add(newInput, 0, newInput.Length); + + stream.SetLength(0); + buffer.MoveTo(stream); + + Assert.Equal(newInput, stream.ToArray()); + } + + [Fact] + public async Task MoveToAsync_CopiesContentToStream() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input, 0, input.Length); + var stream = new MemoryStream(); + + // Act + await buffer.MoveToAsync(stream, default); + + // Assert + Assert.Equal(input, stream.ToArray()); + + // Verify adding and moving new content works. + var newInput = Enumerable.Repeat((byte)0xcb, PagedByteBuffer.PageSize * 2 + 13).ToArray(); + buffer.Add(newInput, 0, newInput.Length); + stream.SetLength(0); + await buffer.MoveToAsync(stream, default); + + Assert.Equal(newInput, stream.ToArray()); + } + + [Fact] + public async Task MoveToAsync_ClearsBuffers() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); + using var buffer = new PagedByteBuffer(ArrayPool.Shared); + buffer.Add(input, 0, input.Length); + var stream = new MemoryStream(); + + // Act + await buffer.MoveToAsync(stream, default); + + // Assert + Assert.Equal(input, stream.ToArray()); + + // Verify copying it again works. + Assert.Equal(0, buffer.Length); + Assert.False(buffer.Disposed); + Assert.Empty(buffer.Pages); + } + + [Fact] + public void MoveTo_WithClear_ReturnsBuffers() + { + // Arrange + var input = new byte[] { 1, }; + var arrayPool = new Mock>(); + var byteArray = new byte[PagedByteBuffer.PageSize]; + arrayPool.Setup(p => p.Rent(PagedByteBuffer.PageSize)) + .Returns(byteArray); + arrayPool.Setup(p => p.Return(byteArray, false)).Verifiable(); + var memoryStream = new MemoryStream(); + + using (var buffer = new PagedByteBuffer(arrayPool.Object)) + { + // Act + buffer.Add(input, 0, input.Length); + buffer.MoveTo(memoryStream); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + arrayPool.Verify(p => p.Rent(It.IsAny()), Times.Once()); + arrayPool.Verify(p => p.Return(It.IsAny(), It.IsAny()), Times.Once()); + } + + [Fact] + public async Task MoveToAsync_ReturnsBuffers() + { + // Arrange + var input = new byte[] { 1, }; + var arrayPool = new Mock>(); + var byteArray = new byte[PagedByteBuffer.PageSize]; + arrayPool.Setup(p => p.Rent(PagedByteBuffer.PageSize)) + .Returns(byteArray); + var memoryStream = new MemoryStream(); + + using (var buffer = new PagedByteBuffer(arrayPool.Object)) + { + // Act + buffer.Add(input, 0, input.Length); + await buffer.MoveToAsync(memoryStream, default); + + // Assert + Assert.Equal(input, memoryStream.ToArray()); + } + + arrayPool.Verify(p => p.Rent(It.IsAny()), Times.Once()); + arrayPool.Verify(p => p.Return(It.IsAny(), It.IsAny()), Times.Once()); + } + + [Fact] + public void Dispose_ReturnsBuffers_ExactlyOnce() + { + // Arrange + var input = Enumerable.Repeat((byte)0xba, PagedByteBuffer.PageSize * 3 + 10).ToArray(); + var arrayPool = new Mock>(); + arrayPool.Setup(p => p.Rent(PagedByteBuffer.PageSize)) + .Returns(new byte[PagedByteBuffer.PageSize]); + + var buffer = new PagedByteBuffer(arrayPool.Object); + + // Act + buffer.Add(input, 0, input.Length); + buffer.Dispose(); + buffer.Dispose(); + + arrayPool.Verify(p => p.Rent(It.IsAny()), Times.Exactly(4)); + arrayPool.Verify(p => p.Return(It.IsAny(), It.IsAny()), Times.Exactly(4)); + } + + private static byte[] ReadBufferedContent(PagedByteBuffer buffer) + { + using var stream = new MemoryStream(); + buffer.MoveTo(stream); + return stream.ToArray(); + } + } +} diff --git a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs index 50a39b3b73aa..a1512c456a96 100644 --- a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs +++ b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs @@ -880,6 +880,7 @@ public MvcOptions() { } public int? SslPort { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public bool SuppressAsyncSuffixInActionNames { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public bool SuppressInputFormatterBuffering { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public bool SuppressOutputFormatterBuffering { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public System.Collections.Generic.IList ValueProviderFactories { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator() { throw null; } System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } diff --git a/src/Mvc/Mvc.Core/src/MvcOptions.cs b/src/Mvc/Mvc.Core/src/MvcOptions.cs index c5d2ea5d161b..3b8c53a539bf 100644 --- a/src/Mvc/Mvc.Core/src/MvcOptions.cs +++ b/src/Mvc/Mvc.Core/src/MvcOptions.cs @@ -105,7 +105,13 @@ public MvcOptions() /// /// Gets or sets the flag to buffer the request body in input formatters. Default is false. /// - public bool SuppressInputFormatterBuffering { get; set; } = false; + public bool SuppressInputFormatterBuffering { get; set; } + + /// + /// Gets or sets the flag that determines if buffering is disabled for output formatters that + /// synchronously write to the HTTP response body. + /// + public bool SuppressOutputFormatterBuffering { get; set; } /// /// Gets or sets the maximum number of validation errors that are allowed by this application before further diff --git a/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs b/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs index 01e7362bc2bb..0088dadf499f 100644 --- a/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs +++ b/src/Mvc/Mvc.Core/test/CreatedAtActionResultTests.cs @@ -91,9 +91,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs b/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs index 7494544d3142..829992317681 100644 --- a/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs +++ b/src/Mvc/Mvc.Core/test/CreatedAtRouteResultTests.cs @@ -2,7 +2,6 @@ // 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.IO; using System.Threading.Tasks; @@ -10,14 +9,12 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; -using Newtonsoft.Json; using Xunit; namespace Microsoft.AspNetCore.Mvc @@ -107,9 +104,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Core/test/CreatedResultTests.cs b/src/Mvc/Mvc.Core/test/CreatedResultTests.cs index f6bcf13473bb..8d984ec7beaa 100644 --- a/src/Mvc/Mvc.Core/test/CreatedResultTests.cs +++ b/src/Mvc/Mvc.Core/test/CreatedResultTests.cs @@ -9,7 +9,6 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; @@ -93,9 +92,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Core/test/Formatters/FormatFilterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/FormatFilterTest.cs index 0560d51d2876..bb8da19ae2a5 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/FormatFilterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/FormatFilterTest.cs @@ -2,13 +2,11 @@ // 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 Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging.Abstractions; @@ -16,7 +14,6 @@ using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using Moq; -using Newtonsoft.Json; using Xunit; namespace Microsoft.AspNetCore.Mvc.Formatters @@ -468,9 +465,7 @@ private void Initialize( // Set up default output formatters. MvcOptions.OutputFormatters.Add(new HttpNoContentOutputFormatter()); MvcOptions.OutputFormatters.Add(new StringOutputFormatter()); - MvcOptions.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + MvcOptions.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); // Set up default mapping for json extensions to content type MvcOptions.FormatterMappings.SetMediaTypeMappingForFormat( diff --git a/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs b/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs index 0039a4a9ade1..0467d0e51cc9 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/JsonOutputFormatterTestBase.cs @@ -100,7 +100,7 @@ public async Task ErrorDuringSerialization_DoesNotCloseTheBrackets() protected static ActionContext GetActionContext( MediaTypeHeaderValue contentType, - MemoryStream responseStream = null) + Stream responseStream = null) { var httpContext = new DefaultHttpContext(); httpContext.Request.ContentType = contentType.ToString(); @@ -115,7 +115,7 @@ protected static OutputFormatterWriteContext GetOutputFormatterContext( object outputValue, Type outputType, string contentType = "application/xml; charset=utf-8", - MemoryStream responseStream = null) + Stream responseStream = null) { var mediaTypeHeaderValue = MediaTypeHeaderValue.Parse(contentType); diff --git a/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs b/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs index b6a64d8b537d..58d6abb3f17c 100644 --- a/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpNotFoundObjectResultTest.cs @@ -2,17 +2,14 @@ // 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.IO; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; -using Newtonsoft.Json; using Xunit; namespace Microsoft.AspNetCore.Mvc @@ -72,9 +69,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs b/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs index b8642a38f453..0c29f4d541f1 100644 --- a/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpOkObjectResultTest.cs @@ -2,18 +2,15 @@ // 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.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.NewtonsoftJson; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; -using Newtonsoft.Json; using Xunit; namespace Microsoft.AspNetCore.Mvc @@ -73,9 +70,7 @@ private static IServiceProvider CreateServices() { var options = Options.Create(new MvcOptions()); options.Value.OutputFormatters.Add(new StringOutputFormatter()); - options.Value.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter( - new JsonSerializerSettings(), - ArrayPool.Shared)); + options.Value.OutputFormatters.Add(new SystemTextJsonOutputFormatter(new MvcOptions())); var services = new ServiceCollection(); services.AddSingleton>(new ObjectResultExecutor( diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs index 48e7a0d15b22..b2a7d486b911 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs @@ -9,8 +9,12 @@ using System.Text; using System.Threading.Tasks; using System.Xml; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters.Xml; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Formatters { @@ -23,6 +27,7 @@ public class XmlDataContractSerializerOutputFormatter : TextOutputFormatter private readonly ConcurrentDictionary _serializerCache = new ConcurrentDictionary(); private readonly ILogger _logger; private DataContractSerializerSettings _serializerSettings; + private MvcOptions _mvcOptions; /// /// Initializes a new instance of @@ -254,24 +259,40 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var dataContractSerializer = GetCachedSerializer(wrappingType); - // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 - var syncIOFeature = context.HttpContext.Features.Get(); - if (syncIOFeature != null) + var httpContext = context.HttpContext; + var response = httpContext.Response; + + _mvcOptions ??= httpContext.RequestServices.GetRequiredService>().Value; + + var responseStream = response.Body; + FileBufferingWriteStream fileBufferingWriteStream = null; + if (!_mvcOptions.SuppressOutputFormatterBuffering) { - syncIOFeature.AllowSynchronousIO = true; + fileBufferingWriteStream = new FileBufferingWriteStream(); + responseStream = fileBufferingWriteStream; } - using (var textWriter = context.WriterFactory(context.HttpContext.Response.Body, writerSettings.Encoding)) + try { - using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) + await using (var textWriter = context.WriterFactory(responseStream, writerSettings.Encoding)) { - dataContractSerializer.WriteObject(xmlWriter, value); + using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) + { + dataContractSerializer.WriteObject(xmlWriter, value); + } } - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous - // write). - await textWriter.FlushAsync(); + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.DrainBufferAsync(response.Body); + } + } + finally + { + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.DisposeAsync(); + } } } diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs index da1812e6e954..fc845f75642b 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs @@ -9,8 +9,12 @@ using System.Threading.Tasks; using System.Xml; using System.Xml.Serialization; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters.Xml; +using Microsoft.AspNetCore.WebUtilities; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Formatters { @@ -22,6 +26,7 @@ public class XmlSerializerOutputFormatter : TextOutputFormatter { private readonly ConcurrentDictionary _serializerCache = new ConcurrentDictionary(); private readonly ILogger _logger; + private MvcOptions _mvcOptions; /// /// Initializes a new instance of @@ -230,24 +235,40 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var xmlSerializer = GetCachedSerializer(wrappingType); - // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 - var syncIOFeature = context.HttpContext.Features.Get(); - if (syncIOFeature != null) + var httpContext = context.HttpContext; + var response = httpContext.Response; + + _mvcOptions ??= httpContext.RequestServices.GetRequiredService>().Value; + + var responseStream = response.Body; + FileBufferingWriteStream fileBufferingWriteStream = null; + if (!_mvcOptions.SuppressOutputFormatterBuffering) { - syncIOFeature.AllowSynchronousIO = true; + fileBufferingWriteStream = new FileBufferingWriteStream(); + responseStream = fileBufferingWriteStream; } - using (var textWriter = context.WriterFactory(context.HttpContext.Response.Body, writerSettings.Encoding)) + try { - using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) + await using (var textWriter = context.WriterFactory(responseStream, selectedEncoding)) { - Serialize(xmlSerializer, xmlWriter, value); + using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) + { + Serialize(xmlSerializer, xmlWriter, value); + } } - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous - // write). - await textWriter.FlushAsync(); + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.DrainBufferAsync(response.Body); + } + } + finally + { + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.DisposeAsync(); + } } } @@ -281,4 +302,4 @@ protected virtual XmlSerializer GetCachedSerializer(Type type) return (XmlSerializer)serializer; } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerOutputFormatterTest.cs b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerOutputFormatterTest.cs index fd33afc14805..d94970348f68 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerOutputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerOutputFormatterTest.cs @@ -10,8 +10,10 @@ using System.Xml; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Testing.xunit; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using Xunit; @@ -735,6 +737,9 @@ private static HttpContext GetHttpContext(string contentType) request.Headers["Accept-Charset"] = MediaTypeHeaderValue.Parse(contentType).Charset.ToString(); request.ContentType = contentType; httpContext.Response.Body = new MemoryStream(); + httpContext.RequestServices = new ServiceCollection() + .AddSingleton(Options.Create(new MvcOptions())) + .BuildServiceProvider(); return httpContext; } diff --git a/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerOutputFormatterTest.cs b/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerOutputFormatterTest.cs index 34bb5e0298fe..fff6d5c062fc 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerOutputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerOutputFormatterTest.cs @@ -10,8 +10,10 @@ using System.Xml; using System.Xml.Serialization; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; using Xunit; @@ -520,6 +522,9 @@ private static HttpContext GetHttpContext(string contentType) request.Headers["Accept-Charset"] = MediaTypeHeaderValue.Parse(contentType).Charset.ToString(); request.ContentType = contentType; httpContext.Response.Body = new MemoryStream(); + httpContext.RequestServices = new ServiceCollection() + .AddSingleton(Options.Create(new MvcOptions())) + .BuildServiceProvider(); return httpContext; } diff --git a/src/Mvc/Mvc.NewtonsoftJson/ref/Microsoft.AspNetCore.Mvc.NewtonsoftJson.netcoreapp3.0.cs b/src/Mvc/Mvc.NewtonsoftJson/ref/Microsoft.AspNetCore.Mvc.NewtonsoftJson.netcoreapp3.0.cs index 066dba3dd51f..94884372e6f2 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/ref/Microsoft.AspNetCore.Mvc.NewtonsoftJson.netcoreapp3.0.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/ref/Microsoft.AspNetCore.Mvc.NewtonsoftJson.netcoreapp3.0.cs @@ -32,7 +32,7 @@ protected virtual void ReleaseJsonSerializer(Newtonsoft.Json.JsonSerializer seri } public partial class NewtonsoftJsonOutputFormatter : Microsoft.AspNetCore.Mvc.Formatters.TextOutputFormatter { - public NewtonsoftJsonOutputFormatter(Newtonsoft.Json.JsonSerializerSettings serializerSettings, System.Buffers.ArrayPool charPool) { } + public NewtonsoftJsonOutputFormatter(Newtonsoft.Json.JsonSerializerSettings serializerSettings, System.Buffers.ArrayPool charPool, Microsoft.AspNetCore.Mvc.MvcOptions mvcOptions) { } protected Newtonsoft.Json.JsonSerializerSettings SerializerSettings { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } protected virtual Newtonsoft.Json.JsonSerializer CreateJsonSerializer() { throw null; } protected virtual Newtonsoft.Json.JsonSerializer CreateJsonSerializer(Microsoft.AspNetCore.Mvc.Formatters.OutputFormatterWriteContext context) { throw null; } diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs index 225385317428..d39ce087cd16 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcCoreBuilderExtensions.cs @@ -102,12 +102,6 @@ internal static void AddServicesCore(IServiceCollection services) } services.TryAddSingleton(); - services.TryAdd(ServiceDescriptor.Singleton(serviceProvider => - { - var options = serviceProvider.GetRequiredService>().Value; - var charPool = serviceProvider.GetRequiredService>(); - return new NewtonsoftJsonOutputFormatter(options.SerializerSettings, charPool); - })); } } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcOptionsSetup.cs b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcOptionsSetup.cs index c466d8d11416..fca499d1fd18 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcOptionsSetup.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/DependencyInjection/NewtonsoftJsonMvcOptionsSetup.cs @@ -60,7 +60,7 @@ public NewtonsoftJsonMvcOptionsSetup( public void Configure(MvcOptions options) { options.OutputFormatters.RemoveType(); - options.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter(_jsonOptions.SerializerSettings, _charPool)); + options.OutputFormatters.Add(new NewtonsoftJsonOutputFormatter(_jsonOptions.SerializerSettings, _charPool, options)); options.InputFormatters.RemoveType(); // Register JsonPatchInputFormatter before JsonInputFormatter, otherwise diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs index b4ccaa171ff7..9bff0e56df9c 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/JsonResultExecutor.cs @@ -3,10 +3,13 @@ using System; using System.Buffers; +using System.IO; using System.Text; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Net.Http.Headers; @@ -26,7 +29,8 @@ internal class JsonResultExecutor : IActionResultExecutor private readonly IHttpResponseStreamWriterFactory _writerFactory; private readonly ILogger _logger; - private readonly MvcNewtonsoftJsonOptions _options; + private readonly MvcOptions _mvcOptions; + private readonly MvcNewtonsoftJsonOptions _jsonOptions; private readonly IArrayPool _charPool; /// @@ -34,12 +38,14 @@ internal class JsonResultExecutor : IActionResultExecutor /// /// The . /// The . - /// The . + /// Accessor to . + /// Accessor to . /// The for creating buffers. public JsonResultExecutor( IHttpResponseStreamWriterFactory writerFactory, ILogger logger, - IOptions options, + IOptions mvcOptions, + IOptions jsonOptions, ArrayPool charPool) { if (writerFactory == null) @@ -52,9 +58,9 @@ public JsonResultExecutor( throw new ArgumentNullException(nameof(logger)); } - if (options == null) + if (jsonOptions == null) { - throw new ArgumentNullException(nameof(options)); + throw new ArgumentNullException(nameof(jsonOptions)); } if (charPool == null) @@ -64,7 +70,8 @@ public JsonResultExecutor( _writerFactory = writerFactory; _logger = logger; - _options = options.Value; + _mvcOptions = mvcOptions?.Value ?? throw new ArgumentNullException(nameof(mvcOptions)); + _jsonOptions = jsonOptions.Value; _charPool = new JsonArrayPool(charPool); } @@ -105,10 +112,20 @@ public async Task ExecuteAsync(ActionContext context, JsonResult result) } _logger.JsonResultExecuting(result.Value); - using (var writer = _writerFactory.CreateWriter(response.Body, resolvedContentTypeEncoding)) + + var responseStream = response.Body; + FileBufferingWriteStream fileBufferingWriteStream = null; + if (!_mvcOptions.SuppressOutputFormatterBuffering) { - using (var jsonWriter = new JsonTextWriter(writer)) + fileBufferingWriteStream = new FileBufferingWriteStream(); + responseStream = fileBufferingWriteStream; + } + + try + { + await using (var writer = _writerFactory.CreateWriter(responseStream, resolvedContentTypeEncoding)) { + using var jsonWriter = new JsonTextWriter(writer); jsonWriter.ArrayPool = _charPool; jsonWriter.CloseOutput = false; jsonWriter.AutoCompleteOnClose = false; @@ -117,9 +134,17 @@ public async Task ExecuteAsync(ActionContext context, JsonResult result) jsonSerializer.Serialize(jsonWriter, result.Value); } - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous write). - await writer.FlushAsync(); + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.DrainBufferAsync(response.Body); + } + } + finally + { + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.DisposeAsync(); + } } } @@ -128,9 +153,9 @@ private JsonSerializerSettings GetSerializerSettings(JsonResult result) var serializerSettings = result.SerializerSettings; if (serializerSettings == null) { - return _options.SerializerSettings; + return _jsonOptions.SerializerSettings; } - else + else { if (!(serializerSettings is JsonSerializerSettings settingsFromResult)) { diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs index 934cf824ff2e..80a1fc3b7dee 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonOutputFormatter.cs @@ -6,7 +6,9 @@ using System.IO; using System.Text; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.NewtonsoftJson; +using Microsoft.AspNetCore.WebUtilities; using Newtonsoft.Json; namespace Microsoft.AspNetCore.Mvc.Formatters @@ -17,6 +19,7 @@ namespace Microsoft.AspNetCore.Mvc.Formatters public class NewtonsoftJsonOutputFormatter : TextOutputFormatter { private readonly IArrayPool _charPool; + private readonly MvcOptions _mvcOptions; // Perf: JsonSerializers are relatively expensive to create, and are thread safe. We cache // the serializer and invalidate it when the settings change. @@ -31,7 +34,11 @@ public class NewtonsoftJsonOutputFormatter : TextOutputFormatter /// initially returned. /// /// The . - public NewtonsoftJsonOutputFormatter(JsonSerializerSettings serializerSettings, ArrayPool charPool) + /// The . + public NewtonsoftJsonOutputFormatter( + JsonSerializerSettings serializerSettings, + ArrayPool charPool, + MvcOptions mvcOptions) { if (serializerSettings == null) { @@ -45,6 +52,7 @@ public NewtonsoftJsonOutputFormatter(JsonSerializerSettings serializerSettings, SerializerSettings = serializerSettings; _charPool = new JsonArrayPool(charPool); + _mvcOptions = mvcOptions ?? throw new ArgumentNullException(nameof(mvcOptions)); SupportedEncodings.Add(Encoding.UTF8); SupportedEncodings.Add(Encoding.Unicode); @@ -123,26 +131,38 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co throw new ArgumentNullException(nameof(selectedEncoding)); } - // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 - var syncIOFeature = context.HttpContext.Features.Get(); - if (syncIOFeature != null) + var response = context.HttpContext.Response; + + var responseStream = response.Body; + FileBufferingWriteStream fileBufferingWriteStream = null; + if (!_mvcOptions.SuppressOutputFormatterBuffering) { - syncIOFeature.AllowSynchronousIO = true; + fileBufferingWriteStream = new FileBufferingWriteStream(); + responseStream = fileBufferingWriteStream; } - var response = context.HttpContext.Response; - using (var writer = context.WriterFactory(response.Body, selectedEncoding)) + try { - using (var jsonWriter = CreateJsonWriter(writer)) + await using (var writer = context.WriterFactory(responseStream, selectedEncoding)) { - var jsonSerializer = CreateJsonSerializer(context); - jsonSerializer.Serialize(jsonWriter, context.Object); + using (var jsonWriter = CreateJsonWriter(writer)) + { + var jsonSerializer = CreateJsonSerializer(context); + jsonSerializer.Serialize(jsonWriter, context.Object); + } } - // Perf: call FlushAsync to call WriteAsync on the stream with any content left in the TextWriter's - // buffers. This is better than just letting dispose handle it (which would result in a synchronous - // write). - await writer.FlushAsync(); + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.DrainBufferAsync(response.Body); + } + } + finally + { + if (fileBufferingWriteStream != null) + { + await fileBufferingWriteStream.DisposeAsync(); + } } } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs index 628916dd25f0..72e030ba6c80 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultExecutorTest.cs @@ -3,7 +3,9 @@ using System; using System.Buffers; +using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -161,10 +163,9 @@ public async Task ExecuteAsync_UsesPassedInSerializerSettings() } [Fact] - public async Task ExecuteAsync_ErrorDuringSerialization_DoesNotCloseTheBrackets() + public async Task ExecuteAsync_ErrorDuringSerialization_DoesNotWriteContent() { // Arrange - var expected = Encoding.UTF8.GetBytes("{\"name\":\"Robert\""); var context = GetActionContext(); var result = new JsonResult(new ModelWithSerializationError()); var executor = CreateExecutor(); @@ -182,7 +183,7 @@ public async Task ExecuteAsync_ErrorDuringSerialization_DoesNotCloseTheBrackets( // Assert var written = GetWrittenBytes(context.HttpContext); - Assert.Equal(expected, written); + Assert.Empty(written); } [Fact] @@ -220,63 +221,29 @@ public async Task ExecuteAsync_NullResult_LogsNull() } [Fact] - public async Task ExecuteAsync_WritesToTheResponseStream_WhenContentIsLargerThanBuffer() + public async Task ExecuteAsync_LargePayload_DoesNotPerformSynchronousWrites() { // Arrange - var writeLength = 2 * TestHttpResponseStreamWriterFactory.DefaultBufferSize + 4; - var text = new string('a', writeLength); - var expectedWriteCallCount = Math.Ceiling((double)writeLength / TestHttpResponseStreamWriterFactory.DefaultBufferSize); + var model = Enumerable.Range(0, 1000).Select(p => new TestModel { Property = new string('a', 5000)}); - var stream = new Mock(); + var stream = new Mock { CallBase = true }; + stream.Setup(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); stream.SetupGet(s => s.CanWrite).Returns(true); - var httpContext = new DefaultHttpContext(); - httpContext.Response.Body = stream.Object; - var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var context = GetActionContext(); + context.HttpContext.Response.Body = stream.Object; - var result = new JsonResult(text); var executor = CreateExecutor(); + var result = new JsonResult(model); // Act - await executor.ExecuteAsync(actionContext, result); + await executor.ExecuteAsync(context, result); // Assert - // HttpResponseStreamWriter buffers content up to the buffer size (16k). When writes exceed the buffer size, it'll perform a synchronous - // write to the response stream. - stream.Verify(s => s.Write(It.IsAny(), It.IsAny(), TestHttpResponseStreamWriterFactory.DefaultBufferSize), Times.Exactly(2)); - - // Remainder buffered content is written asynchronously as part of the FlushAsync. - stream.Verify(s => s.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); - - // Dispose does not call Flush - stream.Verify(s => s.Flush(), Times.Never()); - } - - [Theory] - [InlineData(5)] - [InlineData(TestHttpResponseStreamWriterFactory.DefaultBufferSize - 30)] - public async Task ExecuteAsync_DoesNotWriteSynchronouslyToTheResponseBody_WhenContentIsSmallerThanBufferSize(int writeLength) - { - // Arrange - var text = new string('a', writeLength); + stream.Verify(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.AtLeastOnce()); - var stream = new Mock(); - stream.SetupGet(s => s.CanWrite).Returns(true); - var httpContext = new DefaultHttpContext(); - httpContext.Response.Body = stream.Object; - var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); - - var result = new JsonResult(text); - var executor = CreateExecutor(); - - // Act - await executor.ExecuteAsync(actionContext, result); - - // Assert - // HttpResponseStreamWriter buffers content up to the buffer size (16k) and will asynchronously write content to the response as part - // of the FlushAsync call if the content written to it is smaller than the buffer size. - // This test verifies that no synchronous writes are performed in this scenario. - stream.Verify(s => s.Flush(), Times.Never()); - stream.Verify(s => s.Write(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); + stream.Verify(v => v.Write(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); + stream.Verify(v => v.Flush(), Times.Never()); } private static JsonResultExecutor CreateExecutor(ILogger logger = null) @@ -284,6 +251,7 @@ private static JsonResultExecutor CreateExecutor(ILogger log return new JsonResultExecutor( new TestHttpResponseStreamWriterFactory(), logger ?? NullLogger.Instance, + Options.Create(new MvcOptions()), Options.Create(new MvcNewtonsoftJsonOptions()), ArrayPool.Shared); } @@ -337,5 +305,10 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except MostRecentMessage = formatter(state, exception); } } + + private class TestModel + { + public string Property { get; set; } + } } } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs index dca9a521643a..bc1c0869ee10 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/JsonResultTest.cs @@ -47,6 +47,7 @@ private static HttpContext GetHttpContext() var executor = new JsonResultExecutor( new TestHttpResponseStreamWriterFactory(), NullLogger.Instance, + Options.Create(new MvcOptions()), Options.Create(new MvcNewtonsoftJsonOptions()), ArrayPool.Shared); diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs index e5cce46201ca..a04c79c63821 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonOutputFormatterTest.cs @@ -5,8 +5,11 @@ using System.Buffers; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text; +using System.Threading; using System.Threading.Tasks; +using Moq; using Newtonsoft.Json; using Newtonsoft.Json.Linq; using Newtonsoft.Json.Serialization; @@ -18,7 +21,7 @@ public class NewtonsoftJsonOutputFormatterTest : JsonOutputFormatterTestBase { protected override TextOutputFormatter GetOutputFormatter() { - return new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared); + return new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared, new MvcOptions()); } [Fact] @@ -56,7 +59,7 @@ public async Task ChangesTo_SerializerSettings_AffectSerialization() Formatting = Formatting.Indented, }; var expectedOutput = JsonConvert.SerializeObject(person, settings); - var jsonFormatter = new NewtonsoftJsonOutputFormatter(settings, ArrayPool.Shared); + var jsonFormatter = new NewtonsoftJsonOutputFormatter(settings, ArrayPool.Shared, new MvcOptions()); // Act await jsonFormatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.UTF8); @@ -274,8 +277,7 @@ public async Task WriteToStreamAsync_RoundTripsJToken() { // Arrange var beforeMessage = "Hello World"; - var formatter = new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared); - var before = new JValue(beforeMessage); + var formatter = new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared, new MvcOptions()); var memStream = new MemoryStream(); var outputFormatterContext = GetOutputFormatterContext( beforeMessage, @@ -294,10 +296,38 @@ public async Task WriteToStreamAsync_RoundTripsJToken() Assert.Equal(beforeMessage, afterMessage); } + [Fact] + public async Task WriteToStreamAsync_LargePayload_DoesNotPerformSynchronousWrites() + { + // Arrange + var model = Enumerable.Range(0, 1000).Select(p => new User { FullName = new string('a', 5000) }); + + var stream = new Mock { CallBase = true }; + stream.Setup(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + stream.SetupGet(s => s.CanWrite).Returns(true); + + var formatter = new NewtonsoftJsonOutputFormatter(new JsonSerializerSettings(), ArrayPool.Shared, new MvcOptions()); + var outputFormatterContext = GetOutputFormatterContext( + model, + typeof(string), + "application/json; charset=utf-8", + stream.Object); + + // Act + await formatter.WriteResponseBodyAsync(outputFormatterContext, Encoding.UTF8); + + // Assert + stream.Verify(v => v.WriteAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.AtLeastOnce()); + + stream.Verify(v => v.Write(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); + stream.Verify(v => v.Flush(), Times.Never()); + } + private class TestableJsonOutputFormatter : NewtonsoftJsonOutputFormatter { public TestableJsonOutputFormatter(JsonSerializerSettings serializerSettings) - : base(serializerSettings, ArrayPool.Shared) + : base(serializerSettings, ArrayPool.Shared, new MvcOptions()) { } @@ -331,5 +361,5 @@ private class UserWithJsonObject public string FullName { get; set; } } - } + } } \ No newline at end of file diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/NormalController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/NormalController.cs index 19150c264feb..a86ff9919ab3 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/NormalController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContentNegotiation/NormalController.cs @@ -25,7 +25,7 @@ static NormalController() public NormalController(ArrayPool charPool) { - _indentingFormatter = new NewtonsoftJsonOutputFormatter(_indentedSettings, charPool); + _indentingFormatter = new NewtonsoftJsonOutputFormatter(_indentedSettings, charPool, new MvcOptions()); } public override void OnActionExecuted(ActionExecutedContext context) diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs index 48e29ca041d7..0ebaf0c3242c 100644 --- a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs +++ b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs @@ -23,7 +23,7 @@ static JsonFormatterController() public JsonFormatterController(ArrayPool charPool) { - _indentingFormatter = new NewtonsoftJsonOutputFormatter(_indentedSettings, charPool); + _indentingFormatter = new NewtonsoftJsonOutputFormatter(_indentedSettings, charPool, new MvcOptions()); } public IActionResult ReturnsIndentedJson()