Add MVC filter to remove result if End() was called

Author: twsouthwick

src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/BufferResponseStreamMiddleware.cs

@@ -23,21 +23,20 @@ public BufferResponseStreamMiddleware(RequestDelegate next, ILogger<BufferRespon
     }
 
     public Task InvokeAsync(HttpContextCore context)
-        => context.GetEndpoint()?.Metadata.GetMetadata<BufferResponseStreamAttribute>() is { IsDisabled: false } metadata && context.Features.Get<IHttpResponseBodyFeature>() is { } feature
-            ? BufferResponseStreamAsync(context, feature, metadata)
+        => context.GetEndpoint()?.Metadata.GetMetadata<BufferResponseStreamAttribute>() is { IsDisabled: false } metadata
+            ? BufferResponseStreamAsync(context, metadata)
             : _next(context);
 
-    private async Task BufferResponseStreamAsync(HttpContextCore context, IHttpResponseBodyFeature feature, BufferResponseStreamAttribute metadata)
+    private async Task BufferResponseStreamAsync(HttpContextCore context, BufferResponseStreamAttribute metadata)
     {
         LogBuffering(metadata.BufferLimit, metadata.MemoryThreshold);
 
-        var originalBodyFeature = context.Features.Get<IHttpResponseBodyFeature>();
-        var originalBufferedResponseFeature = context.Features.Get<IBufferedResponseFeature>();
+        var responseBodyFeature = context.Features.GetRequired<IHttpResponseBodyFeature>();
 
-        await using var bufferedFeature = new BufferedHttpResponseFeature(feature, metadata);
+        await using var bufferedFeature = new HttpRequestAdapterFeature(responseBodyFeature, metadata);
 
         context.Features.Set<IHttpResponseBodyFeature>(bufferedFeature);
-        context.Features.Set<IBufferedResponseFeature>(bufferedFeature);
+        context.Features.Set<IHttpRequestAdapterFeature>(bufferedFeature);
 
         try
         {
@@ -46,8 +45,8 @@ private async Task BufferResponseStreamAsync(HttpContextCore context, IHttpRespo
         }
         finally
         {
-            context.Features.Set(originalBodyFeature);
-            context.Features.Set(originalBufferedResponseFeature);
+            context.Features.Set(responseBodyFeature);
+            context.Features.Set<IHttpRequestAdapterFeature>(null);
         }
     }
 }

src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/BufferedHttpResponseFeature.cs renamed to src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/HttpRequestAdapterFeature.cs

@@ -12,7 +12,20 @@
 
 namespace Microsoft.AspNetCore.SystemWebAdapters;
 
-internal class BufferedHttpResponseFeature : Stream, IHttpResponseBodyFeature, IBufferedResponseFeature
+/// <summary>
+/// This feature implements the <see cref="IHttpRequestAdapterFeature"/> to expose functionality for the adapters. As part of that,
+/// it overrides the implementations of two other features:
+/// 
+/// <list>
+///  <item>
+///     <see cref="IHttpResponseFeature"/>: On ASP.NET Framework, after End() was called, it would not throw if headers/status/etc was changed.
+///   </item>
+///   <item>
+///     <see cref="IHttpResponseBodyFeature"/>: Provide ability to turn off writing to the stream, while also supporting the ability to clear and suppress output
+///   </item>
+/// </list> 
+/// </summary>
+internal class HttpRequestAdapterFeature : Stream, IHttpResponseBodyFeature, IHttpRequestAdapterFeature
 {
     public enum StreamState
     {
@@ -22,16 +35,17 @@ public enum StreamState
         Complete,
     }
 
-    private readonly IHttpResponseBodyFeature _other;
+    private readonly IHttpResponseBodyFeature _responseBodyFeature;
     private readonly BufferResponseStreamAttribute _metadata;
 
     private FileBufferingWriteStream? _bufferedStream;
     private PipeWriter? _pipeWriter;
 
-    public BufferedHttpResponseFeature(IHttpResponseBodyFeature other, BufferResponseStreamAttribute metadata)
+    public HttpRequestAdapterFeature(IHttpResponseBodyFeature httpResponseBody, BufferResponseStreamAttribute metadata)
     {
-        _other = other;
+        _responseBodyFeature = httpResponseBody;
         _metadata = metadata;
+
         State = StreamState.NotStarted;
     }
 
@@ -49,11 +63,11 @@ private Stream CurrentStream
         {
             if (State == StreamState.NotBuffering)
             {
-                return _other.Stream;
+                return _responseBodyFeature.Stream;
             }
             else if (State == StreamState.Complete)
             {
-                return Stream.Null;
+                return Null;
             }
             else
             {
@@ -63,7 +77,7 @@ private Stream CurrentStream
         }
     }
 
-    public void End() => CompleteAsync().GetAwaiter().GetResult();
+    public Task EndAsync() => CompleteAsync();
 
     public override async ValueTask DisposeAsync()
     {
@@ -79,7 +93,7 @@ public async ValueTask FlushBufferedStreamAsync()
     {
         if (State is StreamState.Buffering && _bufferedStream is not null && !SuppressContent)
         {
-            await _bufferedStream.DrainBufferAsync(_other.Stream);
+            await _bufferedStream.DrainBufferAsync(_responseBodyFeature.Stream);
         }
     }
 
@@ -97,10 +111,21 @@ public override long Position
         set => throw new NotSupportedException();
     }
 
+    public bool IsEnded => State == StreamState.Complete;
+
+    public void ClearContent()
+    {
+        if (_bufferedStream is not null)
+        {
+            _bufferedStream.Dispose();
+            _bufferedStream = null;
+        }
+    }
+
     public async Task CompleteAsync()
     {
         await FlushBufferedStreamAsync();
-        await _other.CompleteAsync();
+        await _responseBodyFeature.CompleteAsync();
         State = StreamState.Complete;
     }
 
@@ -109,8 +134,8 @@ public void DisableBuffering()
         if (State == StreamState.NotStarted)
         {
             State = StreamState.NotBuffering;
-            _other.DisableBuffering();
-            _pipeWriter = _other.Writer;
+            _responseBodyFeature.DisableBuffering();
+            _pipeWriter = _responseBodyFeature.Writer;
         }
     }
 
@@ -134,7 +159,7 @@ public Task StartAsync(CancellationToken cancellationToken = default)
             State = StreamState.Buffering;
         }
 
-        return _other.StartAsync(cancellationToken);
+        return _responseBodyFeature.StartAsync(cancellationToken);
     }
 
     public override void Write(byte[] buffer, int offset, int count) => CurrentStream.Write(buffer, offset, count);
@@ -148,13 +173,4 @@ public override ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationTo
 
     public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
         => CurrentStream.WriteAsync(buffer, offset, count, cancellationToken);
-
-    public void ClearContent()
-    {
-        if (_bufferedStream is not null)
-        {
-            _bufferedStream.Dispose();
-            _bufferedStream = null;
-        }
-    }
 }

src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IBufferedResponseFeature.cs renamed to src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/IHttpRequestAdapterFeature.cs

@@ -1,17 +1,17 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.IO;
+using System.Threading.Tasks;
 
 namespace Microsoft.AspNetCore.SystemWebAdapters;
 
-internal interface IBufferedResponseFeature
+internal interface IHttpRequestAdapterFeature
 {
-    Stream Stream { get; }
+    bool IsEnded { get; }
 
     bool SuppressContent { get; set; }
 
-    void End();
+    Task EndAsync();
 
     void ClearContent();
 }

src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/Mvc/MvcExtensions.cs

@@ -0,0 +1,26 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.SystemWebAdapters;
+using Microsoft.AspNetCore.SystemWebAdapters.Mvc;
+
+namespace Microsoft.Extensions.DependencyInjection;
+
+internal static class MvcExtensions
+{
+    public static ISystemWebAdapterBuilder AddMvc(this ISystemWebAdapterBuilder builder)
+    {
+        builder.Services.AddTransient<ResponseEndFilter>();
+
+        builder.Services.AddOptions<MvcOptions>()
+            .Configure(options =>
+            {
+                // We want the check for HttpResponse.End() to be done as soon as possible after the action is run.
+                // This will minimize any chance that output will be written which will fail since the response has completed.
+                options.Filters.Add<ResponseEndFilter>(int.MaxValue);
+            });
+
+        return builder;
+    }
+}

src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/Mvc/ResponseEndFilter.cs

@@ -0,0 +1,33 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.AspNetCore.Mvc.Filters;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.AspNetCore.SystemWebAdapters.Mvc;
+
+internal partial class ResponseEndFilter : IActionFilter
+{
+    private readonly ILogger<ResponseEndFilter> _logger;
+
+    [LoggerMessage(0, LogLevel.Trace, "Clearing MVC result since HttpResponse.End() was called")]
+    private partial void LogClearingResult();
+
+    public ResponseEndFilter(ILogger<ResponseEndFilter> logger)
+    {
+        _logger = logger;
+    }
+
+    public void OnActionExecuted(ActionExecutedContext context)
+    {
+        if (context.Result is not null && context.HttpContext.Features.Get<IHttpRequestAdapterFeature>() is { IsEnded: true })
+        {
+            LogClearingResult();
+            context.Result = null;
+        }
+    }
+
+    public void OnActionExecuting(ActionExecutingContext context)
+    {
+    }
+}

src/Microsoft.AspNetCore.SystemWebAdapters/Adapters/SystemWebAdaptersExtensions.cs

@@ -6,7 +6,9 @@
 using System.Web.Configuration;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.SystemWebAdapters;
+using Microsoft.AspNetCore.SystemWebAdapters.Mvc;
 
 namespace Microsoft.Extensions.DependencyInjection;
 
@@ -19,7 +21,8 @@ public static ISystemWebAdapterBuilder AddSystemWebAdapters(this IServiceCollect
         services.AddSingleton<BrowserCapabilitiesFactory>();
         services.AddTransient<IStartupFilter, HttpContextStartupFilter>();
 
-        return new SystemWebAdapterBuilder(services);
+        return new SystemWebAdapterBuilder(services)
+            .AddMvc();
     }
 
     public static void UseSystemWebAdapters(this IApplicationBuilder app)

src/Microsoft.AspNetCore.SystemWebAdapters/HttpResponse.cs

@@ -25,7 +25,7 @@ public class HttpResponse
 
         private NameValueCollection? _headers;
         private ResponseHeaders? _typedHeaders;
-        private IBufferedResponseFeature? _bufferedFeature;
+        private IHttpRequestAdapterFeature? _adapterFeature;
         private TextWriter? _writer;
         private HttpCookieCollection? _cookies;
 
@@ -34,8 +34,8 @@ internal HttpResponse(HttpResponseCore response)
             _response = response;
         }
 
-        private IBufferedResponseFeature BufferedFeature => _bufferedFeature ??= _response.HttpContext.Features.Get<IBufferedResponseFeature>()
-            ?? throw new InvalidOperationException("Response buffering must be enabled on this endpoint for this feature via the IBufferResponseStreamMetadata metadata item");
+        private IHttpRequestAdapterFeature AdapterFeature => _adapterFeature ??= _response.HttpContext.Features.Get<IHttpRequestAdapterFeature>()
+            ?? throw new InvalidOperationException($"Response buffering must be enabled on this endpoint for this API via the {nameof(BufferResponseStreamAttribute)} metadata item");
 
         internal ResponseHeaders TypedHeaders => _typedHeaders ??= new(_response.Headers);
 
@@ -82,8 +82,8 @@ public bool TrySkipIisCustomErrors
 
         public bool SuppressContent
         {
-            get => BufferedFeature.SuppressContent;
-            set => BufferedFeature.SuppressContent = value;
+            get => AdapterFeature.SuppressContent;
+            set => AdapterFeature.SuppressContent = value;
         }
 
         public Encoding ContentEncoding
@@ -201,7 +201,7 @@ private void Redirect(string url, bool endResponse, bool permanent)
 
         public void SetCookie(HttpCookie cookie) => Cookies.Set(cookie);
 
-        public void End() => BufferedFeature.End();
+        public void End() => AdapterFeature.EndAsync().GetAwaiter().GetResult();
 
         public void Write(char ch) => Output.Write(ch);
 
@@ -233,7 +233,7 @@ public void ClearContent()
             }
             else
             {
-                BufferedFeature.ClearContent();
+                AdapterFeature.ClearContent();
             }
         }