PR feedback

Author: Tratcher

src/Http/Http.Features/src/HttpsCompressionMode.cs

@@ -11,7 +11,7 @@ public enum HttpsCompressionMode
         /// <summary>
         /// No value has been specified, use the configured defaults.
         /// </summary>
-        Default,
+        Default = 0,
 
         /// <summary>
         /// Opts out of compression over HTTPS. Enabling compression on HTTPS requests for remotely manipulable content

src/Middleware/ResponseCompression/src/BodyWrapperStream.cs

@@ -46,7 +46,7 @@ internal ValueTask FinishCompressionAsync()
             return _compressionStream?.DisposeAsync() ?? new ValueTask();
         }
 
-        HttpsCompressionMode IHttpsCompressionFeature.Mode { get; set; }
+        HttpsCompressionMode IHttpsCompressionFeature.Mode { get; set; } = HttpsCompressionMode.Default;
 
         public override bool CanRead => false;
 

src/Middleware/ResponseCompression/src/ResponseCompressionProvider.cs

@@ -173,6 +173,7 @@ public virtual bool ShouldCompressResponse(HttpContext context)
         {
             var httpsMode = context.Features.Get<IHttpsCompressionFeature>()?.Mode ?? HttpsCompressionMode.Default;
 
+            // Check if the app has opted into or out of compression over HTTPS
             if (context.Request.IsHttps
                 && (httpsMode == HttpsCompressionMode.DoNotCompress
                     || !(_enableForHttps || httpsMode == HttpsCompressionMode.Compress)))

src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs

@@ -458,9 +458,10 @@ public async Task Request_Https_CompressedIfEnabled(bool enableHttps, int expect
         }
 
         [Theory]
-        [InlineData(false, 100)]
-        [InlineData(true, 30)]
-        public async Task Request_Https_CompressedIfOverriden(bool overrideHttps, int expectedLength)
+        [InlineData(HttpsCompressionMode.Default, 100)]
+        [InlineData(HttpsCompressionMode.DoNotCompress, 100)]
+        [InlineData(HttpsCompressionMode.Compress, 30)]
+        public async Task Request_Https_CompressedIfOptIn(HttpsCompressionMode mode, int expectedLength)
         {
             var sink = new TestSink(
                 TestSink.EnableWithTypeName<ResponseCompressionProvider>,
@@ -482,11 +483,8 @@ public async Task Request_Https_CompressedIfOverriden(bool overrideHttps, int ex
                     app.UseResponseCompression();
                     app.Run(context =>
                     {
-                        if (overrideHttps)
-                        {
-                            var feature = context.Features.Get<IHttpsCompressionFeature>();
-                            feature.Mode = HttpsCompressionMode.Compress;
-                        }
+                        var feature = context.Features.Get<IHttpsCompressionFeature>();
+                        feature.Mode = mode;
                         context.Response.ContentType = TextPlain;
                         return context.Response.WriteAsync(new string('a', 100));
                     });
@@ -507,7 +505,7 @@ public async Task Request_Https_CompressedIfOverriden(bool overrideHttps, int ex
             Assert.Equal(expectedLength, response.Content.ReadAsByteArrayAsync().Result.Length);
 
             var logMessages = sink.Writes.ToList();
-            if (overrideHttps)
+            if (mode == HttpsCompressionMode.Compress)
             {
                 AssertCompressedWithLog(logMessages, "gzip");
             }
@@ -518,9 +516,10 @@ public async Task Request_Https_CompressedIfOverriden(bool overrideHttps, int ex
         }
 
         [Theory]
-        [InlineData(true, 100)]
-        [InlineData(false, 30)]
-        public async Task Request_Https_NotCompressedIfOverriden(bool overrideHttps, int expectedLength)
+        [InlineData(HttpsCompressionMode.Default, 30)]
+        [InlineData(HttpsCompressionMode.Compress, 30)]
+        [InlineData(HttpsCompressionMode.DoNotCompress, 100)]
+        public async Task Request_Https_NotCompressedIfOptOut(HttpsCompressionMode mode, int expectedLength)
         {
             var sink = new TestSink(
                 TestSink.EnableWithTypeName<ResponseCompressionProvider>,
@@ -542,11 +541,8 @@ public async Task Request_Https_NotCompressedIfOverriden(bool overrideHttps, int
                     app.UseResponseCompression();
                     app.Run(context =>
                     {
-                        if (overrideHttps)
-                        {
-                            var feature = context.Features.Get<IHttpsCompressionFeature>();
-                            feature.Mode = HttpsCompressionMode.DoNotCompress;
-                        }
+                        var feature = context.Features.Get<IHttpsCompressionFeature>();
+                        feature.Mode = mode;
                         context.Response.ContentType = TextPlain;
                         return context.Response.WriteAsync(new string('a', 100));
                     });
@@ -567,7 +563,7 @@ public async Task Request_Https_NotCompressedIfOverriden(bool overrideHttps, int
             Assert.Equal(expectedLength, response.Content.ReadAsByteArrayAsync().Result.Length);
 
             var logMessages = sink.Writes.ToList();
-            if (overrideHttps)
+            if (mode == HttpsCompressionMode.DoNotCompress)
             {
                 AssertLog(logMessages.Skip(1).Single(), LogLevel.Debug, "No response compression available for HTTPS requests. See ResponseCompressionOptions.EnableForHttps.");
             }

src/Middleware/StaticFiles/src/StaticFileOptions.cs

@@ -48,7 +48,8 @@ public StaticFileOptions(SharedOptions sharedOptions) : base(sharedOptions)
         public bool ServeUnknownFileTypes { get; set; }
 
         /// <summary>
-        /// Indicates if files should be compressed for HTTPS requests. The default value is <see cref="HttpsCompressionMode.Compress"/>.
+        /// Indicates if files should be compressed for HTTPS requests when the Response Compression middleware is available.
+        /// The default value is <see cref="HttpsCompressionMode.Compress"/>.
         /// </summary>
         /// <remarks>
         /// Enabling compression on HTTPS requests for remotely manipulable content may expose security problems.