Skip to content

Commit 742a06b

Browse files
egorovcharenkometa-codesync[bot]
authored andcommitted
Allow colons in config parameter values
Summary: Parse string-map options at the first colon so substituted metadata values such as ray::RayTrainWorker cannot invalidate the complete config_params map and drop routing selectors. Preserve validation for missing delimiters and empty keys. Reviewed By: vrishal Differential Revision: D113784160 fbshipit-source-id: 7893c16325a1214be9cbd2a7855b3a8d31509e96
1 parent a82d7dd commit 742a06b

2 files changed

Lines changed: 30 additions & 1 deletion

File tree

mcrouter/options.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ to(const string& s) {
9090
string key;
9191
string value;
9292
facebook::memcache::checkLogic(
93-
folly::split(':', it, key, value),
93+
folly::split<false>(':', it, key, value) && !key.empty(),
9494
"Invalid string map pair: '{}'. Expected name:value.",
9595
it);
9696
result.emplace(std::move(key), std::move(value));

mcrouter/test/cpp_unit_tests/options_test.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,32 @@ TEST(OptionsSetFromDictTest, sanity) {
5656
EXPECT_TRUE(e.empty());
5757
EXPECT_TRUE(opts.enable_tw_crash_config_backup_path);
5858
}
59+
60+
TEST(OptionsSetFromDictTest, StringMapValueMayContainColons) {
61+
McrouterOptions opts;
62+
const unordered_map<string, string> dict{
63+
{"config_params", "proc-name:ray::RayTrainWorker,environment:prod"}};
64+
65+
const auto errors = opts.updateFromDict(dict);
66+
67+
EXPECT_TRUE(errors.empty());
68+
EXPECT_EQ(opts.config_params.at("proc-name"), "ray::RayTrainWorker");
69+
EXPECT_EQ(opts.config_params.at("environment"), "prod");
70+
}
71+
72+
TEST(OptionsSetFromDictTest, StringMapRejectsMalformedPairs) {
73+
for (const auto& value : {"missing-delimiter", ":missing-name"}) {
74+
McrouterOptions opts;
75+
opts.config_params = {{"existing", "value"}};
76+
const unordered_map<string, string> dict{{"config_params", value}};
77+
78+
const auto errors = opts.updateFromDict(dict);
79+
80+
ASSERT_EQ(errors.size(), 1);
81+
EXPECT_EQ(errors[0].requestedName, "config_params");
82+
EXPECT_EQ(errors[0].requestedValue, value);
83+
EXPECT_EQ(
84+
opts.config_params,
85+
(unordered_map<string, string>{{"existing", "value"}}));
86+
}
87+
}

0 commit comments

Comments
 (0)