Skip to content

Commit fcc20ac

Browse files
pranavkmpranavkm
authored
Ensure EnableBuffering works with NewtonsoftJsonInputFormatter (#14870)
Fixes #14396
1 parent 1377ced commit fcc20ac

File tree

6 files changed

+110
-7
lines changed

6 files changed

+110
-7
lines changed

‎src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
using Microsoft.AspNetCore.Http;
1313
using Microsoft.AspNetCore.Mvc.ModelBinding;
1414
using Microsoft.Extensions.Logging.Testing;
15+
using Microsoft.AspNetCore.WebUtilities;
1516
using Newtonsoft.Json;
17+
using Moq;
1618
using Xunit;
1719

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

467+
[Fact]
468+
public async Task ReadAsync_DoesNotDisposeBufferedReadStream()
469+
{
470+
// Arrange
471+
var formatter = GetInputFormatter();
472+
473+
var content = "{\"name\": \"Test\"}";
474+
var contentBytes = Encoding.UTF8.GetBytes(content);
475+
var httpContext = GetHttpContext(contentBytes);
476+
var testBufferedReadStream = new Mock<FileBufferingReadStream>(httpContext.Request.Body, 1024) { CallBase = true };
477+
httpContext.Request.Body = testBufferedReadStream.Object;
478+
479+
var formatterContext = CreateInputFormatterContext(typeof(ComplexModel), httpContext);
480+
481+
// Act
482+
var result = await formatter.ReadAsync(formatterContext);
483+
484+
// Assert
485+
var userModel = Assert.IsType<ComplexModel>(result.Model);
486+
Assert.Equal("Test", userModel.Name);
487+
488+
testBufferedReadStream.Verify(v => v.DisposeAsync(), Times.Never());
489+
}
490+
465491
internal abstract string JsonFormatter_EscapedKeys_Bracket_Expected { get; }
466492

467493
internal abstract string JsonFormatter_EscapedKeys_Expected { get; }
@@ -517,7 +543,7 @@ protected static InputFormatterContext CreateInputFormatterContext(
517543
protected sealed class ComplexPoco
518544
{
519545
public int Id { get; set; }
520-
public Person Person{ get; set; }
546+
public Person Person { get; set; }
521547
}
522548

523549
protected sealed class Person

‎src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerInputFormatter.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(InputForma
118118

119119
var request = context.HttpContext.Request;
120120
Stream readStream = new NonDisposableStream(request.Body);
121+
var disposeReadStream = false;
121122

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

136137
await readStream.DrainAsync(CancellationToken.None);
137138
readStream.Seek(0L, SeekOrigin.Begin);
139+
140+
disposeReadStream = true;
138141
}
139142

140143
try
@@ -162,9 +165,9 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(InputForma
162165
}
163166
finally
164167
{
165-
if (readStream is FileBufferingReadStream fileBufferingReadStream)
168+
if (disposeReadStream)
166169
{
167-
await fileBufferingReadStream.DisposeAsync();
170+
await readStream.DisposeAsync();
168171
}
169172
}
170173
}

‎src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerInputFormatter.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(
9999

100100
var request = context.HttpContext.Request;
101101
Stream readStream = new NonDisposableStream(request.Body);
102+
var disposeReadStream = false;
103+
102104
if (!request.Body.CanSeek && !_options.SuppressInputFormatterBuffering)
103105
{
104106
// XmlSerializer does synchronous reads. In order to avoid blocking on the stream, we asynchronously
@@ -115,6 +117,7 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(
115117

116118
await readStream.DrainAsync(CancellationToken.None);
117119
readStream.Seek(0L, SeekOrigin.Begin);
120+
disposeReadStream = true;
118121
}
119122

120123
try
@@ -155,9 +158,9 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(
155158
}
156159
finally
157160
{
158-
if (readStream is FileBufferingReadStream fileBufferingReadStream)
161+
if (disposeReadStream)
159162
{
160-
await fileBufferingReadStream.DisposeAsync();
163+
await readStream.DisposeAsync();
161164
}
162165
}
163166
}

‎src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Microsoft.AspNetCore.Http;
1313
using Microsoft.AspNetCore.Http.Features;
1414
using Microsoft.AspNetCore.Mvc.ModelBinding;
15+
using Microsoft.AspNetCore.WebUtilities;
1516
using Moq;
1617
using Xunit;
1718

@@ -166,6 +167,39 @@ public async Task BuffersRequestBody_ByDefault()
166167
Assert.Equal(expectedString, model.sampleString);
167168
}
168169

170+
[Fact]
171+
public async Task ReadAsync_DoesNotDisposeBufferedStreamIfItDidNotCreateIt()
172+
{
173+
// Arrange
174+
var expectedInt = 10;
175+
var expectedString = "TestString";
176+
177+
var input = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" +
178+
"<TestLevelOne><SampleInt>" + expectedInt + "</SampleInt>" +
179+
"<sampleString>" + expectedString + "</sampleString></TestLevelOne>";
180+
181+
var formatter = new XmlDataContractSerializerInputFormatter(new MvcOptions());
182+
183+
var contentBytes = Encoding.UTF8.GetBytes(input);
184+
var httpContext = new DefaultHttpContext();
185+
var testBufferedReadStream = new Mock<FileBufferingReadStream>(new MemoryStream(contentBytes), 1024) { CallBase = true };
186+
httpContext.Request.Body = testBufferedReadStream.Object;
187+
var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne));
188+
189+
// Act
190+
var result = await formatter.ReadAsync(context);
191+
192+
// Assert
193+
Assert.NotNull(result);
194+
Assert.False(result.HasError);
195+
var model = Assert.IsType<TestLevelOne>(result.Model);
196+
197+
Assert.Equal(expectedInt, model.SampleInt);
198+
Assert.Equal(expectedString, model.sampleString);
199+
200+
testBufferedReadStream.Verify(v => v.DisposeAsync(), Times.Never());
201+
}
202+
169203
[Fact]
170204
public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestBody()
171205
{

‎src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using Microsoft.AspNetCore.Http.Features;
1414
using Microsoft.AspNetCore.Mvc.ModelBinding;
1515
using Microsoft.AspNetCore.Testing;
16+
using Microsoft.AspNetCore.WebUtilities;
1617
using Moq;
1718
using Xunit;
1819

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

626+
[Fact]
627+
public async Task ReadAsync_DoesNotDisposeBufferedStreamIfItDidNotCreateIt()
628+
{
629+
// Arrange
630+
var expectedInt = 10;
631+
var expectedString = "TestString";
632+
633+
var input = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" +
634+
"<TestLevelOne><SampleInt>" + expectedInt + "</SampleInt>" +
635+
"<sampleString>" + expectedString + "</sampleString></TestLevelOne>";
636+
637+
var formatter = new XmlSerializerInputFormatter(new MvcOptions());
638+
639+
var contentBytes = Encoding.UTF8.GetBytes(input);
640+
var httpContext = new DefaultHttpContext();
641+
var testBufferedReadStream = new Mock<FileBufferingReadStream>(new MemoryStream(contentBytes), 1024) { CallBase = true };
642+
httpContext.Request.Body = testBufferedReadStream.Object;
643+
var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne));
644+
645+
// Act
646+
var result = await formatter.ReadAsync(context);
647+
648+
// Assert
649+
Assert.NotNull(result);
650+
Assert.False(result.HasError);
651+
var model = Assert.IsType<TestLevelOne>(result.Model);
652+
653+
Assert.Equal(expectedInt, model.SampleInt);
654+
Assert.Equal(expectedString, model.sampleString);
655+
656+
testBufferedReadStream.Verify(v => v.DisposeAsync(), Times.Never());
657+
}
658+
625659
private InputFormatterContext GetInputFormatterContext(byte[] contentBytes, Type modelType)
626660
{
627661
var httpContext = GetHttpContext(contentBytes);

‎src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ public override async Task<InputFormatterResult> ReadRequestBodyAsync(
129129
var suppressInputFormatterBuffering = _options.SuppressInputFormatterBuffering;
130130

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

146147
await readStream.DrainAsync(CancellationToken.None);
147148
readStream.Seek(0L, SeekOrigin.Begin);
149+
150+
disposeReadStream = true;
148151
}
149152

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

173-
if (readStream is FileBufferingReadStream fileBufferingReadStream)
176+
if (disposeReadStream)
174177
{
175-
await fileBufferingReadStream.DisposeAsync();
178+
await readStream.DisposeAsync();
176179
}
177180
}
178181
}

0 commit comments

Comments
 (0)