Consistently return StringValues.Empty instead of default for missing headers

src/Http/Http/src/HeaderDictionary.cs

@@ -73,6 +73,7 @@ public StringValues this[string key]
             {
                 return value;
             }
+
             return StringValues.Empty;
         }
         set
@@ -94,7 +95,15 @@ public StringValues this[string key]
 
     StringValues IDictionary<string, StringValues>.this[string key]
     {
-        get { return this[key]; }
+        get
+        {
+            if (Store == null)
+            {
+                ThrowKeyNotFoundException();
+            }
+
+            return Store[key];
+        }
         set
         {
             ThrowIfReadOnly();
@@ -361,6 +370,12 @@ private void ThrowIfReadOnly()
         }
     }
 
+    [DoesNotReturn]
+    private static void ThrowKeyNotFoundException()
+    {
+        throw new KeyNotFoundException();
+    }
+
     /// <summary>
     /// Enumerates a <see cref="HeaderDictionary"/>.
     /// </summary>

src/Http/Http/test/HeaderDictionaryTests.cs

@@ -112,4 +112,24 @@ public void GetCommaSeparatedValues_WorksForUnquotedHeaderValuesEndingWithSpace(
 
         Assert.Equal(new[] { "value " }, result);
     }
+
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public void ReturnsCorrectStringValuesEmptyForMissingHeaders(bool withStore)
+    {
+        // Test both with and without HeaderDictionary.Store set.
+        var emptyHeaders = withStore ? new HeaderDictionary(1) : new HeaderDictionary();
+
+        // StringValues.Empty.Equals(default(StringValues)), so we check if the implicit conversion
+        // to string[] returns null or Array.Empty<string>() to tell the difference.
+        Assert.Same(Array.Empty<string>(), (string[])emptyHeaders["Header1"]);
+
+        IHeaderDictionary asIHeaderDictionary = emptyHeaders;
+        Assert.Same(Array.Empty<string>(), (string[])asIHeaderDictionary["Header1"]);
+        Assert.Same(Array.Empty<string>(), (string[])asIHeaderDictionary.Host);
+
+        IDictionary<string, StringValues> asIDictionary = emptyHeaders;
+        Assert.Throws<KeyNotFoundException>(() => asIDictionary["Header1"]);
+    }
 }

src/Http/Http/test/QueryCollectionTests.cs

@@ -0,0 +1,22 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.Extensions.Primitives;
+
+namespace Microsoft.AspNetCore.Http.Tests;
+
+public class QueryCollectionTests
+{
+    [Fact]
+    public void ReturnStringValuesEmptyForMissingQueryKeys()
+    {
+        IQueryCollection query = new QueryCollection(new Dictionary<string, StringValues>());
+
+        // StringValues.Empty.Equals(default(StringValues)), so we check if the implicit conversion
+        // to string[] returns null or Array.Empty<string>() to tell the difference.
+        Assert.Same(Array.Empty<string>(), (string[])query["query1"]);
+
+        // Test the null-dictionary code path too.
+        Assert.Same(Array.Empty<string>(), (string[])QueryCollection.Empty["query1"]);
+    }
+}

src/Servers/HttpSys/test/FunctionalTests/RequestBodyTests.cs

@@ -12,6 +12,7 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Testing;
+using Microsoft.Extensions.Primitives;
 using Microsoft.Net.Http.Headers;
 using Xunit;
 
@@ -263,7 +264,7 @@ static void CheckHeadersCount(string headerName, int expectedCount, HttpRequest
             httpContext.Request.Headers[HeaderNames.ContentLength] = "456";
             CheckHeadersCount(HeaderNames.ContentLength, 1, httpContext.Request);
             Assert.Equal(456, httpContext.Request.ContentLength);
-            httpContext.Request.Headers[HeaderNames.ContentLength] = "";
+            httpContext.Request.Headers[HeaderNames.ContentLength] = StringValues.Empty;
             CheckHeadersCount(HeaderNames.ContentLength, 0, httpContext.Request);
             Assert.Null(httpContext.Request.ContentLength);
             Assert.Equal("", httpContext.Request.Headers[HeaderNames.ContentLength].ToString());
@@ -276,7 +277,7 @@ static void CheckHeadersCount(string headerName, int expectedCount, HttpRequest
             CheckHeadersCount("Custom-Header", 1, httpContext.Request);
             httpContext.Request.Headers["Custom-Header"] = "bar";
             CheckHeadersCount("Custom-Header", 1, httpContext.Request);
-            httpContext.Request.Headers["Custom-Header"] = "";
+            httpContext.Request.Headers["Custom-Header"] = StringValues.Empty;
             CheckHeadersCount("Custom-Header", 0, httpContext.Request);
             Assert.Equal("", httpContext.Request.Headers["Custom-Header"].ToString());
 

src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs

@@ -439,6 +439,13 @@ await Helpers.StressLoad(_fixture.Client, "/GetServerVariableStress", response =
         });
     }
 
+    [ConditionalFact]
+    public async Task TestStringValuesEmptyForMissingHeaders()
+    {
+        var result = await _fixture.Client.GetStringAsync($"/TestRequestHeaders");
+        Assert.Equal("Success", result);
+    }
+
     [ConditionalFact]
     public async Task TestReadOffsetWorks()
     {

src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs

@@ -425,6 +425,63 @@ private async Task ResponseEmptyHeaders(HttpContext ctx)
         await ctx.Response.WriteAsync("EmptyHeaderShouldBeSkipped");
     }
 
+    private Task TestRequestHeaders(HttpContext ctx)
+    {
+        // Test optimized and non-optimized headers behave equivalently
+        foreach (var headerName in new[] { "custom", "Content-Type" })
+        {
+            // StringValues.Empty.Equals(default(StringValues)), so we check if the implicit conversion
+            // to string[] returns null or Array.Empty<string>() to tell the difference.
+            if ((string[])ctx.Request.Headers[headerName] != Array.Empty<string>())
+            {
+                return ctx.Response.WriteAsync($"Failure: '{headerName}' indexer");
+            }
+            if (ctx.Request.Headers.TryGetValue(headerName, out var headerValue) || (string[])headerValue is not null)
+            {
+                return ctx.Response.WriteAsync($"Failure: '{headerName}' TryGetValue");
+            }
+
+            // Both default and StringValues.Empty should unset the header, allowing it to be added again.
+            ArgumentException duplicateKeyException = null;
+            ctx.Request.Headers.Add(headerName, "test");
+            ctx.Request.Headers[headerName] = default;
+            ctx.Request.Headers.Add(headerName, "test");
+            ctx.Request.Headers[headerName] = StringValues.Empty;
+            ctx.Request.Headers.Add(headerName, "test");
+
+            try
+            {
+                // Repeated adds should throw.
+                ctx.Request.Headers.Add(headerName, "test");
+            }
+            catch (ArgumentException ex)
+            {
+                duplicateKeyException = ex;
+                ctx.Request.Headers[headerName] = default;
+            }
+
+            if (duplicateKeyException is null)
+            {
+                return ctx.Response.WriteAsync($"Failure: Repeated '{headerName}' Add did not throw");
+            }
+        }
+
+#if !FORWARDCOMPAT
+        if ((string[])ctx.Request.Headers.ContentType != Array.Empty<string>())
+        {
+            return ctx.Response.WriteAsync("Failure: ContentType property");
+        }
+
+        ctx.Request.Headers.ContentType = default;
+        if ((string[])ctx.Request.Headers.ContentType != Array.Empty<string>())
+        {
+            return ctx.Response.WriteAsync("Failure: ContentType property after setting default");
+        }
+#endif
+
+        return ctx.Response.WriteAsync("Success");
+    }
+
     private async Task ResponseInvalidOrdering(HttpContext ctx)
     {
         if (ctx.Request.Path.Equals("/SetStatusCodeAfterWrite"))