Ensure default response headers are only set if empty

This change moves it to be on the `Response.OnStarting` call rather than at the beginning of the request. As part of this, a bug in `HttpRequest.AppendHeader` was fixed.

Author: twsouthwick

src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/SetDefaultResponseHeadersMiddleware.cs

@@ -14,7 +14,7 @@ internal class SetDefaultResponseHeadersMiddleware
 {
     private readonly RequestDelegate _next;
 
-    private readonly (string Header, string Value)[] _defaultHeaders = new (string, string)[]
+    private static readonly (string Header, string Value)[] _defaultHeaders = new (string, string)[]
     {
         (HeaderNames.CacheControl, CacheControlHeaderValue.PrivateString),
         (HeaderNames.ContentType, "text/html")
@@ -24,13 +24,20 @@ internal class SetDefaultResponseHeadersMiddleware
 
     public Task InvokeAsync(HttpContext context)
     {
-        foreach (var (header, value) in _defaultHeaders)
+        context.Response.OnStarting(static state =>
         {
-            if (!context.Response.Headers.ContainsKey(header))
+            var context = (HttpContext)state;
+
+            foreach (var (header, value) in _defaultHeaders)
             {
-                context.Response.Headers[header] = value;
+                if (!context.Response.Headers.ContainsKey(header))
+                {
+                    context.Response.Headers[header] = value;
+                }
             }
-        }
+
+            return Task.CompletedTask;
+        }, context);
 
         return _next(context);
     }

src/Microsoft.AspNetCore.SystemWebAdapters/HttpResponse.cs

@@ -173,7 +173,7 @@ public void AppendHeader(string name, string value)
         {
             if (_response.Headers.TryGetValue(name, out var existing))
             {
-                _response.Headers.Add(name, StringValues.Concat(existing, value));
+                _response.Headers[name] = StringValues.Concat(existing, value);
             }
             else
             {

test/Microsoft.AspNetCore.SystemWebAdapters.CoreServices.Tests/SetDefaultResponseHeadersMiddlewareTests.cs

@@ -1,8 +1,10 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features;
 using Xunit;
 
 namespace Microsoft.AspNetCore.SystemWebAdapters.Tests;
@@ -14,11 +16,14 @@ public async Task NoHeaders()
     {
         // Arrange
         var context = new DefaultHttpContext();
+        var startupFeature = new StartupCallbackFeature();
+        context.Features.Set<IHttpResponseFeature>(startupFeature);
         var next = Task (HttpContextCore context) => Task.CompletedTask;
         var middleware = new SetDefaultResponseHeadersMiddleware(new(next));
 
         // Act
         await middleware.InvokeAsync(context);
+        await startupFeature.RunAsync();
 
         // Assert
         Assert.Equal(2, context.Response.Headers.Count);
@@ -33,15 +38,67 @@ public async Task Existing(string name, string value)
     {
         // Arrange
         var context = new DefaultHttpContext();
+        var startupFeature = new StartupCallbackFeature();
+        context.Features.Set<IHttpResponseFeature>(startupFeature);
+
         context.Response.Headers[name] = value;
 
         var next = Task (HttpContextCore context) => Task.CompletedTask;
         var middleware = new SetDefaultResponseHeadersMiddleware(new(next));
 
         // Act
         await middleware.InvokeAsync(context);
+        await startupFeature.RunAsync();
+
+        // Assert
+        Assert.Equal(value, context.Response.Headers[name]);
+    }
+
+    [Theory]
+    [InlineData("Content-Type", "some-content-type")]
+    [InlineData("Cache-Control", "cache-value")]
+    public async Task AddedInLaterMiddleware(string name, string value)
+    {
+        // Arrange
+        var context = new DefaultHttpContext();
+        var startupFeature = new StartupCallbackFeature();
+        context.Features.Set<IHttpResponseFeature>(startupFeature);
+
+        var next = Task (HttpContextCore context) =>
+        {
+            var adapter = context.GetAdapter();
+            adapter.Response.AppendHeader(name, value);
+
+            return Task.CompletedTask;
+        };
+        var middleware = new SetDefaultResponseHeadersMiddleware(new(next));
+
+        // Act
+        await middleware.InvokeAsync(context);
+        await startupFeature.RunAsync();
 
         // Assert
         Assert.Equal(value, context.Response.Headers[name]);
     }
+
+    // The default feature does not include a function `OnStarting` method
+    private class StartupCallbackFeature : HttpResponseFeature
+    {
+        private Func<Task>? _callback;
+
+        public override void OnStarting(Func<object, Task> callback, object state)
+        {
+            _callback = () => callback(state);
+        }
+
+        public Task RunAsync()
+        {
+            if (_callback is null)
+            {
+                throw new InvalidOperationException();
+            }
+
+            return _callback();
+        }
+    }
 }