router: allow headers with multiple variables and static data

RAW_RELEASE_NOTES.md

@@ -38,3 +38,4 @@ final version.
 * Added `gateway-error` retry-on policy.
 * Added support for building envoy with exported symbols
   This change allows scripts loaded with the lua filter to load shared object libraries such as those installed via luarocks.
+* Added support for custom request/response headers with mixed static and dynamic values.

source/common/router/header_formatter.cc

@@ -1,5 +1,6 @@
 #include "common/router/header_formatter.h"
 
+#include <regex>
 #include <string>
 
 #include "envoy/common/optional.h"
@@ -11,6 +12,7 @@
 #include "common/json/json_loader.h"
 #include "common/request_info/utility.h"
 
+#include "absl/strings/str_cat.h"
 #include "fmt/format.h"
 
 namespace Envoy {
@@ -19,16 +21,26 @@ namespace Router {
 namespace {
 
 std::string formatUpstreamMetadataParseException(const std::string& params,
-                                                 const EnvoyException* cause = nullptr) {
-  std::string reason;
-  if (cause != nullptr) {
-    reason = fmt::format(", because {}", cause->what());
+                                                 const std::string& reason) {
+  std::string formatted_reason;
+  if (!reason.empty()) {
+    formatted_reason = fmt::format(", because {}", reason);
   }
 
   return fmt::format("Incorrect header configuration. Expected format "
                      "UPSTREAM_METADATA([\"namespace\", \"k\", ...]), actual format "
                      "UPSTREAM_METADATA{}{}",
-                     params, reason);
+                     params, formatted_reason);
+}
+
+std::string formatUpstreamMetadataParseException(const std::string& params,
+                                                 const EnvoyException* cause = nullptr) {
+  std::string reason;
+  if (cause != nullptr) {
+    reason = fmt::format("{}", cause->what());
+  }
+
+  return formatUpstreamMetadataParseException(params, reason);
 }
 
 // Parses the parameters for UPSTREAM_METADATA and returns a function suitable for accessing the
@@ -37,10 +49,6 @@ std::string formatUpstreamMetadataParseException(const std::string& params,
 // There must be at least 2 array elements (a metadata namespace and at least 1 key).
 std::function<std::string(const Envoy::RequestInfo::RequestInfo&)>
 parseUpstreamMetadataField(const std::string& params_str) {
-  if (params_str.empty() || params_str.front() != '(' || params_str.back() != ')') {
-    throw EnvoyException(formatUpstreamMetadataParseException(params_str));
-  }
-
   std::vector<std::string> params;
   try {
     Json::ObjectSharedPtr parsed_params =
@@ -111,26 +119,112 @@ parseUpstreamMetadataField(const std::string& params_str) {
   };
 }
 
+// Given a string staring with `(["a", "b", "c"])`, produce a function suitable for accessing the
+// selected metadata and update the len reference to indicate how many characters were consumed.
+std::function<std::string(const Envoy::RequestInfo::RequestInfo&)>
+parseUpstreamMetadataParams(absl::string_view params, size_t& len) {
+  // Use a regex to determine where the params end. Without encoding JSON string rules for the
+  // parameters it's very difficult to distinguish the sequence ")%" within the params from the
+  // sequence ")%" at the end of the params (especially given that another %-expression may
+  // immediately follow).
+  static std::string string_match = R"EOF("(?:[^"\\[:cntrl:]]|\\["\\bfnrtu])*")EOF";
+  static std::regex params_regex(
+      R"EOF(^\(\s*\[\s*)EOF" + string_match +
+      R"EOF(\s*(?:,\s*)EOF" + string_match +
+      R"EOF(\s*)*\]\s*\))EOF");
+
+  std::string params_str = absl::StrCat(params);
+
+  if (params_str.empty() || params_str[0] == '%') {
+    throw EnvoyException(formatUpstreamMetadataParseException(""));
+  }
+
+  if (params_str.substr(0, 2) == "()") {
+    throw EnvoyException(formatUpstreamMetadataParseException("()"));
+  }
+
+  std::smatch params_match;
+  if (!std::regex_search(params_str, params_match, params_regex)) {
+    throw EnvoyException(
+        formatUpstreamMetadataParseException(params_str, "JSON supplied is not valid."));
+  }
+
+  len = params_match.length(0);
+  if (len >= params.size() || params[len] != '%') {
+    // No trailing %
+    len = std::string::npos;
+    return nullptr;
+  }
+
+  return parseUpstreamMetadataField(params_match.str(0));
+}
+
 } // namespace
 
-RequestInfoHeaderFormatter::RequestInfoHeaderFormatter(const std::string& field_name, bool append)
-    : append_(append) {
-  if (field_name == "PROTOCOL") {
-    field_extractor_ = [](const Envoy::RequestInfo::RequestInfo& request_info) {
-      return Envoy::AccessLog::AccessLogFormatUtils::protocolToString(request_info.protocol());
-    };
-  } else if (field_name == "CLIENT_IP" || field_name == "DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT") {
+#define CLIENT_IP_VAR "%CLIENT_IP%"
+#define DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT_VAR "%DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT%"
+#define PROTOCOL_VAR "%PROTOCOL%"
+#define UPSTREAM_METADATA_VAR_PREFIX "%UPSTREAM_METADATA"
+
+HeaderFormatterPtr RequestInfoHeaderFormatter::parse(absl::string_view var, bool append,
+                                                     size_t& len) {
+  if (var.size() < 2 || var[0] != '%') {
+    len = std::string::npos;
+    return nullptr;
+  }
+
+  if (var.compare(0, sizeof(PROTOCOL_VAR) - 1, PROTOCOL_VAR) == 0) {
+    len = sizeof(PROTOCOL_VAR) - 1;
+    return HeaderFormatterPtr{new RequestInfoHeaderFormatter(
+        [](const Envoy::RequestInfo::RequestInfo& request_info) {
+          return Envoy::AccessLog::AccessLogFormatUtils::protocolToString(request_info.protocol());
+        },
+        append)};
+  }
+
+  if (var.compare(0, sizeof(CLIENT_IP_VAR) - 1, CLIENT_IP_VAR) == 0) {
     // DEPRECATED: "CLIENT_IP" will be removed post 1.6.0.
-    field_extractor_ = [](const Envoy::RequestInfo::RequestInfo& request_info) {
-      return RequestInfo::Utility::formatDownstreamAddressNoPort(
-          *request_info.downstreamRemoteAddress());
-    };
-  } else if (StringUtil::startsWith(field_name.c_str(), "UPSTREAM_METADATA")) {
-    field_extractor_ =
-        parseUpstreamMetadataField(field_name.substr(sizeof("UPSTREAM_METADATA") - 1));
+    len = sizeof(CLIENT_IP_VAR) - 1;
+    return HeaderFormatterPtr{new RequestInfoHeaderFormatter(
+        [](const Envoy::RequestInfo::RequestInfo& request_info) {
+          return RequestInfo::Utility::formatDownstreamAddressNoPort(
+              *request_info.downstreamRemoteAddress());
+        },
+        append)};
+  }
+
+  if (var.compare(0, sizeof(DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT_VAR) - 1,
+                  DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT_VAR) == 0) {
+    len = sizeof(DOWNSTREAM_REMOTE_ADDRESS_WITHOUT_PORT_VAR) - 1;
+    return HeaderFormatterPtr{new RequestInfoHeaderFormatter(
+        [](const Envoy::RequestInfo::RequestInfo& request_info) {
+          return RequestInfo::Utility::formatDownstreamAddressNoPort(
+              *request_info.downstreamRemoteAddress());
+        },
+        append)};
+  }
+
+  if (var.compare(0, sizeof(UPSTREAM_METADATA_VAR_PREFIX) - 1, UPSTREAM_METADATA_VAR_PREFIX) == 0) {
+    auto extractor =
+        parseUpstreamMetadataParams(var.substr(sizeof(UPSTREAM_METADATA_VAR_PREFIX) - 1), len);
+    if (len != std::string::npos) {
+      len += sizeof(UPSTREAM_METADATA_VAR_PREFIX);
+    }
+    if (extractor == nullptr) {
+      return nullptr;
+    }
+
+    return HeaderFormatterPtr{new RequestInfoHeaderFormatter(extractor, append)};
+  }
+
+  // Find next %, if any.
+  size_t pos = var.find("%", 1);
+  if (pos == absl::string_view::npos) {
+    len = std::string::npos;
   } else {
-    throw EnvoyException(fmt::format("field '{}' not supported as custom header", field_name));
+    len = pos + 1;
   }
+  return nullptr;
 }
 
 const std::string

source/common/router/header_formatter.h

@@ -2,10 +2,13 @@
 
 #include <functional>
 #include <memory>
+#include <sstream>
 #include <string>
 
 #include "envoy/access_log/access_log.h"
 
+#include "absl/strings/string_view.h"
+
 namespace Envoy {
 namespace Router {
 
@@ -32,13 +35,31 @@ typedef std::unique_ptr<HeaderFormatter> HeaderFormatterPtr;
  */
 class RequestInfoHeaderFormatter : public HeaderFormatter {
 public:
-  RequestInfoHeaderFormatter(const std::string& field_name, bool append);
+  /*
+   * Constructs a RequestInfoHeaderFormatter. The view should contain the complete variable
+   * definition, include leading and trailing % symbols. It may include further content. If
+   * successful, a HeaderFormatterPtr is returned and the len reference is updated with the length
+   * of the variable definition, including the delimiters. Otherwise a nullptr is returned and len
+   * may be the length of the variable definition (provided the trailing % was found or std::npos).
+   *
+   * @param var an absl::string_view containing at least the variable definition.
+   * @param append controls whether the header data should be appended or not.
+   * @param len a size_t& updated to contain the length of the variable definition (or std::npos
+   *            if the definition is un-terminated).
+   * @return a HeaderFormatterPtr on success or nullptr on failure
+   */
+  static HeaderFormatterPtr parse(absl::string_view var, bool append, size_t& len);
 
   // HeaderFormatter::format
   const std::string format(const Envoy::RequestInfo::RequestInfo& request_info) const override;
   bool append() const override { return append_; }
 
 private:
+  RequestInfoHeaderFormatter(
+      std::function<std::string(const Envoy::RequestInfo::RequestInfo&)> field_extractor,
+      bool append)
+      : field_extractor_(field_extractor), append_(append){};
+
   std::function<std::string(const Envoy::RequestInfo::RequestInfo&)> field_extractor_;
   const bool append_;
 };
@@ -62,5 +83,28 @@ class PlainHeaderFormatter : public HeaderFormatter {
   const bool append_;
 };
 
+/**
+ * A formatter that produces a value by concatenating the results of multiple HeaderFormatters.
+ */
+class CompoundHeaderFormatter : public HeaderFormatter {
+public:
+  CompoundHeaderFormatter(std::vector<HeaderFormatterPtr>&& formatters, bool append)
+      : formatters_(std::move(formatters)), append_(append){};
+
+  // HeaderFormatter::format
+  const std::string format(const Envoy::RequestInfo::RequestInfo& request_info) const override {
+    std::ostringstream buf;
+    for (const auto& formatter : formatters_) {
+      buf << formatter->format(request_info);
+    }
+    return buf.str();
+  };
+  bool append() const override { return append_; }
+
+private:
+  const std::vector<HeaderFormatterPtr> formatters_;
+  const bool append_;
+};
+
 } // namespace Router
 } // namespace Envoy

source/common/router/header_parser.cc

@@ -1,7 +1,13 @@
 #include "common/router/header_parser.h"
 
+#include <string>
+
+#include "common/common/assert.h"
 #include "common/protobuf/utility.h"
 
+#include "absl/strings/str_cat.h"
+#include "absl/strings/string_view.h"
+
 namespace Envoy {
 namespace Router {
 
@@ -11,18 +17,70 @@ HeaderFormatterPtr parseInternal(const envoy::api::v2::HeaderValueOption& header
   const std::string& format = header_value_option.header().value();
   const bool append = PROTOBUF_GET_WRAPPED_OR_DEFAULT(header_value_option, append, true);
 
-  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));
+  std::vector<HeaderFormatterPtr> formatters;
+
+  const absl::string_view format_view(format);
+  std::string buf;
+  buf.reserve(format_view.size());
+
+  size_t prev = 0;
+  do {
+    bool flush_buf = false;
+    HeaderFormatterPtr formatter;
+    size_t pos = format_view.find("%", prev);
+    if (pos == absl::string_view::npos) {
+      // End of format.
+      absl::StrAppend(&buf, format_view.substr(prev));
+      prev = format_view.size();
+      flush_buf = true;
+    } else if (pos + 1 < format_view.size() && format_view[pos + 1] == '%') {
+      // Found "%%", copy the first % into the buffer and skip the second.
+      absl::StrAppend(&buf, format_view.substr(prev, pos + 1 - prev));
+      prev = pos + 2;
+      flush_buf = prev >= format_view.size();
+    } else {
+      // Found the start of a %variable%.
+      absl::StrAppend(&buf, format_view.substr(prev, pos - prev));
+      flush_buf = true;
+
+      size_t len = std::string::npos;
+      absl::string_view var_view = format_view.substr(pos);
+      formatter = RequestInfoHeaderFormatter::parse(var_view, append, len);
+      if (!formatter) {
+        if (len == std::string::npos) {
+          throw EnvoyException(fmt::format(
+              "Incorrect header configuration. Un-terminated variable expression in '{}'",
+              absl::StrCat(var_view)));
+        }
+
+        throw EnvoyException(
+            fmt::format("Incorrect header configuration. Variable '{}' is not supported",
+                        absl::StrCat(var_view.substr(0, len))));
+      }
+
+      ASSERT(len != std::string::npos);
+      prev = pos + len;
+      ASSERT(prev <= format_view.size());
+    }
+
+    if (flush_buf && !buf.empty()) {
+      formatters.emplace_back(new PlainHeaderFormatter(buf, append));
+      buf.clear();
     }
-    return HeaderFormatterPtr{
-        new RequestInfoHeaderFormatter(format.substr(1, last_occ_pos - 1), append)};
-  } else {
-    return HeaderFormatterPtr{new PlainHeaderFormatter(format, append)};
+
+    if (formatter) {
+      formatters.push_back(std::move(formatter));
+    }
+  } while (prev < format_view.size());
+
+  ASSERT(buf.empty());
+  ASSERT(formatters.size() > 0);
+
+  if (formatters.size() == 1) {
+    return std::move(formatters[0]);
   }
+
+  return HeaderFormatterPtr{new CompoundHeaderFormatter(std::move(formatters), append)};
 }
 
 } // namespace

test/common/router/config_impl_test.cc

@@ -3165,8 +3165,7 @@ TEST(CustomRequestHeadersTest, CustomHeaderWrongFormat) {
   NiceMock<Envoy::RequestInfo::MockRequestInfo> request_info;
   EXPECT_THROW_WITH_MESSAGE(
       ConfigImpl config(parseRouteConfigurationFromJson(json), runtime, cm, true), EnvoyException,
-      "Incorrect header configuration. Expected variable format %<variable_name>%, actual format "
-      "%CLIENT_IP");
+      "Incorrect header configuration. Un-terminated variable expression in '%CLIENT_IP'");
 }
 
 TEST(MetadataMatchCriteriaImpl, Create) {