Skip to content

Ensure EnableBuffering works with NewtonsoftJsonInputFormatter #14870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.Extensions.Logging.Testing;
using Microsoft.AspNetCore.WebUtilities;
using Newtonsoft.Json;
using Moq;
using Xunit;

namespace Microsoft.AspNetCore.Mvc.Formatters
Expand Down Expand Up @@ -462,6 +464,30 @@ public virtual async Task ReadAsync_RequiredAttribute()
Assert.Single(formatterContext.ModelState["Person.Name"].Errors);
}

[Fact]
public async Task ReadAsync_DoesNotDisposeBufferedReadStream()
{
// Arrange
var formatter = GetInputFormatter();

var content = "{\"name\": \"Test\"}";
var contentBytes = Encoding.UTF8.GetBytes(content);
var httpContext = GetHttpContext(contentBytes);
var testBufferedReadStream = new Mock<FileBufferingReadStream>(httpContext.Request.Body, 1024) { CallBase = true };
httpContext.Request.Body = testBufferedReadStream.Object;

var formatterContext = CreateInputFormatterContext(typeof(ComplexModel), httpContext);

// Act
var result = await formatter.ReadAsync(formatterContext);

// Assert
var userModel = Assert.IsType<ComplexModel>(result.Model);
Assert.Equal("Test", userModel.Name);

testBufferedReadStream.Verify(v => v.DisposeAsync(), Times.Never());
}

internal abstract string JsonFormatter_EscapedKeys_Bracket_Expected { get; }

internal abstract string JsonFormatter_EscapedKeys_Expected { get; }
Expand Down Expand Up @@ -517,7 +543,7 @@ protected static InputFormatterContext CreateInputFormatterContext(
protected sealed class ComplexPoco
{
public int Id { get; set; }
public Person Person{ get; set; }
public Person Person { get; set; }
}

protected sealed class Person
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(InputForma

var request = context.HttpContext.Request;
Stream readStream = new NonDisposableStream(request.Body);
var disposeReadStream = false;

if (!request.Body.CanSeek && !_options.SuppressInputFormatterBuffering)
{
Expand All @@ -135,6 +136,8 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(InputForma

await readStream.DrainAsync(CancellationToken.None);
readStream.Seek(0L, SeekOrigin.Begin);

disposeReadStream = true;
}

try
Expand Down Expand Up @@ -162,9 +165,9 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(InputForma
}
finally
{
if (readStream is FileBufferingReadStream fileBufferingReadStream)
if (disposeReadStream)
{
await fileBufferingReadStream.DisposeAsync();
await readStream.DisposeAsync();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(

var request = context.HttpContext.Request;
Stream readStream = new NonDisposableStream(request.Body);
var disposeReadStream = false;

if (!request.Body.CanSeek && !_options.SuppressInputFormatterBuffering)
{
// XmlSerializer does synchronous reads. In order to avoid blocking on the stream, we asynchronously
Expand All @@ -115,6 +117,7 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(

await readStream.DrainAsync(CancellationToken.None);
readStream.Seek(0L, SeekOrigin.Begin);
disposeReadStream = true;
}

try
Expand Down Expand Up @@ -155,9 +158,9 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(
}
finally
{
if (readStream is FileBufferingReadStream fileBufferingReadStream)
if (disposeReadStream)
{
await fileBufferingReadStream.DisposeAsync();
await readStream.DisposeAsync();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.AspNetCore.WebUtilities;
using Moq;
using Xunit;

Expand Down Expand Up @@ -166,6 +167,39 @@ public async Task BuffersRequestBody_ByDefault()
Assert.Equal(expectedString, model.sampleString);
}

[Fact]
public async Task ReadAsync_DoesNotDisposeBufferedStreamIfItDidNotCreateIt()
{
// Arrange
var expectedInt = 10;
var expectedString = "TestString";

var input = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" +
"<TestLevelOne><SampleInt>" + expectedInt + "</SampleInt>" +
"<sampleString>" + expectedString + "</sampleString></TestLevelOne>";

var formatter = new XmlDataContractSerializerInputFormatter(new MvcOptions());

var contentBytes = Encoding.UTF8.GetBytes(input);
var httpContext = new DefaultHttpContext();
var testBufferedReadStream = new Mock<FileBufferingReadStream>(new MemoryStream(contentBytes), 1024) { CallBase = true };
httpContext.Request.Body = testBufferedReadStream.Object;
var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne));

// Act
var result = await formatter.ReadAsync(context);

// Assert
Assert.NotNull(result);
Assert.False(result.HasError);
var model = Assert.IsType<TestLevelOne>(result.Model);

Assert.Equal(expectedInt, model.SampleInt);
Assert.Equal(expectedString, model.sampleString);

testBufferedReadStream.Verify(v => v.DisposeAsync(), Times.Never());
}

[Fact]
public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestBody()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.AspNetCore.Testing;
using Microsoft.AspNetCore.WebUtilities;
using Moq;
using Xunit;

Expand Down Expand Up @@ -622,6 +623,39 @@ public async Task ReadAsync_AcceptsUTF16Characters()
Assert.Equal(XmlConvert.ToDateTime(expectedDateTime, XmlDateTimeSerializationMode.Utc), model.SampleDate);
}

[Fact]
public async Task ReadAsync_DoesNotDisposeBufferedStreamIfItDidNotCreateIt()
{
// Arrange
var expectedInt = 10;
var expectedString = "TestString";

var input = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" +
"<TestLevelOne><SampleInt>" + expectedInt + "</SampleInt>" +
"<sampleString>" + expectedString + "</sampleString></TestLevelOne>";

var formatter = new XmlSerializerInputFormatter(new MvcOptions());

var contentBytes = Encoding.UTF8.GetBytes(input);
var httpContext = new DefaultHttpContext();
var testBufferedReadStream = new Mock<FileBufferingReadStream>(new MemoryStream(contentBytes), 1024) { CallBase = true };
httpContext.Request.Body = testBufferedReadStream.Object;
var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne));

// Act
var result = await formatter.ReadAsync(context);

// Assert
Assert.NotNull(result);
Assert.False(result.HasError);
var model = Assert.IsType<TestLevelOne>(result.Model);

Assert.Equal(expectedInt, model.SampleInt);
Assert.Equal(expectedString, model.sampleString);

testBufferedReadStream.Verify(v => v.DisposeAsync(), Times.Never());
}

private InputFormatterContext GetInputFormatterContext(byte[] contentBytes, Type modelType)
{
var httpContext = GetHttpContext(contentBytes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(
var suppressInputFormatterBuffering = _options.SuppressInputFormatterBuffering;

var readStream = request.Body;
var disposeReadStream = false;
if (!request.Body.CanSeek && !suppressInputFormatterBuffering)
{
// JSON.Net does synchronous reads. In order to avoid blocking on the stream, we asynchronously
Expand All @@ -145,6 +146,8 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(

await readStream.DrainAsync(CancellationToken.None);
readStream.Seek(0L, SeekOrigin.Begin);

disposeReadStream = true;
}

var successful = true;
Expand All @@ -170,9 +173,9 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(
jsonSerializer.Error -= ErrorHandler;
ReleaseJsonSerializer(jsonSerializer);

if (readStream is FileBufferingReadStream fileBufferingReadStream)
if (disposeReadStream)
{
await fileBufferingReadStream.DisposeAsync();
await readStream.DisposeAsync();
}
}
}
Expand Down