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

include/envoy/http/header_map.h

@@ -129,6 +129,14 @@ class HeaderString {
    */
   Type type() const { return type_; }
 
+  /**
+   * Move the rhs value into this HeaderString.
+   * @param rhs an existing HeaderString whose contents 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).
+   */
+  HeaderString& operator=(HeaderString&& rhs);
+
   bool operator==(const char* rhs) const { return 0 == strcmp(c_str(), rhs); }
   bool operator!=(const char* rhs) const { return 0 != strcmp(c_str(), rhs); }
 
@@ -171,6 +179,11 @@ class HeaderEntry {
    */
   virtual void value(const std::string& value) PURE;
 
+  /**
+   * Set the header value by moving data into it.
+   */
+  virtual void value(HeaderString&& value) PURE;
+
   /**
    * Set the header value by copying an integer into it.
    */
@@ -347,6 +360,67 @@ 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 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, uint64_t 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;
+
+  /**
+   * Set a header by copying both the header key and the value.
+   *
+   * Calling setCopy 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 be copied.
+   * @param value specifies the value of the header to set; it WILL be copied.
+   */
+  virtual void setCopy(const LowerCaseString& key, uint64_t value) PURE;
+
+  /**
+   * Set a header by copying both the header key and the value.
+   *
+   * Calling setCopy 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 be copied.
+   * @param value specifies the value of the header to set; it WILL be copied.
+   */
+  virtual void setCopy(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

@@ -417,6 +417,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.
  */
@@ -442,11 +470,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

@@ -190,6 +190,36 @@ void HeaderString::setReference(const std::string& ref_value) {
   string_length_ = ref_value.size();
 }
 
+HeaderString& HeaderString::operator=(HeaderString&& move_value) {
+  freeDynamic();
+
+  type_ = move_value.type_;
+  string_length_ = move_value.string_length_;
+  switch (move_value.type_) {
+  case Type::Reference: {
+    buffer_.ref_ = move_value.buffer_.ref_;
+    break;
+  }
+  case Type::Dynamic: {
+    // When we move a dynamic header, we switch the moved header back to its default state (inline).
+    buffer_.dynamic_ = move_value.buffer_.dynamic_;
+    dynamic_capacity_ = move_value.dynamic_capacity_;
+    move_value.type_ = Type::Inline;
+    move_value.buffer_.dynamic_ = move_value.inline_buffer_;
+    move_value.clear();
+    break;
+  }
+  case Type::Inline: {
+    buffer_.dynamic_ = inline_buffer_;
+    memcpy(inline_buffer_, move_value.inline_buffer_, string_length_ + 1);
+    move_value.string_length_ = 0;
+    move_value.inline_buffer_[0] = 0;
+    break;
+  }
+  }
+  return *this;
+}
+
 HeaderMapImpl::HeaderEntryImpl::HeaderEntryImpl(const LowerCaseString& key) : key_(key) {}
 
 HeaderMapImpl::HeaderEntryImpl::HeaderEntryImpl(const LowerCaseString& key, HeaderString&& value)
@@ -206,6 +236,8 @@ void HeaderMapImpl::HeaderEntryImpl::value(const std::string& value) {
   this->value(value.c_str(), static_cast<uint32_t>(value.size()));
 }
 
+void HeaderMapImpl::HeaderEntryImpl::value(HeaderString&& value) { value_ = std::move(value); }
+
 void HeaderMapImpl::HeaderEntryImpl::value(uint64_t value) { value_.setInteger(value); }
 
 void HeaderMapImpl::HeaderEntryImpl::value(const HeaderEntry& header) {
@@ -317,10 +349,45 @@ void HeaderMapImpl::insertByKey(HeaderString&& key, HeaderString&& value) {
   }
 }
 
+void HeaderMapImpl::updateByKey(HeaderString&& key, HeaderString&& value) {
+  StaticLookupEntry::EntryCb cb = ConstSingleton<StaticLookupTable>::get().find(key.c_str());
+  if (cb) {
+    key.clear();
+    StaticLookupResponse ref_lookup_response = cb(*this);
+    maybeCreateInline(ref_lookup_response.entry_, *ref_lookup_response.key_, std::move(value));
+  } else {
+    std::list<HeaderEntryImpl>::iterator i = headers_.begin();
+    bool found = false;
+    while (i != headers_.end()) {
+      if (i->key() == key.c_str()) {
+        if (found) {
+          // Erase any other entries with this key.
+          i = headers_.erase(i);
+          continue;
+        }
+
+        i->value(std::move(value));
+        found = true;
+      }
+      ++i;
+    }
+
+    if (!found) {
+      // Not found, append.
+      i = headers_.emplace(headers_.end(), std::move(key), std::move(value));
+      i->entry_ = i;
+    }
+  }
+}
+
 void HeaderMapImpl::addViaMove(HeaderString&& key, HeaderString&& value) {
   insertByKey(std::move(key), std::move(value));
 }
 
+void HeaderMapImpl::setViaMove(HeaderString&& key, HeaderString&& value) {
+  updateByKey(std::move(key), std::move(value));
+}
+
 void HeaderMapImpl::addReference(const LowerCaseString& key, const std::string& value) {
   HeaderString ref_key(key);
   HeaderString ref_value(value);
@@ -363,6 +430,46 @@ 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);
+  setViaMove(std::move(ref_key), std::move(ref_value));
+}
+
+void HeaderMapImpl::setReferenceKey(const LowerCaseString& key, uint64_t value) {
+  HeaderString ref_key(key);
+  HeaderString new_value;
+  new_value.setInteger(value);
+  updateByKey(std::move(ref_key), std::move(new_value));
+  ASSERT(new_value.empty());
+}
+
+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());
+  updateByKey(std::move(ref_key), std::move(new_value));
+  ASSERT(new_value.empty());
+}
+
+void HeaderMapImpl::setCopy(const LowerCaseString& key, uint64_t value) {
+  HeaderString new_key;
+  new_key.setCopy(key.get().c_str(), key.get().size());
+  HeaderString new_value;
+  new_value.setInteger(value);
+  updateByKey(std::move(new_key), std::move(new_value));
+  ASSERT(new_value.empty());
+}
+
+void HeaderMapImpl::setCopy(const LowerCaseString& key, const std::string& value) {
+  HeaderString new_key;
+  new_key.setCopy(key.get().c_str(), key.get().size());
+  HeaderString new_value;
+  new_value.setCopy(value.c_str(), value.size());
+  updateByKey(std::move(new_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

@@ -47,6 +47,11 @@ class HeaderMapImpl : public HeaderMap {
    */
   void addViaMove(HeaderString&& key, HeaderString&& value);
 
+  /**
+   * Set a header via full move.
+   */
+  void setViaMove(HeaderString&& key, HeaderString&& value);
+
   /**
    * For testing. Equality is based on equality of the backing list. This is an exact match
    * comparison (order matters).
@@ -59,6 +64,11 @@ 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, uint64_t value) override;
+  void setReferenceKey(const LowerCaseString& key, const std::string& value) override;
+  void setCopy(const LowerCaseString& key, uint64_t value) override;
+  void setCopy(const LowerCaseString& key, const std::string& value) override;
 
   uint64_t byteSize() const override;
   const HeaderEntry* get(const LowerCaseString& key) const override;
@@ -77,6 +87,7 @@ class HeaderMapImpl : public HeaderMap {
     const HeaderString& key() const override { return key_; }
     void value(const char* value, uint32_t size) override;
     void value(const std::string& value) override;
+    void value(HeaderString&& value) override;
     void value(uint64_t value) override;
     void value(const HeaderEntry& header) override;
     const HeaderString& value() const override { return value_; }
@@ -116,6 +127,7 @@ class HeaderMapImpl : public HeaderMap {
   };
 
   void insertByKey(HeaderString&& key, HeaderString&& value);
+  void updateByKey(HeaderString&& key, HeaderString&& value);
   HeaderEntryImpl& maybeCreateInline(HeaderEntryImpl** entry, const LowerCaseString& key);
   HeaderEntryImpl& maybeCreateInline(HeaderEntryImpl** entry, const LowerCaseString& key,
                                      HeaderString&& value);

source/common/router/config_impl.cc

@@ -30,6 +30,16 @@
 
 namespace Envoy {
 namespace Router {
+namespace {
+
+Router::HeaderAddition
+make_header_to_add(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);
@@ -290,8 +300,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(make_header_to_add(header_value_option));
   }
 
   // Only set include_vh_rate_limits_ to true if the rate limit policy for the route is empty
@@ -606,8 +615,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(make_header_to_add(header_value_option));
   }
 
   for (const auto& route : virtual_host.routes()) {
@@ -790,17 +798,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(make_header_to_add(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(make_header_to_add(header_value_option));
   }
   request_headers_parser_ = RequestHeaderParser::parse(config.request_headers_to_add());
 }