From a168e50d121237d7b9149e72d4f3fd7249d92a9e Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 29 Oct 2019 13:51:23 -0700 Subject: [PATCH] Allow EnableBuffering + Json.NET \ Xml input formatters to work better (#16616) * Allow EnableBuffering + Json.NET \ Xml input formatters to work better With EnableBufering, using Newtonsoft.Json or a XML input formatter will throw a sync IO exception by default. Instead actively drain the stream before deserializing the content. Fixes https://github.com/aspnet/AspNetCore/issues/16615 --- .../Formatters/JsonInputFormatterTestBase.cs | 49 +++++++++++++++++++ ...XmlDataContractSerializerInputFormatter.cs | 12 ++++- .../src/XmlSerializerInputFormatter.cs | 12 ++++- .../src/NewtonsoftJsonInputFormatter.cs | 12 ++++- 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs b/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs index fd08c4978e..3057701a72 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs @@ -489,6 +489,55 @@ namespace Microsoft.AspNetCore.Mvc.Formatters testBufferedReadStream.Verify(v => v.DisposeAsync(), Times.Never()); } + [Fact] + public async Task ReadAsync_WithEnableBufferingWorks() + { + // Arrange + var formatter = GetInputFormatter(); + + var content = "{\"name\": \"Test\"}"; + var contentBytes = Encoding.UTF8.GetBytes(content); + var httpContext = GetHttpContext(contentBytes); + httpContext.Request.EnableBuffering(); + + var formatterContext = CreateInputFormatterContext(typeof(ComplexModel), httpContext); + + // Act + var result = await formatter.ReadAsync(formatterContext); + + // Assert + var userModel = Assert.IsType(result.Model); + Assert.Equal("Test", userModel.Name); + var requestBody = httpContext.Request.Body; + requestBody.Position = 0; + Assert.Equal(content, new StreamReader(requestBody).ReadToEnd()); + } + + [Fact] + public async Task ReadAsync_WithEnableBufferingWorks_WithInputStreamAtOffset() + { + // Arrange + var formatter = GetInputFormatter(); + + var content = "abc{\"name\": \"Test\"}"; + var contentBytes = Encoding.UTF8.GetBytes(content); + var httpContext = GetHttpContext(contentBytes); + httpContext.Request.EnableBuffering(); + var requestBody = httpContext.Request.Body; + requestBody.Position = 3; + + var formatterContext = CreateInputFormatterContext(typeof(ComplexModel), httpContext); + + // Act + var result = await formatter.ReadAsync(formatterContext); + + // Assert + var userModel = Assert.IsType(result.Model); + Assert.Equal("Test", userModel.Name); + requestBody.Position = 0; + Assert.Equal(content, new StreamReader(requestBody).ReadToEnd()); + } + internal abstract string JsonFormatter_EscapedKeys_Bracket_Expected { get; } internal abstract string JsonFormatter_EscapedKeys_Expected { get; } diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerInputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerInputFormatter.cs index e23a508053..8132dfdba8 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerInputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerInputFormatter.cs @@ -120,7 +120,17 @@ namespace Microsoft.AspNetCore.Mvc.Formatters Stream readStream = new NonDisposableStream(request.Body); var disposeReadStream = false; - if (!request.Body.CanSeek && !_options.SuppressInputFormatterBuffering) + if (readStream.CanSeek) + { + // The most common way of getting here is the user has request buffering on. + // However, request buffering isn't eager, and consequently it will peform pass-thru synchronous + // reads as part of the deserialization. + // To avoid this, drain and reset the stream. + var position = request.Body.Position; + await readStream.DrainAsync(CancellationToken.None); + readStream.Position = position; + } + else if (!_options.SuppressInputFormatterBuffering) { // XmlDataContractSerializer does synchronous reads. In order to avoid blocking on the stream, we asynchronously // read everything into a buffer, and then seek back to the beginning. diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerInputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerInputFormatter.cs index 78e592370b..ab3abdf801 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerInputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerInputFormatter.cs @@ -101,7 +101,17 @@ namespace Microsoft.AspNetCore.Mvc.Formatters Stream readStream = new NonDisposableStream(request.Body); var disposeReadStream = false; - if (!request.Body.CanSeek && !_options.SuppressInputFormatterBuffering) + if (readStream.CanSeek) + { + // The most common way of getting here is the user has request buffering on. + // However, request buffering isn't eager, and consequently it will peform pass-thru synchronous + // reads as part of the deserialization. + // To avoid this, drain and reset the stream. + var position = request.Body.Position; + await readStream.DrainAsync(CancellationToken.None); + readStream.Position = position; + } + else if (!_options.SuppressInputFormatterBuffering) { // XmlSerializer does synchronous reads. In order to avoid blocking on the stream, we asynchronously // read everything into a buffer, and then seek back to the beginning. diff --git a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs index 2138c5b117..ac26b61911 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs @@ -130,7 +130,17 @@ namespace Microsoft.AspNetCore.Mvc.Formatters var readStream = request.Body; var disposeReadStream = false; - if (!request.Body.CanSeek && !suppressInputFormatterBuffering) + if (readStream.CanSeek) + { + // The most common way of getting here is the user has request buffering on. + // However, request buffering isn't eager, and consequently it will peform pass-thru synchronous + // reads as part of the deserialization. + // To avoid this, drain and reset the stream. + var position = request.Body.Position; + await readStream.DrainAsync(CancellationToken.None); + readStream.Position = position; + } + else if (!suppressInputFormatterBuffering) { // JSON.Net does synchronous reads. In order to avoid blocking on the stream, we asynchronously // read everything into a buffer, and then seek back to the beginning.