Prevent ODE when response body isn't reset (#25779)

This commit is contained in:
Stephen Halter 2020-09-10 18:14:08 -07:00 committed by GitHub
parent 5aa4a7cd41
commit 327d55d01c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 39 additions and 6 deletions

View File

@ -70,21 +70,17 @@ namespace Microsoft.AspNetCore.Http
set
{
var otherFeature = _features.Collection.Get<IHttpResponseBodyFeature>();
if (otherFeature is StreamResponseBodyFeature streamFeature
&& streamFeature.PriorFeature != null
&& object.ReferenceEquals(value, streamFeature.PriorFeature.Stream))
{
// They're reverting the stream back to the prior one. Revert the whole feature.
_features.Collection.Set(streamFeature.PriorFeature);
// CompleteAsync is registered with HttpResponse.OnCompleted and there's no way to unregister it.
// Prevent it from running by marking as disposed.
streamFeature.Dispose();
return;
}
var feature = new StreamResponseBodyFeature(value, otherFeature);
OnCompleted(feature.CompleteAsync);
_features.Collection.Set<IHttpResponseBodyFeature>(feature);
_features.Collection.Set<IHttpResponseBodyFeature>(new StreamResponseBodyFeature(value, otherFeature));
}
}

View File

@ -73,6 +73,43 @@ namespace Microsoft.AspNetCore.Http
Assert.NotNull(bodyPipe);
}
[Fact]
public void ReplacingResponseBody_DoesNotCreateOnCompletedRegistration()
{
var features = new FeatureCollection();
var originalStream = new FlushAsyncCheckStream();
var replacementStream = new FlushAsyncCheckStream();
var responseBodyMock = new Mock<IHttpResponseBodyFeature>();
responseBodyMock.Setup(o => o.Stream).Returns(originalStream);
features.Set(responseBodyMock.Object);
var responseMock = new Mock<IHttpResponseFeature>();
features.Set(responseMock.Object);
var context = new DefaultHttpContext(features);
Assert.Same(originalStream, context.Response.Body);
Assert.Same(responseBodyMock.Object, context.Features.Get<IHttpResponseBodyFeature>());
context.Response.Body = replacementStream;
Assert.Same(replacementStream, context.Response.Body);
Assert.NotSame(responseBodyMock.Object, context.Features.Get<IHttpResponseBodyFeature>());
context.Response.Body = originalStream;
Assert.Same(originalStream, context.Response.Body);
Assert.Same(responseBodyMock.Object, context.Features.Get<IHttpResponseBodyFeature>());
// The real issue was not that an OnCompleted registration existed, but that it would previously flush
// the original response body in the OnCompleted callback after the response body was disposed.
// However, since now there's no longer an OnCompleted registration at all, it's easier to verify that.
// https://github.com/dotnet/aspnetcore/issues/25342
responseMock.Verify(m => m.OnCompleted(It.IsAny<Func<object, Task>>(), It.IsAny<object>()), Times.Never);
}
[Fact]
public async Task ResponseStart_CallsFeatureIfSet()
{