Skip to content

Commit 72964b7

Browse files
tomerdbzgalnoam
authored andcommitted
issue: 4545376 reject JSON keys containing dots
This change enforces a single conf format by rejecting JSON keys that contain dots, preventing ambiguous configurations where both flat dot notation and nested object structures could be used. Changes: - Add validation in json_loader::process_json_object() to throw xlio_exception when keys contain dots - Convert key to std::string for proper validation - Add comprehensive unit tests covering: * Method A rejection (flat dot notation like "resources.memory_limit") * Method B acceptance (proper nested objects) * Edge cases (multiple dots, leading/trailing dots, dots-only keys) * Valid keys without dots still work correctly This resolves the issue where both configuration methods were accepted: - Method A: "core": {"resources.memory_limit": "50GB"} (now rejected) - Method B: "core": {"resources": {"memory_limit": "50GB"}} (works) Signed-off-by: Tomer Cabouly <[email protected]>
1 parent 564636e commit 72964b7

File tree

2 files changed

+111
-1
lines changed

2 files changed

+111
-1
lines changed

src/core/config/loaders/json_loader.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,13 @@ void json_loader::process_json_object(const std::string &prefix, json_object *ob
4545
{
4646
json_object_object_foreach(obj, key, value)
4747
{
48-
std::string current_key = prefix.empty() ? key : (prefix + config_strings::misc::DOT + key);
48+
std::string key_str(key);
49+
if (key_str.find('.') != std::string::npos) {
50+
throw_xlio_exception("Key cannot contain dots: " + key_str);
51+
}
52+
53+
std::string current_key =
54+
prefix.empty() ? std::move(key_str) : (prefix + config_strings::misc::DOT + key_str);
4955

5056
json_type type = json_object_get_type(value);
5157
if (type == json_type_object) {

tests/unit_tests/config/json_loader.cpp

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,110 @@ TEST(config, json_loader_array_simple)
170170
ASSERT_EQ(true, std::experimental::any_cast<bool>(bool_array[2]));
171171
}
172172

173+
// Test for dot validation - prevents keys with dots in JSON
174+
TEST(config, json_loader_rejects_keys_with_dots)
175+
{
176+
// Test Method A from the ticket - should be rejected
177+
conf_file_writer json_config(R"({
178+
"core": {
179+
"resources.memory_limit": "50GB"
180+
}
181+
})");
182+
183+
json_loader loader(json_config.get());
184+
ASSERT_THROW(loader.load_all(), xlio_exception);
185+
}
186+
187+
// Test that proper nested structure (Method B) still works
188+
TEST(config, json_loader_accepts_proper_nested_structure)
189+
{
190+
// Test Method B from the ticket - should work correctly
191+
conf_file_writer json_config(R"({
192+
"core": {
193+
"resources": {
194+
"memory_limit": "50GB"
195+
}
196+
}
197+
})");
198+
199+
json_loader loader(json_config.get());
200+
std::map<std::string, std::experimental::any> data = loader.load_all();
201+
202+
ASSERT_EQ("50GB",
203+
std::experimental::any_cast<std::string>(data["core.resources.memory_limit"]));
204+
}
205+
206+
// Test various edge cases for dot validation
207+
TEST(config, json_loader_dot_validation_edge_cases)
208+
{
209+
// Test key with multiple dots
210+
{
211+
conf_file_writer json_config(R"({
212+
"core": {
213+
"a.b.c": "value"
214+
}
215+
})");
216+
json_loader loader(json_config.get());
217+
ASSERT_THROW(loader.load_all(), xlio_exception);
218+
}
219+
220+
// Test key starting with dot
221+
{
222+
conf_file_writer json_config(R"({
223+
"core": {
224+
".hidden": "value"
225+
}
226+
})");
227+
json_loader loader(json_config.get());
228+
ASSERT_THROW(loader.load_all(), xlio_exception);
229+
}
230+
231+
// Test key ending with dot
232+
{
233+
conf_file_writer json_config(R"({
234+
"core": {
235+
"suffix.": "value"
236+
}
237+
})");
238+
json_loader loader(json_config.get());
239+
ASSERT_THROW(loader.load_all(), xlio_exception);
240+
}
241+
242+
// Test key with only dots
243+
{
244+
conf_file_writer json_config(R"({
245+
"core": {
246+
"...": "value"
247+
}
248+
})");
249+
json_loader loader(json_config.get());
250+
ASSERT_THROW(loader.load_all(), xlio_exception);
251+
}
252+
}
253+
254+
// Test that valid keys without dots still work
255+
TEST(config, json_loader_valid_keys_without_dots)
256+
{
257+
conf_file_writer json_config(R"({
258+
"core": {
259+
"log_level": 2,
260+
"memory_limit": "1GB",
261+
"enable_feature": true,
262+
"nested": {
263+
"value": "test"
264+
}
265+
}
266+
})");
267+
268+
json_loader loader(json_config.get());
269+
std::map<std::string, std::experimental::any> data = loader.load_all();
270+
271+
ASSERT_EQ(2, std::experimental::any_cast<int64_t>(data["core.log_level"]));
272+
ASSERT_EQ("1GB", std::experimental::any_cast<std::string>(data["core.memory_limit"]));
273+
ASSERT_EQ(true, std::experimental::any_cast<bool>(data["core.enable_feature"]));
274+
ASSERT_EQ("test", std::experimental::any_cast<std::string>(data["core.nested.value"]));
275+
}
276+
173277
TEST(config, json_loader_array_of_objects)
174278
{
175279
conf_file_writer json_config(R"({

0 commit comments

Comments
 (0)