Do not cache 304 responses (#23367)

* Do not cache 304 responses #23345

* More guids

src/Servers/HttpSys/src/FeatureContext.cs

@@ -635,6 +635,13 @@ namespace Microsoft.AspNetCore.Server.HttpSys
         private static TimeSpan? GetCacheTtl(RequestContext requestContext)
         {
             var response = requestContext.Response;
+            // A 304 response is supposed to have the same headers as its associated 200 response, including Cache-Control, but the 304 response itself
+            // should not be cached. Otherwise Http.Sys will serve the 304 response to all requests without checking conditional headers like If-None-Match.
+            if (response.StatusCode == StatusCodes.Status304NotModified)
+            {
+                return null;
+            }
+
             // Only consider kernel-mode caching if the Cache-Control response header is present.
             var cacheControlHeader = response.Headers[HeaderNames.CacheControl];
             if (string.IsNullOrEmpty(cacheControlHeader))

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

@@ -80,11 +80,33 @@ namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests
                 return httpContext.Response.Body.WriteAsync(new byte[10], 0, 10);
             }))
             {
+                address += Guid.NewGuid().ToString(); // Avoid cache collisions for failed tests.
                 Assert.Equal("1", await SendRequestAsync(address));
                 Assert.Equal("2", await SendRequestAsync(address));
             }
         }
 
+        [ConditionalFact]
+        public async Task Caching_304_NotCached()
+        {
+            var requestCount = 1;
+            using (Utilities.CreateHttpServer(out string address, httpContext =>
+            {
+                // 304 responses are not themselves cachable. Their cache header mirrors the resource's original cache header.
+                httpContext.Response.StatusCode = StatusCodes.Status304NotModified;
+                httpContext.Response.ContentType = "some/thing"; // Http.Sys requires content-type for caching
+                httpContext.Response.Headers["x-request-count"] = (requestCount++).ToString();
+                httpContext.Response.Headers["Cache-Control"] = "public, max-age=10";
+                httpContext.Response.ContentLength = 10;
+                return httpContext.Response.Body.WriteAsync(new byte[10], 0, 10);
+            }))
+            {
+                address += Guid.NewGuid().ToString(); // Avoid cache collisions for failed tests.
+                Assert.Equal("1", await SendRequestAsync(address, StatusCodes.Status304NotModified));
+                Assert.Equal("2", await SendRequestAsync(address, StatusCodes.Status304NotModified));
+            }
+        }
+
         [ConditionalFact]
         [QuarantinedTest("https://github.com/dotnet/aspnetcore-internal/issues/2207")]
         public async Task Caching_WithoutContentType_Cached_OnWin7AndWin2008R2()
@@ -105,6 +127,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests
                 return httpContext.Response.Body.WriteAsync(new byte[10], 0, 10);
             }))
             {
+                address += Guid.NewGuid().ToString(); // Avoid cache collisions for failed tests.
                 Assert.Equal("1", await SendRequestAsync(address));
                 Assert.Equal("1", await SendRequestAsync(address));
             }
@@ -144,6 +167,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests
                 return httpContext.Response.Body.WriteAsync(new byte[10], 0, 10);
             }))
             {
+                address += Guid.NewGuid().ToString(); // Avoid cache collisions for failed tests.
                 Assert.Equal("1", await SendRequestAsync(address));
                 Assert.Equal("1", await SendRequestAsync(address));
             }
@@ -314,6 +338,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests
                 return httpContext.Response.Body.WriteAsync(new byte[10], 0, 10);
             }))
             {
+                address += Guid.NewGuid().ToString(); // Avoid cache collisions for failed tests.
                 Assert.Equal("1", await SendRequestAsync(address));
                 Assert.Equal("2", await SendRequestAsync(address));
             }
@@ -335,6 +360,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests
                 Assert.Null(httpContext.Response.ContentLength);
             }))
             {
+                address += Guid.NewGuid().ToString(); // Avoid cache collisions for failed tests.
                 Assert.Equal("1", await SendRequestAsync(address));
                 Assert.Equal("1", await SendRequestAsync(address));
             }
@@ -353,6 +379,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests
                 await httpContext.Response.SendFileAsync(_absoluteFilePath, 0, null, CancellationToken.None);
             }))
             {
+                address += Guid.NewGuid().ToString(); // Avoid cache collisions for failed tests.
                 Assert.Equal("1", await GetFileAsync(address));
                 Assert.Equal("2", await GetFileAsync(address));
             }
@@ -372,6 +399,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests
                 await httpContext.Response.SendFileAsync(_absoluteFilePath, 0, null, CancellationToken.None);
             }))
             {
+                address += Guid.NewGuid().ToString(); // Avoid cache collisions for failed tests.
                 Assert.Equal("1", await GetFileAsync(address));
                 Assert.Equal("1", await GetFileAsync(address));
             }
@@ -400,13 +428,15 @@ namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests
                     switch (status)
                     {
                         case 206: // 206 (Partial Content) is not cached
+                        case 304: // 304 (Not Modified) is not cached
                         case 407: // 407 (Proxy Authentication Required) makes CoreCLR's HttpClient throw
                             continue;
                     }
                     requestCount = 1;
+                    var query = "?" + Guid.NewGuid().ToString(); // Avoid cache collisions for failed tests.
                     try
                     {
-                        Assert.Equal("1", await SendRequestAsync(address + status, status));
+                        Assert.Equal("1", await SendRequestAsync(address + status + query, status));
                     }
                     catch (Exception ex)
                     {
@@ -414,7 +444,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys.FunctionalTests
                     }
                     try
                     {
-                        Assert.Equal("1", await SendRequestAsync(address + status, status));
+                        Assert.Equal("1", await SendRequestAsync(address + status + query, status));
                     }
                     catch (Exception ex)
                     {