From 929631f26d1615639f2b8cc693fa4fc4b744ad67 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 28 Dec 2020 18:58:24 +0800 Subject: [PATCH 1/4] feat: add validation for value updating through http api --- include/dsn/utility/flags.h | 8 ++++---- src/utils/flags.cpp | 18 ++++++++++-------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index da23db64fb..a3b909cd10 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -65,9 +65,8 @@ struct hash // The program corrupts if the validation failed. #define DSN_DEFINE_validator(name, validator) \ static auto FLAGS_VALIDATOR_FN_##name = validator; \ - static const dsn::flag_validator FLAGS_VALIDATOR_##name(#name, []() { \ - dassert(FLAGS_VALIDATOR_FN_##name(FLAGS_##name), "validation failed: %s", #name); \ - }) + static const dsn::flag_validator FLAGS_VALIDATOR_##name( \ + #name, []() { return FLAGS_VALIDATOR_FN_##name(FLAGS_##name); }) #define DSN_TAG_VARIABLE(name, tag) \ COMPILE_ASSERT(sizeof(decltype(FLAGS_##name)), exist_##name##_##tag); \ @@ -89,10 +88,11 @@ class flag_registerer }; // An utility class that registers a validator upon initialization. +using validator_fn = std::function; class flag_validator { public: - flag_validator(const char *name, std::function); + flag_validator(const char *name, validator_fn); }; class flag_tagger diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index bf84e3ff0e..114460d6d7 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include @@ -27,8 +27,6 @@ enum value_type FV_MAX_INDEX = 6, }; -using validator_fn = std::function; - class flag_data { public: @@ -36,18 +34,22 @@ class flag_data case type_enum: \ value() = dsn_config_get_value_##suffix(_section, _name, value(), _desc); \ if (_validator) { \ - _validator(); \ + dassert_f(_validator(), "validation failed: {}", _name); \ } \ break #define FLAG_DATA_UPDATE_CASE(type, type_enum, suffix) \ - case type_enum: \ - type tmpval_##type_enum; \ + case type_enum: { \ + type old_val = value(), tmpval_##type_enum; \ if (!dsn::buf2##suffix(val, tmpval_##type_enum)) { \ - return error_s::make(ERR_INVALID_PARAMETERS, fmt::format("{} in invalid", val)); \ + return error_s::make(ERR_INVALID_PARAMETERS, fmt::format("{} is invalid", val)); \ } \ value() = tmpval_##type_enum; \ - break + if (!_validator()) { \ + value() = old_val; \ + return error_s::make(ERR_INVALID_PARAMETERS, "value validation failed"); \ + } \ + } break #define FLAG_DATA_UPDATE_STRING() \ case FV_STRING: \ From eab030bfcc5f0294b9ab0e2854f2eddf0bb7bcb5 Mon Sep 17 00:00:00 2001 From: zhao liwei Date: Mon, 28 Dec 2020 19:25:58 +0800 Subject: [PATCH 2/4] Update include/dsn/utility/flags.h Co-authored-by: Wu Tao --- include/dsn/utility/flags.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index a3b909cd10..c4b5c63458 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -66,7 +66,7 @@ struct hash #define DSN_DEFINE_validator(name, validator) \ static auto FLAGS_VALIDATOR_FN_##name = validator; \ static const dsn::flag_validator FLAGS_VALIDATOR_##name( \ - #name, []() { return FLAGS_VALIDATOR_FN_##name(FLAGS_##name); }) + #name, []() -> bool { return FLAGS_VALIDATOR_FN_##name(FLAGS_##name); }) #define DSN_TAG_VARIABLE(name, tag) \ COMPILE_ASSERT(sizeof(decltype(FLAGS_##name)), exist_##name##_##tag); \ From fc609b29ec0ec9634f463efb94a75608558bc95c Mon Sep 17 00:00:00 2001 From: levy Date: Tue, 29 Dec 2020 10:22:08 +0800 Subject: [PATCH 3/4] fix --- include/dsn/utility/flags.h | 2 +- src/utils/flags.cpp | 2 +- src/utils/test/flag_test.cpp | 13 +++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/include/dsn/utility/flags.h b/include/dsn/utility/flags.h index a3b909cd10..c4b5c63458 100644 --- a/include/dsn/utility/flags.h +++ b/include/dsn/utility/flags.h @@ -66,7 +66,7 @@ struct hash #define DSN_DEFINE_validator(name, validator) \ static auto FLAGS_VALIDATOR_FN_##name = validator; \ static const dsn::flag_validator FLAGS_VALIDATOR_##name( \ - #name, []() { return FLAGS_VALIDATOR_FN_##name(FLAGS_##name); }) + #name, []() -> bool { return FLAGS_VALIDATOR_FN_##name(FLAGS_##name); }) #define DSN_TAG_VARIABLE(name, tag) \ COMPILE_ASSERT(sizeof(decltype(FLAGS_##name)), exist_##name##_##tag); \ diff --git a/src/utils/flags.cpp b/src/utils/flags.cpp index 114460d6d7..f2105eba35 100644 --- a/src/utils/flags.cpp +++ b/src/utils/flags.cpp @@ -45,7 +45,7 @@ class flag_data return error_s::make(ERR_INVALID_PARAMETERS, fmt::format("{} is invalid", val)); \ } \ value() = tmpval_##type_enum; \ - if (!_validator()) { \ + if (_validator && !_validator()) { \ value() = old_val; \ return error_s::make(ERR_INVALID_PARAMETERS, "value validation failed"); \ } \ diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index f67c3adaea..b95d8e703b 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -41,6 +41,14 @@ DSN_TAG_VARIABLE(test_bool, FT_MUTABLE); DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", ""); +DSN_DEFINE_int32("flag_test", test_validator, 10, ""); +DSN_DEFINE_validator(test_validator, [](int32_t test_validator) -> bool { + if (test_validator < 0) { + return false; + } + return true; +}); + TEST(flag_test, update_config) { auto res = update_flag("test_int32", "3"); @@ -80,6 +88,11 @@ TEST(flag_test, update_config) res = update_flag("test_int32", "3ab"); ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); ASSERT_EQ(FLAGS_test_int32, 3); + + // validation failed + res = update_flag("test_validator", "-1"); + ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); + ASSERT_EQ(FLAGS_test_validator, 10); } DSN_DEFINE_int32("flag_test", has_tag, 5, ""); From 153b696283d65feef282b86069ec742275efad50 Mon Sep 17 00:00:00 2001 From: levy Date: Tue, 29 Dec 2020 10:50:41 +0800 Subject: [PATCH 4/4] fix --- src/utils/test/flag_test.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/utils/test/flag_test.cpp b/src/utils/test/flag_test.cpp index b95d8e703b..c7bb907cfa 100644 --- a/src/utils/test/flag_test.cpp +++ b/src/utils/test/flag_test.cpp @@ -42,6 +42,7 @@ DSN_TAG_VARIABLE(test_bool, FT_MUTABLE); DSN_DEFINE_string("flag_test", test_string_immutable, "immutable_string", ""); DSN_DEFINE_int32("flag_test", test_validator, 10, ""); +DSN_TAG_VARIABLE(test_validator, FT_MUTABLE); DSN_DEFINE_validator(test_validator, [](int32_t test_validator) -> bool { if (test_validator < 0) { return false; @@ -89,10 +90,15 @@ TEST(flag_test, update_config) ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); ASSERT_EQ(FLAGS_test_int32, 3); + // validation succeed + res = update_flag("test_validator", "5"); + ASSERT_TRUE(res.is_ok()); + ASSERT_EQ(FLAGS_test_validator, 5); + // validation failed res = update_flag("test_validator", "-1"); ASSERT_EQ(res.code(), ERR_INVALID_PARAMETERS); - ASSERT_EQ(FLAGS_test_validator, 10); + ASSERT_EQ(FLAGS_test_validator, 5); } DSN_DEFINE_int32("flag_test", has_tag, 5, "");