http: fix cookie attributes (envoyproxy#34885)

--------- Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
juanmolle · Jun 27, 2024 · c213589 · c213589
1 parent cb515cb
commit c213589
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 3 deletions.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -218,6 +218,10 @@ bug_fixes:
   change: |
     Fixed missing :ref:`additional addresses <envoy_v3_api_msg_config.endpoint.v3.Endpoint.AdditionalAddress>`
     for :ref:`LbEndpoint <envoy_v3_api_field_config.endpoint.v3.LbEndpoint.endpoint>` in config dump.
+- area: http
+  change: |
+    Fixed a bug where additional :ref:`cookie attributes <envoy_v3_api_msg_config.route.v3.RouteAction.HashPolicy.cookie>`
+    are not sent properly to clients.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

diff --git a/source/common/http/hash_policy.cc b/source/common/http/hash_policy.cc
@@ -82,7 +82,11 @@ class CookieHashMethod : public HashMethodImplBase {
   CookieHashMethod(const std::string& key, const std::string& path,
                    const absl::optional<std::chrono::seconds>& ttl, bool terminal,
                    const CookieAttributeRefVector attributes)
-      : HashMethodImplBase(terminal), key_(key), path_(path), ttl_(ttl), attributes_(attributes) {}
+      : HashMethodImplBase(terminal), key_(key), path_(path), ttl_(ttl) {
+    for (const auto& attribute : attributes) {
+      attributes_.push_back(attribute);
+    }
+  }
 
   absl::optional<uint64_t> evaluate(const Network::Address::Instance*,
                                     const RequestHeaderMap& headers,
@@ -91,7 +95,11 @@ class CookieHashMethod : public HashMethodImplBase {
     absl::optional<uint64_t> hash;
     std::string value = Utility::parseCookieValue(headers, key_);
     if (value.empty() && ttl_.has_value()) {
-      value = add_cookie(key_, path_, ttl_.value(), attributes_);
+      CookieAttributeRefVector attributes;
+      for (const auto& attribute : attributes_) {
+        attributes.push_back(attribute);
+      }
+      value = add_cookie(key_, path_, ttl_.value(), attributes);
       hash = HashUtil::xxHash64(value);
 
     } else if (!value.empty()) {
@@ -104,7 +112,7 @@ class CookieHashMethod : public HashMethodImplBase {
   const std::string key_;
   const std::string path_;
   const absl::optional<std::chrono::seconds> ttl_;
-  const CookieAttributeRefVector attributes_;
+  std::vector<CookieAttribute> attributes_;
 };
 
 class IpHashMethod : public HashMethodImplBase {

diff --git a/test/integration/multiplexed_integration_test.cc b/test/integration/multiplexed_integration_test.cc
@@ -1841,6 +1841,45 @@ TEST_P(MultiplexedRingHashIntegrationTest, CookieRoutingNoCookieWithNonzeroTtlSe
   EXPECT_EQ(set_cookies.size(), 1);
 }
 
+TEST_P(MultiplexedRingHashIntegrationTest,
+       CookieRoutingNoCookieWithNonzeroTtlSetAndWithAttributes) {
+  config_helper_.addConfigModifier(
+      [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
+              hcm) -> void {
+        auto* hash_policy = hcm.mutable_route_config()
+                                ->mutable_virtual_hosts(0)
+                                ->mutable_routes(0)
+                                ->mutable_route()
+                                ->add_hash_policy();
+        auto* cookie = hash_policy->mutable_cookie();
+        cookie->set_name("foo");
+        cookie->mutable_ttl()->set_seconds(15);
+        auto* attribute_1 = cookie->mutable_attributes()->Add();
+        attribute_1->set_name("test1");
+        attribute_1->set_value("value1");
+        auto* attribute_2 = cookie->mutable_attributes()->Add();
+        attribute_2->set_name("test2");
+        attribute_2->set_value("value2");
+      });
+
+  std::set<std::string> set_cookies;
+  sendMultipleRequests(
+      1024,
+      Http::TestRequestHeaderMapImpl{{":method", "POST"},
+                                     {":path", "/test/long/url"},
+                                     {":scheme", "http"},
+                                     {":authority", "host"}},
+      [&](IntegrationStreamDecoder& response) {
+        EXPECT_EQ("200", response.headers().getStatusValue());
+        std::string value(
+            response.headers().get(Http::Headers::get().SetCookie)[0]->value().getStringView());
+        set_cookies.insert(value);
+        EXPECT_THAT(value,
+                    MatchesRegex("foo=.*; Max-Age=15; test1=value1; test2=value2; HttpOnly"));
+      });
+  EXPECT_EQ(set_cookies.size(), 1);
+}
+
 TEST_P(MultiplexedRingHashIntegrationTest, CookieRoutingNoCookieWithZeroTtlSet) {
   config_helper_.addConfigModifier(
       [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&