router: implement append/override of request/response headers to add (fixes #2003)

include/envoy/http/header_map.h

@@ -347,6 +347,32 @@ class HeaderMap {
    */
   virtual void addCopy(const LowerCaseString& key, const std::string& value) PURE;
 
+  /**
+   * Set a reference header in the map. Both key and value MUST point to data that will live beyond
+   * the lifetime of any request/response using the string (since a codec may optimize for zero
+   * copy). Nothing will be copied.
+   *
+   * Calling setReference multiple times for the same header will result in only the last header
+   * being present in the HeaderMap.
+   *
+   * @param key specifies the name of the header to set; it WILL NOT be copied.
+   * @param value specifies the value of the header to set; it WILL NOT be copied.
+   */
+  virtual void setReference(const LowerCaseString& key, const std::string& value) PURE;
+
+  /**
+   * Set a header with a reference key in the map. The key MUST point to point to data that will
+   * live beyond the lifetime of any request/response using the string (since a codec may optimize
+   * for zero copy). The value will be copied.
+   *
+   * Calling setReferenceKey multiple times for the same header will result in only the last header
+   * being present in the HeaderMap.
+   *
+   * @param key specifies the name of the header to set; it WILL NOT be copied.
+   * @param value specifies the value of the header to set; it WILL be copied.
+   */
+  virtual void setReferenceKey(const LowerCaseString& key, const std::string& value) PURE;
+
   /**
    * @return uint64_t the approximate size of the header map in bytes.
    */

include/envoy/router/router.h

@@ -424,6 +424,34 @@ class Route {
 
 typedef std::shared_ptr<const Route> RouteConstSharedPtr;
 
+/**
+ * Header to be added to a request or response.
+ */
+struct HeaderAddition {
+  /**
+   * The name of the header to add.
+   */
+  const Http::LowerCaseString header_;
+
+  /**
+   * The value of the header to add.
+   */
+  const std::string value_;
+
+  /**
+   * Indicates whether the value should be appended to existing values for the header (true) or
+   * replace them (false).
+   */
+  const bool append_;
+
+  /**
+   * Equality comparison.
+   */
+  bool operator==(const HeaderAddition& rhs) const {
+    return header_ == rhs.header_ && value_ == rhs.value_ && append_ == rhs.append_;
+  }
+};
+
 /**
  * The router configuration.
  */
@@ -449,11 +477,10 @@ class Config {
   virtual const std::list<Http::LowerCaseString>& internalOnlyHeaders() const PURE;
 
   /**
-   * Return a list of header key/value pairs that will be added to every response that transits the
-   * router.
+   * Return a list of header key/value pairs will be appended to or override every response that
+   * transits the router.
    */
-  virtual const std::list<std::pair<Http::LowerCaseString, std::string>>&
-  responseHeadersToAdd() const PURE;
+  virtual const std::list<HeaderAddition>& responseHeadersToAdd() const PURE;
 
   /**
    * Return a list of upstream headers that will be stripped from every response that transits the

source/common/http/conn_manager_utility.cc

@@ -197,9 +197,12 @@ void ConnectionManagerUtility::mutateResponseHeaders(Http::HeaderMap& response_h
     response_headers.remove(to_remove);
   }
 
-  for (const std::pair<Http::LowerCaseString, std::string>& to_add :
-       route_config.responseHeadersToAdd()) {
-    response_headers.addReference(to_add.first, to_add.second);
+  for (const Router::HeaderAddition& to_add : route_config.responseHeadersToAdd()) {
+    if (to_add.append_) {
+      response_headers.addReference(to_add.header_, to_add.value_);
+    } else {
+      response_headers.setReference(to_add.header_, to_add.value_);
+    }
   }
 
   if (request_headers.EnvoyForceTrace() && request_headers.RequestId()) {

source/common/http/header_map_impl.cc

@@ -363,6 +363,22 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, const std::string& value
   ASSERT(new_value.empty());
 }
 
+void HeaderMapImpl::setReference(const LowerCaseString& key, const std::string& value) {
+  HeaderString ref_key(key);
+  HeaderString ref_value(value);
+  remove(key);
+  insertByKey(std::move(ref_key), std::move(ref_value));
+}
+
+void HeaderMapImpl::setReferenceKey(const LowerCaseString& key, const std::string& value) {
+  HeaderString ref_key(key);
+  HeaderString new_value;
+  new_value.setCopy(value.c_str(), value.size());
+  remove(key);
+  insertByKey(std::move(ref_key), std::move(new_value));
+  ASSERT(new_value.empty());
+}
+
 uint64_t HeaderMapImpl::byteSize() const {
   uint64_t byte_size = 0;
   for (const HeaderEntryImpl& header : headers_) {

source/common/http/header_map_impl.h

@@ -59,6 +59,8 @@ class HeaderMapImpl : public HeaderMap {
   void addReferenceKey(const LowerCaseString& key, const std::string& value) override;
   void addCopy(const LowerCaseString& key, uint64_t value) override;
   void addCopy(const LowerCaseString& key, const std::string& value) override;
+  void setReference(const LowerCaseString& key, const std::string& value) override;
+  void setReferenceKey(const LowerCaseString& key, const std::string& value) override;
 
   uint64_t byteSize() const override;
   const HeaderEntry* get(const LowerCaseString& key) const override;

source/common/router/config_impl.cc

@@ -30,6 +30,16 @@
 
 namespace Envoy {
 namespace Router {
+namespace {
+
+Router::HeaderAddition
+makeHeaderToAdd(const envoy::api::v2::HeaderValueOption& header_value_option) {
+  return Router::HeaderAddition{Http::LowerCaseString(header_value_option.header().key()),
+                                header_value_option.header().value(),
+                                PROTOBUF_GET_WRAPPED_OR_DEFAULT(header_value_option, append, true)};
+}
+
+} // namespace
 
 std::string SslRedirector::newPath(const Http::HeaderMap& headers) const {
   return Http::Utility::createSslRedirectPath(headers);
@@ -292,8 +302,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost,
   }
 
   for (const auto& header_value_option : route.route().request_headers_to_add()) {
-    request_headers_to_add_.push_back({Http::LowerCaseString(header_value_option.header().key()),
-                                       header_value_option.header().value()});
+    request_headers_to_add_.push_back(makeHeaderToAdd(header_value_option));
   }
 
   // Only set include_vh_rate_limits_ to true if the rate limit policy for the route is empty
@@ -608,8 +617,7 @@ VirtualHostImpl::VirtualHostImpl(const envoy::api::v2::VirtualHost& virtual_host
   }
 
   for (const auto& header_value_option : virtual_host.request_headers_to_add()) {
-    request_headers_to_add_.push_back({Http::LowerCaseString(header_value_option.header().key()),
-                                       header_value_option.header().value()});
+    request_headers_to_add_.push_back(makeHeaderToAdd(header_value_option));
   }
 
   for (const auto& route : virtual_host.routes()) {
@@ -792,17 +800,15 @@ ConfigImpl::ConfigImpl(const envoy::api::v2::RouteConfiguration& config, Runtime
   }
 
   for (const auto& header_value_option : config.response_headers_to_add()) {
-    response_headers_to_add_.push_back({Http::LowerCaseString(header_value_option.header().key()),
-                                        header_value_option.header().value()});
+    response_headers_to_add_.push_back(makeHeaderToAdd(header_value_option));
   }
 
   for (const std::string& header : config.response_headers_to_remove()) {
     response_headers_to_remove_.push_back(Http::LowerCaseString(header));
   }
 
   for (const auto& header_value_option : config.request_headers_to_add()) {
-    request_headers_to_add_.push_back({Http::LowerCaseString(header_value_option.header().key()),
-                                       header_value_option.header().value()});
+    request_headers_to_add_.push_back(makeHeaderToAdd(header_value_option));
   }
   request_headers_parser_ = RequestHeaderParser::parse(config.request_headers_to_add());
 }

source/common/router/config_impl.h

@@ -105,7 +105,7 @@ class VirtualHostImpl : public VirtualHost {
   RouteConstSharedPtr getRouteFromEntries(const Http::HeaderMap& headers,
                                           uint64_t random_value) const;
   const VirtualCluster* virtualClusterFromEntries(const Http::HeaderMap& headers) const;
-  const std::list<std::pair<Http::LowerCaseString, std::string>>& requestHeadersToAdd() const {
+  const std::list<Router::HeaderAddition>& requestHeadersToAdd() const {
     return request_headers_to_add_;
   }
   const ConfigImpl& globalRouteConfig() const { return global_route_config_; }
@@ -148,7 +148,7 @@ class VirtualHostImpl : public VirtualHost {
   std::unique_ptr<const CorsPolicyImpl> cors_policy_;
   const ConfigImpl& global_route_config_; // See note in RouteEntryImplBase::clusterEntry() on why
                                           // raw ref to the top level config is currently safe.
-  std::list<std::pair<Http::LowerCaseString, std::string>> request_headers_to_add_;
+  std::list<Router::HeaderAddition> request_headers_to_add_;
   RequestHeaderParserPtr request_headers_parser_;
 };
 
@@ -298,7 +298,8 @@ class RouteEntryImplBase : public RouteEntry,
 
   bool matchRoute(const Http::HeaderMap& headers, uint64_t random_value) const;
   void validateClusters(Upstream::ClusterManager& cm) const;
-  const std::list<std::pair<Http::LowerCaseString, std::string>>& requestHeadersToAdd() const {
+
+  const std::list<Router::HeaderAddition>& requestHeadersToAdd() const {
     return request_headers_to_add_;
   }
 
@@ -469,7 +470,7 @@ class RouteEntryImplBase : public RouteEntry,
   std::vector<WeightedClusterEntrySharedPtr> weighted_clusters_;
   std::unique_ptr<const HashPolicyImpl> hash_policy_;
   MetadataMatchCriteriaImplConstPtr metadata_match_criteria_;
-  std::list<std::pair<Http::LowerCaseString, std::string>> request_headers_to_add_;
+  std::list<Router::HeaderAddition> request_headers_to_add_;
   RequestHeaderParserPtr request_headers_parser_;
 
   // TODO(danielhochman): refactor multimap into unordered_map since JSON is unordered map.
@@ -575,7 +576,7 @@ class ConfigImpl : public Config {
   ConfigImpl(const envoy::api::v2::RouteConfiguration& config, Runtime::Loader& runtime,
              Upstream::ClusterManager& cm, bool validate_clusters_default);
 
-  const std::list<std::pair<Http::LowerCaseString, std::string>>& requestHeadersToAdd() const {
+  const std::list<Router::HeaderAddition>& requestHeadersToAdd() const {
     return request_headers_to_add_;
   }
 
@@ -590,8 +591,7 @@ class ConfigImpl : public Config {
     return internal_only_headers_;
   }
 
-  const std::list<std::pair<Http::LowerCaseString, std::string>>&
-  responseHeadersToAdd() const override {
+  const std::list<Router::HeaderAddition>& responseHeadersToAdd() const override {
     return response_headers_to_add_;
   }
 
@@ -602,9 +602,9 @@ class ConfigImpl : public Config {
 private:
   std::unique_ptr<RouteMatcher> route_matcher_;
   std::list<Http::LowerCaseString> internal_only_headers_;
-  std::list<std::pair<Http::LowerCaseString, std::string>> response_headers_to_add_;
+  std::list<Router::HeaderAddition> response_headers_to_add_;
   std::list<Http::LowerCaseString> response_headers_to_remove_;
-  std::list<std::pair<Http::LowerCaseString, std::string>> request_headers_to_add_;
+  std::list<Router::HeaderAddition> request_headers_to_add_;
   RequestHeaderParserPtr request_headers_parser_;
 };
 
@@ -620,8 +620,7 @@ class NullConfigImpl : public Config {
     return internal_only_headers_;
   }
 
-  const std::list<std::pair<Http::LowerCaseString, std::string>>&
-  responseHeadersToAdd() const override {
+  const std::list<Router::HeaderAddition>& responseHeadersToAdd() const override {
     return response_headers_to_add_;
   }
 
@@ -631,7 +630,7 @@ class NullConfigImpl : public Config {
 
 private:
   std::list<Http::LowerCaseString> internal_only_headers_;
-  std::list<std::pair<Http::LowerCaseString, std::string>> response_headers_to_add_;
+  std::list<Router::HeaderAddition> response_headers_to_add_;
   std::list<Http::LowerCaseString> response_headers_to_remove_;
 };
 

source/common/router/req_header_formatter.cc

@@ -4,32 +4,36 @@
 #include <string>
 
 #include "common/config/rds_json.h"
+#include "common/protobuf/utility.h"
 
 #include "fmt/format.h"
 
 namespace Envoy {
 namespace Router {
 
-HeaderFormatterPtr RequestHeaderParser::parseInternal(const std::string& format) {
+HeaderFormatterPtr RequestHeaderParser::parseInternal(const std::string& format,
+                                                      const bool append) {
   if (format.find("%") == 0) {
     const size_t last_occ_pos = format.rfind("%");
     if (last_occ_pos == std::string::npos || last_occ_pos <= 1) {
       throw EnvoyException(fmt::format("Incorrect header configuration. Expected variable format "
                                        "%<variable_name>%, actual format {}",
                                        format));
     }
-    return HeaderFormatterPtr{new RequestHeaderFormatter(format.substr(1, last_occ_pos - 1))};
+    return HeaderFormatterPtr{
+        new RequestHeaderFormatter(format.substr(1, last_occ_pos - 1), append)};
   } else {
-    return HeaderFormatterPtr{new PlainHeaderFormatter(format)};
+    return HeaderFormatterPtr{new PlainHeaderFormatter(format, append)};
   }
 }
 
 RequestHeaderParserPtr RequestHeaderParser::parse(
     const Protobuf::RepeatedPtrField<envoy::api::v2::HeaderValueOption>& headers) {
   RequestHeaderParserPtr request_header_parser(new RequestHeaderParser());
   for (const auto& header_value_option : headers) {
-    HeaderFormatterPtr header_formatter =
-        RequestHeaderParser::parseInternal(header_value_option.header().value());
+    HeaderFormatterPtr header_formatter = RequestHeaderParser::parseInternal(
+        header_value_option.header().value(),
+        PROTOBUF_GET_WRAPPED_OR_DEFAULT(header_value_option, append, true));
     request_header_parser->header_formatters_.push_back(
         {Http::LowerCaseString(header_value_option.header().key()), std::move(header_formatter)});
   }
@@ -39,11 +43,16 @@ RequestHeaderParserPtr RequestHeaderParser::parse(
 void RequestHeaderParser::evaluateRequestHeaders(Http::HeaderMap& headers,
                                                  const AccessLog::RequestInfo& request_info) const {
   for (const auto& formatter : header_formatters_) {
-    headers.addReferenceKey(formatter.first, formatter.second->format(request_info));
+    if (formatter.second->append()) {
+      headers.addReferenceKey(formatter.first, formatter.second->format(request_info));
+    } else {
+      headers.setReferenceKey(formatter.first, formatter.second->format(request_info));
+    }
   }
 }
 
-RequestHeaderFormatter::RequestHeaderFormatter(const std::string& field_name) {
+RequestHeaderFormatter::RequestHeaderFormatter(const std::string& field_name, bool append)
+    : append_(append) {
   if (field_name == "PROTOCOL") {
     field_extractor_ = [](const Envoy::AccessLog::RequestInfo& request_info) {
       return Envoy::AccessLog::AccessLogFormatUtils::protocolToString(request_info.protocol());

source/common/router/req_header_formatter.h

@@ -19,6 +19,12 @@ class HeaderFormatter {
   virtual ~HeaderFormatter() {}
 
   virtual const std::string format(const Envoy::AccessLog::RequestInfo& request_info) const PURE;
+
+  /**
+   * @return bool indicating whether the formatted header should be appended to the existing
+   *              headers
+   */
+  virtual bool append() const PURE;
 };
 
 typedef std::unique_ptr<HeaderFormatter> HeaderFormatterPtr;
@@ -28,30 +34,34 @@ typedef std::unique_ptr<HeaderFormatter> HeaderFormatterPtr;
  */
 class RequestHeaderFormatter : public HeaderFormatter {
 public:
-  RequestHeaderFormatter(const std::string& field_name);
+  RequestHeaderFormatter(const std::string& field_name, bool append);
 
   // HeaderFormatter::format
   const std::string format(const Envoy::AccessLog::RequestInfo& request_info) const override;
+  bool append() const override { return append_; }
 
 private:
   std::function<std::string(const Envoy::AccessLog::RequestInfo&)> field_extractor_;
+  const bool append_;
 };
 
 /**
  * A formatter that returns back the same static header value.
  */
 class PlainHeaderFormatter : public HeaderFormatter {
 public:
-  PlainHeaderFormatter(const std::string& static_header_value)
-      : static_value_(static_header_value){};
+  PlainHeaderFormatter(const std::string& static_header_value, bool append)
+      : static_value_(static_header_value), append_(append){};
 
   // HeaderFormatter::format
   const std::string format(const Envoy::AccessLog::RequestInfo&) const override {
     return static_value_;
   };
+  bool append() const override { return append_; }
 
 private:
   const std::string static_value_;
+  const bool append_;
 };
 
 class RequestHeaderParser;
@@ -74,7 +84,7 @@ class RequestHeaderParser {
 private:
   std::list<std::pair<Http::LowerCaseString, HeaderFormatterPtr>> header_formatters_;
 
-  static HeaderFormatterPtr parseInternal(const std::string& format);
+  static HeaderFormatterPtr parseInternal(const std::string& format, const bool append);
 };
 
 } // namespace Router