common: added a field in HCM proto to be able to reverse the order of HTTP encoder filters.

DEPRECATED.md

@@ -8,6 +8,8 @@ A logged warning is expected for each deprecated item that is in deprecation win
 
 ## Version 1.9.0 (pending)
 
+* Use of the boolean field `bugfix_reverse_encode_order` in [http_connection_manager.proto](https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto) is deprecated. This is a bug fix to resolve issue [#4599](https://github.com/envoyproxy/envoy/issues/4599) for envoy where it is designed to have the encode filters in reversed order to the configuration.
+
 ## Version 1.8.0 (Oct 4, 2018)
 
 * Use of the v1 API (including `*.deprecated_v1` fields in the v2 API) is deprecated.

api/envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.proto

@@ -19,7 +19,7 @@ import "gogoproto/gogo.proto";
 // [#protodoc-title: HTTP connection manager]
 // HTTP connection manager :ref:`configuration overview <config_http_conn_man>`.
 
-// [#comment:next free field: 27]
+// [#comment:next free field: 28]
 message HttpConnectionManager {
   enum CodecType {
     option (gogoproto.goproto_enum_prefix) = false;
@@ -348,6 +348,12 @@ message HttpConnectionManager {
     repeated HttpFilter filters = 2;
   };
   repeated UpgradeConfig upgrade_configs = 23;
+
+  // If true, the order of encoder filters will be reversed to that of filters
+  // configured in the HTTP filter chain.
+  // Note: this is a bug fix for envoy, which is designed to have the reversed
+  // order of encode filters to that of decode ones.
+  bool bugfix_reverse_encode_order = 27 [deprecated = true];
 }
 
 message Rds {

source/common/http/conn_manager_config.h

@@ -209,6 +209,12 @@ class ConnectionManagerConfig {
    */
   virtual FilterChainFactory& filterFactory() PURE;
 
+  /**
+   * @return whether the connection manager will reverse the order of encoder
+   * filters in the HTTP filter chain.
+   */
+  virtual bool reverseEncodeOrder() PURE;
+
   /**
    * @return whether the connection manager will generate a fresh x-request-id if the request does
    *         not have one.

source/common/http/conn_manager_impl.cc

@@ -458,7 +458,11 @@ void ConnectionManagerImpl::ActiveStream::addStreamEncoderFilterWorker(
     StreamEncoderFilterSharedPtr filter, bool dual_filter) {
   ActiveStreamEncoderFilterPtr wrapper(new ActiveStreamEncoderFilter(*this, filter, dual_filter));
   filter->setEncoderFilterCallbacks(*wrapper);
-  wrapper->moveIntoListBack(std::move(wrapper), encoder_filters_);
+  if (connection_manager_.config_.reverseEncodeOrder()) {
+    wrapper->moveIntoList(std::move(wrapper), encoder_filters_);
+  } else {
+    wrapper->moveIntoListBack(std::move(wrapper), encoder_filters_);
+  }
 }
 
 void ConnectionManagerImpl::ActiveStream::addAccessLogHandler(

source/extensions/filters/network/http_connection_manager/config.cc

@@ -134,7 +134,8 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
         config,
     Server::Configuration::FactoryContext& context, Http::DateProvider& date_provider,
     Router::RouteConfigProviderManager& route_config_provider_manager)
-    : context_(context), stats_prefix_(fmt::format("http.{}.", config.stat_prefix())),
+    : context_(context), reverse_encode_order_(config.bugfix_reverse_encode_order()),
+      stats_prefix_(fmt::format("http.{}.", config.stat_prefix())),
       stats_(Http::ConnectionManagerImpl::generateStats(stats_prefix_, context_.scope())),
       tracing_stats_(
           Http::ConnectionManagerImpl::generateTracingStats(stats_prefix_, context_.scope())),

source/extensions/filters/network/http_connection_manager/config.h

@@ -107,6 +107,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   Http::DateProvider& dateProvider() override { return date_provider_; }
   std::chrono::milliseconds drainTimeout() override { return drain_timeout_; }
   FilterChainFactory& filterFactory() override { return *this; }
+  bool reverseEncodeOrder() override { return reverse_encode_order_; }
   bool generateRequestId() override { return generate_request_id_; }
   absl::optional<std::chrono::milliseconds> idleTimeout() const override { return idle_timeout_; }
   std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; }
@@ -145,6 +146,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   Server::Configuration::FactoryContext& context_;
   FilterFactoriesList filter_factories_;
   std::map<std::string, std::unique_ptr<FilterFactoriesList>> upgrade_filter_factories_;
+  const bool reverse_encode_order_;
   std::list<AccessLog::InstanceSharedPtr> access_logs_;
   const std::string stats_prefix_;
   Http::ConnectionManagerStats stats_;

source/server/http/admin.h

@@ -96,6 +96,7 @@ class AdminImpl : public Admin,
   Http::DateProvider& dateProvider() override { return date_provider_; }
   std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); }
   Http::FilterChainFactory& filterFactory() override { return *this; }
+  bool reverseEncodeOrder() override { return false; }
   bool generateRequestId() override { return false; }
   absl::optional<std::chrono::milliseconds> idleTimeout() const override { return idle_timeout_; }
   std::chrono::milliseconds streamIdleTimeout() const override { return {}; }

test/common/http/conn_manager_impl_fuzz_test.cc

@@ -88,6 +88,7 @@ class FuzzConfig : public ConnectionManagerConfig {
   DateProvider& dateProvider() override { return date_provider_; }
   std::chrono::milliseconds drainTimeout() override { return std::chrono::milliseconds(100); }
   FilterChainFactory& filterFactory() override { return filter_factory_; }
+  bool reverseEncodeOrder() override { return false; }
   bool generateRequestId() override { return true; }
   absl::optional<std::chrono::milliseconds> idleTimeout() const override { return idle_timeout_; }
   std::chrono::milliseconds streamIdleTimeout() const override { return stream_idle_timeout_; }