[gzip]: allow gzip to work w/ http2 backend w/o content-length (envoy…

…proxy#14405)

Signed-off-by: Nikita V. Shirokov <tehnerd@tehnerd.com>

docs/root/version_history/current.rst

@@ -64,6 +64,7 @@ Removed Config or Runtime
 
 New Features
 ------------
+* compression: added new runtime guard `envoy.reloadable_features.enable_compression_without_chunked_header` (enabled by default) to control compression if no transfer-encoding=chunked header exists (now envoy would compress in such cases) to account for http/2 connections, where such transfer-encoding were removed.
 * compression: the :ref:`compressor <envoy_v3_api_msg_extensions.filters.http.compressor.v3.Compressor>` filter adds support for compressing request payloads. Its configuration is unified with the :ref:`decompressor <envoy_v3_api_msg_extensions.filters.http.decompressor.v3.Decompressor>` filter with two new fields for different directions - :ref:`requests <envoy_v3_api_field_extensions.filters.http.compressor.v3.Compressor.request_direction_config>` and :ref:`responses <envoy_v3_api_field_extensions.filters.http.compressor.v3.Compressor.response_direction_config>`. The latter deprecates the old response-specific fields and, if used, roots the response-specific stats in `<stat_prefix>.compressor.<compressor_library.name>.<compressor_library_stat_prefix>.response.*` instead of `<stat_prefix>.compressor.<compressor_library.name>.<compressor_library_stat_prefix>.*`.
 * config: added ability to flush stats when the admin's :ref:`/stats endpoint <operations_admin_interface_stats>` is hit instead of on a timer via :ref:`stats_flush_on_admin <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.stats_flush_on_admin>`.
 * config: added new runtime feature `envoy.features.enable_all_deprecated_features` that allows the use of all deprecated features.

source/common/runtime/runtime_features.cc

@@ -66,6 +66,7 @@ constexpr const char* runtime_features[] = {
     "envoy.reloadable_features.disable_tls_inspector_injection",
     "envoy.reloadable_features.disallow_unbounded_access_logs",
     "envoy.reloadable_features.early_errors_via_hcm",
+    "envoy.reloadable_features.enable_compression_without_chunked_header",
     "envoy.reloadable_features.enable_dns_cache_circuit_breakers",
     "envoy.reloadable_features.fix_upgrade_response",
     "envoy.reloadable_features.fix_wildcard_matching",

source/extensions/filters/http/common/compressor/compressor.cc

@@ -495,6 +495,10 @@ bool CompressorFilter::isEtagAllowed(Http::ResponseHeaderMap& headers) const {
 
 bool CompressorFilterConfig::DirectionConfig::isMinimumContentLength(
     const Http::RequestOrResponseHeaderMap& headers) const {
+  if (StringUtil::caseFindToken(headers.getTransferEncodingValue(), ",",
+                                Http::Headers::get().TransferEncodingValues.Chunked)) {
+    return true;
+  }
   const Http::HeaderEntry* content_length = headers.ContentLength();
   if (content_length != nullptr) {
     uint64_t length;
@@ -506,9 +510,12 @@ bool CompressorFilterConfig::DirectionConfig::isMinimumContentLength(
     }
     return is_minimum_content_length;
   }
-
-  return StringUtil::caseFindToken(headers.getTransferEncodingValue(), ",",
-                                   Http::Headers::get().TransferEncodingValues.Chunked);
+  if (Runtime::runtimeFeatureEnabled(
+          "envoy.reloadable_features.enable_compression_without_chunked_header")) {
+    // returning true to account for HTTP/2 where content-length is optional
+    return true;
+  }
+  return false;
 }
 
 bool CompressorFilter::isTransferEncodingAllowed(Http::RequestOrResponseHeaderMap& headers) const {

test/extensions/filters/http/common/compressor/BUILD

@@ -21,6 +21,7 @@ envoy_cc_test(
         "//test/mocks/http:http_mocks",
         "//test/mocks/protobuf:protobuf_mocks",
         "//test/mocks/runtime:runtime_mocks",
+        "//test/test_common:test_runtime_lib",
         "//test/test_common:utility_lib",
         "@envoy_api//envoy/extensions/filters/http/compressor/v3:pkg_cc_proto",
     ],

test/extensions/filters/http/common/compressor/compressor_filter_test.cc

@@ -11,6 +11,7 @@
 #include "test/mocks/protobuf/mocks.h"
 #include "test/mocks/runtime/mocks.h"
 #include "test/mocks/stats/mocks.h"
+#include "test/test_common/test_runtime.h"
 #include "test/test_common/utility.h"
 
 #include "gtest/gtest.h"
@@ -77,8 +78,14 @@ class CompressorFilterTest : public testing::Test {
   }
 
   void verifyCompressedData() {
-    EXPECT_EQ(expected_str_.length(), stats_.counter("test.test.total_uncompressed_bytes").value());
-    EXPECT_EQ(data_.length(), stats_.counter("test.test.total_compressed_bytes").value());
+    EXPECT_EQ(
+        expected_str_.length(),
+        stats_.counter(fmt::format("test.test.{}total_uncompressed_bytes", response_stats_prefix_))
+            .value());
+    EXPECT_EQ(
+        data_.length(),
+        stats_.counter(fmt::format("test.test.{}total_compressed_bytes", response_stats_prefix_))
+            .value());
   }
 
   void populateBuffer(uint64_t size) {
@@ -132,9 +139,6 @@ class CompressorFilterTest : public testing::Test {
                   bool with_trailers) {
     uint64_t buffer_content_size;
     if (!absl::SimpleAtoi(headers.get_("content-length"), &buffer_content_size)) {
-      ASSERT_TRUE(
-          StringUtil::CaseInsensitiveCompare()(headers.get_("transfer-encoding"), "chunked"));
-      // In case of chunked stream just feed the buffer with 1000 bytes.
       buffer_content_size = 1000;
     }
     populateBuffer(buffer_content_size);
@@ -156,7 +160,8 @@ class CompressorFilterTest : public testing::Test {
         EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->encodeTrailers(trailers));
       }
       verifyCompressedData();
-      EXPECT_EQ(1, stats_.counter("test.test.compressed").value());
+      EXPECT_EQ(
+          1, stats_.counter(fmt::format("test.test.{}compressed", response_stats_prefix_)).value());
     } else {
       EXPECT_EQ("", headers.get_("content-encoding"));
       EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(data_, false));
@@ -233,6 +238,82 @@ TEST_F(CompressorFilterTest, CompressRequest) {
   doResponseNoCompression(headers);
 }
 
+TEST_F(CompressorFilterTest, CompressRequestNoContentLength) {
+  setUpFilter(R"EOF(
+{
+  "request_direction_config": {},
+  "compressor_library": {
+     "name": "test",
+     "typed_config": {
+       "@type": "type.googleapis.com/envoy.extensions.compression.gzip.compressor.v3.Gzip"
+     }
+  }
+}
+)EOF");
+  doRequestCompression({{":method", "post"}}, false);
+  Http::TestResponseHeaderMapImpl headers{{":status", "200"}};
+  doResponseNoCompression(headers);
+}
+
+TEST_F(CompressorFilterTest, CompressRequestNoContentLengthRuntimeDisabled) {
+  setUpFilter(R"EOF(
+{
+  "request_direction_config": {},
+  "compressor_library": {
+    "name": "test",
+    "typed_config": {
+      "@type": "type.googleapis.com/envoy.extensions.compression.gzip.compressor.v3.Gzip"
+    }
+  }
+}
+)EOF");
+  TestScopedRuntime scoped_runtime;
+  Runtime::LoaderSingleton::getExisting()->mergeValues(
+      {{"envoy.reloadable_features.enable_compression_without_chunked_header", "false"}});
+  doRequestNoCompression({{":method", "post"}});
+  Http::TestResponseHeaderMapImpl headers{{":status", "200"}};
+  doResponseNoCompression(headers);
+}
+
+TEST_F(CompressorFilterTest, CompressResponseNoContentLength) {
+  setUpFilter(R"EOF(
+{
+  "response_direction_config": {},
+  "compressor_library": {
+     "name": "test",
+     "typed_config": {
+       "@type": "type.googleapis.com/envoy.extensions.compression.gzip.compressor.v3.Gzip"
+     }
+  }
+}
+)EOF");
+  response_stats_prefix_ = "response.";
+  doRequestNoCompression({{":method", "get"}, {"accept-encoding", "deflate, test"}});
+  Http::TestResponseHeaderMapImpl headers{{":status", "200"}};
+  doResponseCompression(headers, false);
+}
+
+TEST_F(CompressorFilterTest, CompressResponseNoContentLengthRuntimeDisabled) {
+  setUpFilter(R"EOF(
+{
+  "response_direction_config": {},
+  "compressor_library": {
+    "name": "test",
+    "typed_config": {
+      "@type": "type.googleapis.com/envoy.extensions.compression.gzip.compressor.v3.Gzip"
+    }
+  }
+}
+)EOF");
+  TestScopedRuntime scoped_runtime;
+  Runtime::LoaderSingleton::getExisting()->mergeValues(
+      {{"envoy.reloadable_features.enable_compression_without_chunked_header", "false"}});
+  response_stats_prefix_ = "response.";
+  doRequestNoCompression({{":method", "get"}, {"accept-encoding", "deflate, test"}});
+  Http::TestResponseHeaderMapImpl headers{{":status", "200"}};
+  doResponseNoCompression(headers);
+}
+
 TEST_F(CompressorFilterTest, CompressRequestWithTrailers) {
   setUpFilter(R"EOF(
 {
@@ -556,6 +637,7 @@ INSTANTIATE_TEST_SUITE_P(
                     std::make_tuple("transfer-encoding", "Chunked", "", true),
                     std::make_tuple("transfer-encoding", "chunked", "\"content_length\": 500,",
                                     true),
+                    std::make_tuple("", "", "\"content_length\": 500,", true),
                     std::make_tuple("content-length", "501", "\"content_length\": 500,", true),
                     std::make_tuple("content-length", "499", "\"content_length\": 500,", false)));