Skip to content

Commit 50b384c

Browse files
ramaraochavaliphlax
authored andcommitted
http: fix cookie attributes (#34885)
--------- Signed-off-by: Rama Chavali <[email protected]>
1 parent ddc00c9 commit 50b384c

File tree

3 files changed

+54
-3
lines changed

3 files changed

+54
-3
lines changed

changelogs/current.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ minor_behavior_changes:
88

99
bug_fixes:
1010
# *Changes expected to improve the state of the world and are unlikely to have negative effects*
11+
- area: http
12+
change: |
13+
Fixed a bug where additional :ref:`cookie attributes <envoy_v3_api_msg_config.route.v3.RouteAction.HashPolicy.cookie>`
14+
are not sent properly to clients.
1115
1216
removed_config_or_runtime:
1317
# *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

source/common/http/hash_policy.cc

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,11 @@ class CookieHashMethod : public HashMethodImplBase {
8282
CookieHashMethod(const std::string& key, const std::string& path,
8383
const absl::optional<std::chrono::seconds>& ttl, bool terminal,
8484
const CookieAttributeRefVector attributes)
85-
: HashMethodImplBase(terminal), key_(key), path_(path), ttl_(ttl), attributes_(attributes) {}
85+
: HashMethodImplBase(terminal), key_(key), path_(path), ttl_(ttl) {
86+
for (const auto& attribute : attributes) {
87+
attributes_.push_back(attribute);
88+
}
89+
}
8690

8791
absl::optional<uint64_t> evaluate(const Network::Address::Instance*,
8892
const RequestHeaderMap& headers,
@@ -91,7 +95,11 @@ class CookieHashMethod : public HashMethodImplBase {
9195
absl::optional<uint64_t> hash;
9296
std::string value = Utility::parseCookieValue(headers, key_);
9397
if (value.empty() && ttl_.has_value()) {
94-
value = add_cookie(key_, path_, ttl_.value(), attributes_);
98+
CookieAttributeRefVector attributes;
99+
for (const auto& attribute : attributes_) {
100+
attributes.push_back(attribute);
101+
}
102+
value = add_cookie(key_, path_, ttl_.value(), attributes);
95103
hash = HashUtil::xxHash64(value);
96104

97105
} else if (!value.empty()) {
@@ -104,7 +112,7 @@ class CookieHashMethod : public HashMethodImplBase {
104112
const std::string key_;
105113
const std::string path_;
106114
const absl::optional<std::chrono::seconds> ttl_;
107-
const CookieAttributeRefVector attributes_;
115+
std::vector<CookieAttribute> attributes_;
108116
};
109117

110118
class IpHashMethod : public HashMethodImplBase {

test/integration/multiplexed_integration_test.cc

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1847,6 +1847,45 @@ TEST_P(MultiplexedRingHashIntegrationTest, CookieRoutingNoCookieWithNonzeroTtlSe
18471847
EXPECT_EQ(set_cookies.size(), 1);
18481848
}
18491849

1850+
TEST_P(MultiplexedRingHashIntegrationTest,
1851+
CookieRoutingNoCookieWithNonzeroTtlSetAndWithAttributes) {
1852+
config_helper_.addConfigModifier(
1853+
[&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
1854+
hcm) -> void {
1855+
auto* hash_policy = hcm.mutable_route_config()
1856+
->mutable_virtual_hosts(0)
1857+
->mutable_routes(0)
1858+
->mutable_route()
1859+
->add_hash_policy();
1860+
auto* cookie = hash_policy->mutable_cookie();
1861+
cookie->set_name("foo");
1862+
cookie->mutable_ttl()->set_seconds(15);
1863+
auto* attribute_1 = cookie->mutable_attributes()->Add();
1864+
attribute_1->set_name("test1");
1865+
attribute_1->set_value("value1");
1866+
auto* attribute_2 = cookie->mutable_attributes()->Add();
1867+
attribute_2->set_name("test2");
1868+
attribute_2->set_value("value2");
1869+
});
1870+
1871+
std::set<std::string> set_cookies;
1872+
sendMultipleRequests(
1873+
1024,
1874+
Http::TestRequestHeaderMapImpl{{":method", "POST"},
1875+
{":path", "/test/long/url"},
1876+
{":scheme", "http"},
1877+
{":authority", "host"}},
1878+
[&](IntegrationStreamDecoder& response) {
1879+
EXPECT_EQ("200", response.headers().getStatusValue());
1880+
std::string value(
1881+
response.headers().get(Http::Headers::get().SetCookie)[0]->value().getStringView());
1882+
set_cookies.insert(value);
1883+
EXPECT_THAT(value,
1884+
MatchesRegex("foo=.*; Max-Age=15; test1=value1; test2=value2; HttpOnly"));
1885+
});
1886+
EXPECT_EQ(set_cookies.size(), 1);
1887+
}
1888+
18501889
TEST_P(MultiplexedRingHashIntegrationTest, CookieRoutingNoCookieWithZeroTtlSet) {
18511890
config_helper_.addConfigModifier(
18521891
[&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&

0 commit comments

Comments
 (0)