router: Add do_formatting arg to responseHeaderTransforms

include/envoy/router/router.h

@@ -64,9 +64,11 @@ class ResponseEntry {
    * process them later. Note: do not use unless you are sure that there will be no route
    * modifications later in the filter chain.
    * @param stream_info holds additional information about the request.
+   * @param do_formatting whether or not to evaluate configured transformations; if false, returns
+   * original values instead.
    */
-  virtual Http::HeaderTransforms
-  responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info) const PURE;
+  virtual Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
+                                                          bool do_formatting = true) const PURE;
 };
 
 /**

source/common/http/async_client_impl.h

@@ -237,7 +237,8 @@ class AsyncStreamImpl : public AsyncClient::Stream,
                                 bool) const override {}
     void finalizeResponseHeaders(Http::ResponseHeaderMap&,
                                  const StreamInfo::StreamInfo&) const override {}
-    Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo&) const override {
+    Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo&,
+                                                    bool) const override {
       return {};
     }
     const HashPolicy* hashPolicy() const override { return hash_policy_.get(); }

source/common/router/config_impl.cc

@@ -624,24 +624,29 @@ void RouteEntryImplBase::finalizeResponseHeaders(Http::ResponseHeaderMap& header
 }
 
 Http::HeaderTransforms
-RouteEntryImplBase::responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info) const {
+RouteEntryImplBase::responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
+                                             bool do_formatting) const {
   Http::HeaderTransforms transforms;
   if (!vhost_.globalRouteConfig().mostSpecificHeaderMutationsWins()) {
     // Append user-specified request headers from most to least specific: route-level headers,
     // virtual host level headers and finally global connection manager level headers.
-    mergeTransforms(transforms, response_headers_parser_->getHeaderTransforms(stream_info));
-    mergeTransforms(transforms, vhost_.responseHeaderParser().getHeaderTransforms(stream_info));
-    mergeTransforms(
-        transforms,
-        vhost_.globalRouteConfig().responseHeaderParser().getHeaderTransforms(stream_info));
+    mergeTransforms(transforms,
+                    response_headers_parser_->getHeaderTransforms(stream_info, do_formatting));
+    mergeTransforms(transforms,
+                    vhost_.responseHeaderParser().getHeaderTransforms(stream_info, do_formatting));
+    mergeTransforms(transforms,
+                    vhost_.globalRouteConfig().responseHeaderParser().getHeaderTransforms(
+                        stream_info, do_formatting));
   } else {
     // Most specific mutations (route-level) take precedence by being applied
     // last: if a header is specified at all levels, the last one applied wins.
-    mergeTransforms(
-        transforms,
-        vhost_.globalRouteConfig().responseHeaderParser().getHeaderTransforms(stream_info));
-    mergeTransforms(transforms, vhost_.responseHeaderParser().getHeaderTransforms(stream_info));
-    mergeTransforms(transforms, response_headers_parser_->getHeaderTransforms(stream_info));
+    mergeTransforms(transforms,
+                    vhost_.globalRouteConfig().responseHeaderParser().getHeaderTransforms(
+                        stream_info, do_formatting));
+    mergeTransforms(transforms,
+                    vhost_.responseHeaderParser().getHeaderTransforms(stream_info, do_formatting));
+    mergeTransforms(transforms,
+                    response_headers_parser_->getHeaderTransforms(stream_info, do_formatting));
   }
   return transforms;
 }
@@ -1044,9 +1049,10 @@ RouteEntryImplBase::WeightedClusterEntry::WeightedClusterEntry(
 }
 
 Http::HeaderTransforms RouteEntryImplBase::WeightedClusterEntry::responseHeaderTransforms(
-    const StreamInfo::StreamInfo& stream_info) const {
-  auto transforms = response_headers_parser_->getHeaderTransforms(stream_info);
-  mergeTransforms(transforms, DynamicRouteEntry::responseHeaderTransforms(stream_info));
+    const StreamInfo::StreamInfo& stream_info, bool do_formatting) const {
+  auto transforms = response_headers_parser_->getHeaderTransforms(stream_info, do_formatting);
+  mergeTransforms(transforms,
+                  DynamicRouteEntry::responseHeaderTransforms(stream_info, do_formatting));
   return transforms;
 }
 

source/common/router/config_impl.h

@@ -96,7 +96,8 @@ class SslRedirector : public DirectResponseEntry {
   // Router::DirectResponseEntry
   void finalizeResponseHeaders(Http::ResponseHeaderMap&,
                                const StreamInfo::StreamInfo&) const override {}
-  Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo&) const override {
+  Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo&,
+                                                  bool) const override {
     return {};
   }
   std::string newPath(const Http::RequestHeaderMap& headers) const override;
@@ -501,8 +502,8 @@ class RouteEntryImplBase : public RouteEntry,
                               bool insert_envoy_original_path) const override;
   void finalizeResponseHeaders(Http::ResponseHeaderMap& headers,
                                const StreamInfo::StreamInfo& stream_info) const override;
-  Http::HeaderTransforms
-  responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info) const override;
+  Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
+                                                  bool do_formatting = true) const override;
   const Http::HashPolicy* hashPolicy() const override { return hash_policy_.get(); }
 
   const HedgePolicy& hedgePolicy() const override { return hedge_policy_; }
@@ -636,9 +637,9 @@ class RouteEntryImplBase : public RouteEntry,
                                  const StreamInfo::StreamInfo& stream_info) const override {
       return parent_->finalizeResponseHeaders(headers, stream_info);
     }
-    Http::HeaderTransforms
-    responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info) const override {
-      return parent_->responseHeaderTransforms(stream_info);
+    Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
+                                                    bool do_formatting = true) const override {
+      return parent_->responseHeaderTransforms(stream_info, do_formatting);
     }
 
     const CorsPolicy* corsPolicy() const override { return parent_->corsPolicy(); }
@@ -765,8 +766,8 @@ class RouteEntryImplBase : public RouteEntry,
       response_headers_parser_->evaluateHeaders(headers, stream_info);
       DynamicRouteEntry::finalizeResponseHeaders(headers, stream_info);
     }
-    Http::HeaderTransforms
-    responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info) const override;
+    Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
+                                                    bool do_formatting = true) const override;
 
     const RouteSpecificFilterConfig* perFilterConfig(const std::string& name) const override;
 

source/common/router/delegating_route_impl.cc

@@ -25,8 +25,9 @@ void DelegatingRouteEntry::finalizeResponseHeaders(
 }
 
 Http::HeaderTransforms
-DelegatingRouteEntry::responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info) const {
-  return base_route_->routeEntry()->responseHeaderTransforms(stream_info);
+DelegatingRouteEntry::responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
+                                               bool do_formatting) const {
+  return base_route_->routeEntry()->responseHeaderTransforms(stream_info, do_formatting);
 }
 
 const std::string& DelegatingRouteEntry::clusterName() const {

source/common/router/delegating_route_impl.h

@@ -46,8 +46,8 @@ class DelegatingRouteEntry : public Router::RouteEntry {
   // Router::ResponseEntry
   void finalizeResponseHeaders(Http::ResponseHeaderMap& headers,
                                const StreamInfo::StreamInfo& stream_info) const override;
-  Http::HeaderTransforms
-  responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info) const override;
+  Http::HeaderTransforms responseHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
+                                                  bool do_formatting = true) const override;
 
   // Router::RouteEntry
   const std::string& clusterName() const override;

source/common/router/header_parser.cc

@@ -296,17 +296,25 @@ void HeaderParser::evaluateHeaders(Http::HeaderMap& headers,
   }
 }
 
-Http::HeaderTransforms
-HeaderParser::getHeaderTransforms(const StreamInfo::StreamInfo& stream_info) const {
+Http::HeaderTransforms HeaderParser::getHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
+                                                         bool do_formatting) const {
   Http::HeaderTransforms transforms;
 
   for (const auto& [key, entry] : headers_to_add_) {
-    const std::string value = entry.formatter_->format(stream_info);
-    if (!value.empty()) {
+    if (do_formatting) {
+      const std::string value = entry.formatter_->format(stream_info);
+      if (!value.empty()) {
+        if (entry.formatter_->append()) {
+          transforms.headers_to_append.push_back({key, value});
+        } else {
+          transforms.headers_to_overwrite.push_back({key, value});
+        }
+      }
+    } else {
       if (entry.formatter_->append()) {
-        transforms.headers_to_append.push_back({key, value});
+        transforms.headers_to_append.push_back({key, entry.original_value_});
       } else {
-        transforms.headers_to_overwrite.push_back({key, value});
+        transforms.headers_to_overwrite.push_back({key, entry.original_value_});
       }
     }
   }

source/common/router/header_parser.h

@@ -55,8 +55,11 @@ class HeaderParser {
    * Same as evaluateHeaders, but returns the modifications that would have been made rather than
    * modifying an existing HeaderMap.
    * @param stream_info contains additional information about the request.
+   * @param do_formatting whether or not to evaluate configured transformations; if false, returns
+   * original values instead.
    */
-  Http::HeaderTransforms getHeaderTransforms(const StreamInfo::StreamInfo& stream_info) const;
+  Http::HeaderTransforms getHeaderTransforms(const StreamInfo::StreamInfo& stream_info,
+                                             bool do_formatting = true) const;
 
 protected:
   HeaderParser() = default;

source/docs/response_header_transforms.md

@@ -26,3 +26,6 @@ Http::FilterHeadersStatus MyFilter::decodeHeaders(
       "local_reply");
 }
 ```
+
+If you want to retrieve the original values rather than the formatted ones, pass
+`/*do_formatting=*/false` as the second argument to `responseHeaderTransforms`.

test/common/router/BUILD

@@ -29,6 +29,7 @@ envoy_cc_test_library(
         "//source/common/http:header_map_lib",
         "//source/common/http:headers_lib",
         "//source/common/router:config_lib",
+        "//source/common/router:string_accessor_lib",
         "//source/common/stream_info:filter_state_lib",
         "//test/extensions/filters/http/common:empty_http_filter_config_lib",
         "//test/fuzz:utility_lib",

test/common/router/config_impl_test.cc

@@ -18,6 +18,7 @@
 #include "common/http/headers.h"
 #include "common/network/address_impl.h"
 #include "common/router/config_impl.h"
+#include "common/router/string_accessor_impl.h"
 #include "common/stream_info/filter_state_impl.h"
 
 #include "test/common/router/route_fuzz.pb.h"
@@ -1789,6 +1790,52 @@ TEST_F(RouteMatcherTest, TestAddRemoveResponseHeadersAppendMostSpecificWins) {
                                                         Http::LowerCaseString("x-vhost-remove")));
 }
 
+TEST_F(RouteMatcherTest, TestResponseHeaderTransformsDoFormatting) {
+  factory_context_.cluster_manager_.initializeClusters({"default"}, {});
+  const std::string yaml = R"EOF(
+virtual_hosts:
+  - name: default
+    domains: ["*"]
+    routes:
+      - match:
+          prefix: "/"
+        route:
+          cluster: "default"
+response_headers_to_add:
+  - header:
+      key: x-has-variable
+      value: "%PER_REQUEST_STATE(testing)%"
+    append: false
+)EOF";
+  NiceMock<StreamInfo::MockStreamInfo> stream_info;
+
+  Envoy::StreamInfo::FilterStateSharedPtr filter_state(
+      std::make_shared<Envoy::StreamInfo::FilterStateImpl>(
+          Envoy::StreamInfo::FilterState::LifeSpan::FilterChain));
+  filter_state->setData("testing", std::make_unique<StringAccessorImpl>("test_value"),
+                        StreamInfo::FilterState::StateType::ReadOnly,
+                        StreamInfo::FilterState::LifeSpan::FilterChain);
+  ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state));
+  ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state));
+
+  TestConfigImpl config(parseRouteConfigurationFromYaml(yaml), factory_context_, true);
+
+  Http::TestRequestHeaderMapImpl req_headers =
+      genHeaders("www.lyft.com", "/new_endpoint/foo", "GET");
+  const RouteEntry* route = config.route(req_headers, 0)->routeEntry();
+  Http::TestResponseHeaderMapImpl headers;
+  route->finalizeResponseHeaders(headers, stream_info);
+
+  auto transforms = route->responseHeaderTransforms(stream_info, /*do_formatting=*/true);
+  EXPECT_THAT(transforms.headers_to_overwrite,
+              ElementsAre(Pair(Http::LowerCaseString("x-has-variable"), "test_value")));
+
+  transforms = route->responseHeaderTransforms(stream_info, /*do_formatting=*/false);
+  EXPECT_THAT(
+      transforms.headers_to_overwrite,
+      ElementsAre(Pair(Http::LowerCaseString("x-has-variable"), "%PER_REQUEST_STATE(testing)%")));
+}
+
 TEST_F(RouteMatcherTest, TestAddGlobalResponseHeaderRemoveFromRoute) {
   const std::string yaml = R"EOF(
 virtual_hosts:

test/common/router/header_formatter_test.cc

@@ -1438,7 +1438,7 @@ response_headers_to_remove: ["x-foo-header"]
   EXPECT_EQ("bar", header_map.get_("x-foo-header"));
 }
 
-TEST(HeaderParserTest, GetHeaderTransforms) {
+TEST(HeaderParserTest, GetHeaderTransformsWithFormatting) {
   const std::string yaml = R"EOF(
 match: { prefix: "/new_endpoint" }
 route:
@@ -1452,6 +1452,10 @@ match: { prefix: "/new_endpoint" }
       key: "x-bar-header"
       value: "bar"
     append: false
+  - header:
+      key: "x-per-request-header"
+      value: "%PER_REQUEST_STATE(testing)%"
+    append: false
 response_headers_to_remove: ["x-baz-header"]
 )EOF";
 
@@ -1460,11 +1464,67 @@ response_headers_to_remove: ["x-baz-header"]
       HeaderParser::configure(route.response_headers_to_add(), route.response_headers_to_remove());
   NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
 
+  Envoy::StreamInfo::FilterStateSharedPtr filter_state(
+      std::make_shared<Envoy::StreamInfo::FilterStateImpl>(
+          Envoy::StreamInfo::FilterState::LifeSpan::FilterChain));
+  filter_state->setData("testing", std::make_unique<StringAccessorImpl>("test_value"),
+                        StreamInfo::FilterState::StateType::ReadOnly,
+                        StreamInfo::FilterState::LifeSpan::FilterChain);
+  ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state));
+  ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state));
+
   auto transforms = resp_header_parser->getHeaderTransforms(stream_info);
   EXPECT_THAT(transforms.headers_to_append,
               ElementsAre(Pair(Http::LowerCaseString("x-foo-header"), "foo")));
   EXPECT_THAT(transforms.headers_to_overwrite,
-              ElementsAre(Pair(Http::LowerCaseString("x-bar-header"), "bar")));
+              ElementsAre(Pair(Http::LowerCaseString("x-bar-header"), "bar"),
+                          Pair(Http::LowerCaseString("x-per-request-header"), "test_value")));
+  EXPECT_THAT(transforms.headers_to_remove, ElementsAre(Http::LowerCaseString("x-baz-header")));
+}
+
+TEST(HeaderParserTest, GetHeaderTransformsOriginalValues) {
+  const std::string yaml = R"EOF(
+match: { prefix: "/new_endpoint" }
+route:
+  cluster: www2
+response_headers_to_add:
+  - header:
+      key: "x-foo-header"
+      value: "foo"
+    append: true
+  - header:
+      key: "x-bar-header"
+      value: "bar"
+    append: false
+  - header:
+      key: "x-per-request-header"
+      value: "%PER_REQUEST_STATE(testing)%"
+    append: false
+response_headers_to_remove: ["x-baz-header"]
+)EOF";
+
+  const auto route = parseRouteFromV3Yaml(yaml);
+  HeaderParserPtr response_header_parser =
+      HeaderParser::configure(route.response_headers_to_add(), route.response_headers_to_remove());
+  NiceMock<Envoy::StreamInfo::MockStreamInfo> stream_info;
+
+  Envoy::StreamInfo::FilterStateSharedPtr filter_state(
+      std::make_shared<Envoy::StreamInfo::FilterStateImpl>(
+          Envoy::StreamInfo::FilterState::LifeSpan::FilterChain));
+  filter_state->setData("testing", std::make_unique<StringAccessorImpl>("test_value"),
+                        StreamInfo::FilterState::StateType::ReadOnly,
+                        StreamInfo::FilterState::LifeSpan::FilterChain);
+  ON_CALL(stream_info, filterState()).WillByDefault(ReturnRef(filter_state));
+  ON_CALL(Const(stream_info), filterState()).WillByDefault(ReturnRef(*filter_state));
+
+  auto transforms =
+      response_header_parser->getHeaderTransforms(stream_info, /*do_formatting=*/false);
+  EXPECT_THAT(transforms.headers_to_append,
+              ElementsAre(Pair(Http::LowerCaseString("x-foo-header"), "foo")));
+  EXPECT_THAT(transforms.headers_to_overwrite,
+              ElementsAre(Pair(Http::LowerCaseString("x-bar-header"), "bar"),
+                          Pair(Http::LowerCaseString("x-per-request-header"),
+                               "%PER_REQUEST_STATE(testing)%")));
   EXPECT_THAT(transforms.headers_to_remove, ElementsAre(Http::LowerCaseString("x-baz-header")));
 }
 

test/mocks/router/mocks.h

@@ -50,7 +50,7 @@ class MockDirectResponseEntry : public DirectResponseEntry {
               (Http::ResponseHeaderMap & headers, const StreamInfo::StreamInfo& stream_info),
               (const));
   MOCK_METHOD(Http::HeaderTransforms, responseHeaderTransforms,
-              (const StreamInfo::StreamInfo& stream_info), (const));
+              (const StreamInfo::StreamInfo& stream_info, bool do_formatting), (const));
   MOCK_METHOD(std::string, newPath, (const Http::RequestHeaderMap& headers), (const));
   MOCK_METHOD(void, rewritePathHeader,
               (Http::RequestHeaderMap & headers, bool insert_envoy_original_path), (const));
@@ -361,7 +361,7 @@ class MockRouteEntry : public RouteEntry {
               (Http::ResponseHeaderMap & headers, const StreamInfo::StreamInfo& stream_info),
               (const));
   MOCK_METHOD(Http::HeaderTransforms, responseHeaderTransforms,
-              (const StreamInfo::StreamInfo& stream_info), (const));
+              (const StreamInfo::StreamInfo& stream_info, bool do_formatting), (const));
   MOCK_METHOD(const Http::HashPolicy*, hashPolicy, (), (const));
   MOCK_METHOD(const HedgePolicy&, hedgePolicy, (), (const));
   MOCK_METHOD(const Router::MetadataMatchCriteria*, metadataMatchCriteria, (), (const));