This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 8bfaf91994522aab0d6befbf8474b674f73e19dd Author: Damian Meden <[email protected]> AuthorDate: Mon Apr 1 14:28:56 2024 +0200 records.yaml - Make sure we validate the input value with the configured check expr. (#11161) * records.yaml - Make sure we validate the input value with the configured check expr. * Fix proxy.config.cache.ram_cache.size regular expression. (cherry picked from commit 608ec5b300678bf32f22647fddeeee4790a9338f) --- include/mgmt/rpc/handlers/common/RecordsUtils.h | 11 --- src/mgmt/rpc/handlers/common/RecordsUtils.cc | 117 ----------------------- src/mgmt/rpc/handlers/config/Configuration.cc | 3 +- src/records/P_RecUtils.h | 12 +++ src/records/RecUtils.cc | 118 +++++++++++++++++++++++- src/records/RecYAMLDecoder.cc | 17 +++- src/records/RecordsConfig.cc | 2 +- 7 files changed, 146 insertions(+), 134 deletions(-) diff --git a/include/mgmt/rpc/handlers/common/RecordsUtils.h b/include/mgmt/rpc/handlers/common/RecordsUtils.h index 22be1e9416..f5328301ac 100644 --- a/include/mgmt/rpc/handlers/common/RecordsUtils.h +++ b/include/mgmt/rpc/handlers/common/RecordsUtils.h @@ -89,15 +89,4 @@ std::tuple<YAML::Node, std::error_code> get_yaml_record(std::string const &name, /// std::tuple<YAML::Node, std::error_code> get_yaml_record_regex(std::string const ®ex, unsigned recType); -/// -/// @brief Runs a validity check base on the type and the pattern. -/// -/// @param value Value where the validity check should be applied. -/// @param checkType The type of the value. -/// @param pattern The pattern. -/// @return true if the validity was ok, false otherwise. -/// -bool recordValidityCheck(const char *value, RecCheckT checkType, - const char *pattern); // code originally from WebMgmtUtils - } // namespace rpc::handlers::records::utils diff --git a/src/mgmt/rpc/handlers/common/RecordsUtils.cc b/src/mgmt/rpc/handlers/common/RecordsUtils.cc index c285f0e50f..0aaced049f 100644 --- a/src/mgmt/rpc/handlers/common/RecordsUtils.cc +++ b/src/mgmt/rpc/handlers/common/RecordsUtils.cc @@ -18,14 +18,6 @@ limitations under the License. */ -#if __has_include("pcre/pcre.h") -#include <pcre/pcre.h> -#elif __has_include("pcre.h") -#include <pcre.h> -#else -#error "Unable to locate PCRE heeader" -#endif - #include "mgmt/rpc/handlers/common/RecordsUtils.h" #include <system_error> @@ -212,113 +204,4 @@ get_yaml_record(std::string const &name, ValidateRecType check) return {ctx.yaml, ctx.ec}; } - -// Basic functions to help setting a record value properly. All this functionality is originally from WebMgmtUtils. -// TODO: we can work out something different. -namespace -{ // anonymous namespace - bool - recordRegexCheck(const char *pattern, const char *value) - { - pcre *regex; - const char *error; - int erroffset; - - regex = pcre_compile(pattern, 0, &error, &erroffset, nullptr); - if (!regex) { - return false; - } else { - int r = pcre_exec(regex, nullptr, value, strlen(value), 0, 0, nullptr, 0); - - pcre_free(regex); - return (r != -1) ? true : false; - } - - return false; // no-op - } - - bool - recordRangeCheck(const char *pattern, const char *value) - { - char *p = const_cast<char *>(pattern); - Tokenizer dashTok("-"); - - if (recordRegexCheck("^[0-9]+$", value)) { - while (*p != '[') { - p++; - } // skip to '[' - if (dashTok.Initialize(++p, COPY_TOKS) == 2) { - int l_limit = atoi(dashTok[0]); - int u_limit = atoi(dashTok[1]); - int val = atoi(value); - if (val >= l_limit && val <= u_limit) { - return true; - } - } - } - return false; - } - - bool - recordIPCheck(const char *pattern, const char *value) - { - // regex_t regex; - // int result; - bool check; - const char *range_pattern = R"(\[[0-9]+\-[0-9]+\]\\\.\[[0-9]+\-[0-9]+\]\\\.\[[0-9]+\-[0-9]+\]\\\.\[[0-9]+\-[0-9]+\])"; - const char *ip_pattern = "[0-9]*[0-9]*[0-9].[0-9]*[0-9]*[0-9].[0-9]*[0-9]*[0-9].[0-9]*[0-9]*[0-9]"; - - Tokenizer dotTok1("."); - Tokenizer dotTok2("."); - - check = true; - if (recordRegexCheck(range_pattern, pattern) && recordRegexCheck(ip_pattern, value)) { - if (dotTok1.Initialize(const_cast<char *>(pattern), COPY_TOKS) == 4 && - dotTok2.Initialize(const_cast<char *>(value), COPY_TOKS) == 4) { - for (int i = 0; i < 4 && check; i++) { - if (!recordRangeCheck(dotTok1[i], dotTok2[i])) { - check = false; - } - } - if (check) { - return true; - } - } - } else if (strcmp(value, "") == 0) { - return true; - } - return false; - } -} // namespace - -bool -recordValidityCheck(const char *value, RecCheckT checkType, const char *pattern) -{ - switch (checkType) { - case RECC_STR: - if (recordRegexCheck(pattern, value)) { - return true; - } - break; - case RECC_INT: - if (recordRangeCheck(pattern, value)) { - return true; - } - break; - case RECC_IP: - if (recordIPCheck(pattern, value)) { - return true; - } - break; - case RECC_NULL: - // skip checking - return true; - default: - // unknown RecordCheckType... - ; - } - - return false; -} - } // namespace rpc::handlers::records::utils diff --git a/src/mgmt/rpc/handlers/config/Configuration.cc b/src/mgmt/rpc/handlers/config/Configuration.cc index 9a0a0b80e0..fb42ac52fc 100644 --- a/src/mgmt/rpc/handlers/config/Configuration.cc +++ b/src/mgmt/rpc/handlers/config/Configuration.cc @@ -23,6 +23,7 @@ #include "records/RecCore.h" #include "../../../../records/P_RecCore.h" +#include "../../../../records/P_RecUtils.h" #include "tscore/Diags.h" #include "mgmt/config/FileManager.h" @@ -147,7 +148,7 @@ set_config_records(std::string_view const &id, YAML::Node const ¶ms) auto const &[dataType, checkType, pattern, updateType] = recordCtx; // run the check only if we have something to check against it. - if (pattern != nullptr && utils::recordValidityCheck(info.value.c_str(), checkType, pattern) == false) { + if (pattern != nullptr && RecordValidityCheck(info.value.c_str(), checkType, pattern) == false) { resp.errata() .assign(std::error_code{errors::Codes::RECORD}) .note("{}", std::error_code{err::RecordError::VALIDITY_CHECK_ERROR}); diff --git a/src/records/P_RecUtils.h b/src/records/P_RecUtils.h index 9ee44baec3..d16e3f0e28 100644 --- a/src/records/P_RecUtils.h +++ b/src/records/P_RecUtils.h @@ -76,3 +76,15 @@ void RecDebugOff(); #define RecLog(level, fmt, ...) _RecLog(level, MakeSourceLocation(), fmt, ##__VA_ARGS__) #define RecDebug(level, fmt, ...) _RecDebug(level, MakeSourceLocation(), fmt, ##__VA_ARGS__) + +// -- Helper functions +/// +/// @brief Runs a validity check base on the type and the pattern. +/// +/// @param value Value where the validity check should be applied. +/// @param checkType The type of the value. +/// @param pattern The pattern. +/// @return true if the validity was ok, false otherwise. +/// +bool RecordValidityCheck(const char *value, RecCheckT checkType, + const char *pattern); // code originally from WebMgmtUtils diff --git a/src/records/RecUtils.cc b/src/records/RecUtils.cc index 4308c8db3d..8aaeecd4ba 100644 --- a/src/records/RecUtils.cc +++ b/src/records/RecUtils.cc @@ -21,13 +21,21 @@ limitations under the License. */ +#if __has_include("pcre/pcre.h") +#include <pcre/pcre.h> +#elif __has_include("pcre.h") +#include <pcre.h> +#else +#error "Unable to locate PCRE heeader" +#endif + #include "tscore/ink_platform.h" #include "tscore/ink_memory.h" #include "tscore/ParseRules.h" +#include "tscore/Tokenizer.h" #include "records/RecordsConfig.h" #include "P_RecUtils.h" #include "P_RecCore.h" - //------------------------------------------------------------------------- // RecRecord initializer / Free //------------------------------------------------------------------------- @@ -374,3 +382,111 @@ RecDataSetFromString(RecDataT data_type, RecData *data_dst, const char *data_str return RecDataSet(data_type, data_dst, &data_src); } +//------------------------------------------------------------------------- +// Basic functions to help setting a record value properly. All this functionality is originally from WebMgmtUtils. +// TODO: we can work out something different. +namespace +{ // anonymous namespace +bool +recordRegexCheck(const char *pattern, const char *value) +{ + pcre *regex; + const char *error; + int erroffset; + + regex = pcre_compile(pattern, 0, &error, &erroffset, nullptr); + if (!regex) { + return false; + } else { + int r = pcre_exec(regex, nullptr, value, strlen(value), 0, 0, nullptr, 0); + + pcre_free(regex); + return (r != -1) ? true : false; + } + + return false; // no-op +} + +bool +recordRangeCheck(const char *pattern, const char *value) +{ + char *p = const_cast<char *>(pattern); + Tokenizer dashTok("-"); + + if (recordRegexCheck("^[0-9]+$", value)) { + while (*p != '[') { + p++; + } // skip to '[' + if (dashTok.Initialize(++p, COPY_TOKS) == 2) { + int l_limit = atoi(dashTok[0]); + int u_limit = atoi(dashTok[1]); + int val = atoi(value); + if (val >= l_limit && val <= u_limit) { + return true; + } + } + } + return false; +} + +bool +recordIPCheck(const char *pattern, const char *value) +{ + // regex_t regex; + // int result; + bool check; + const char *range_pattern = R"(\[[0-9]+\-[0-9]+\]\\\.\[[0-9]+\-[0-9]+\]\\\.\[[0-9]+\-[0-9]+\]\\\.\[[0-9]+\-[0-9]+\])"; + const char *ip_pattern = "[0-9]*[0-9]*[0-9].[0-9]*[0-9]*[0-9].[0-9]*[0-9]*[0-9].[0-9]*[0-9]*[0-9]"; + + Tokenizer dotTok1("."); + Tokenizer dotTok2("."); + + check = true; + if (recordRegexCheck(range_pattern, pattern) && recordRegexCheck(ip_pattern, value)) { + if (dotTok1.Initialize(const_cast<char *>(pattern), COPY_TOKS) == 4 && + dotTok2.Initialize(const_cast<char *>(value), COPY_TOKS) == 4) { + for (int i = 0; i < 4 && check; i++) { + if (!recordRangeCheck(dotTok1[i], dotTok2[i])) { + check = false; + } + } + if (check) { + return true; + } + } + } else if (strcmp(value, "") == 0) { + return true; + } + return false; +} +} // namespace + +bool +RecordValidityCheck(const char *value, RecCheckT checkType, const char *pattern) +{ + switch (checkType) { + case RECC_STR: + if (recordRegexCheck(pattern, value)) { + return true; + } + break; + case RECC_INT: + if (recordRangeCheck(pattern, value)) { + return true; + } + break; + case RECC_IP: + if (recordIPCheck(pattern, value)) { + return true; + } + break; + case RECC_NULL: + // skip checking + return true; + default: + // unknown RecordCheckType... + ; + } + + return false; +} diff --git a/src/records/RecYAMLDecoder.cc b/src/records/RecYAMLDecoder.cc index 0b04581335..cfb06fd2b1 100644 --- a/src/records/RecYAMLDecoder.cc +++ b/src/records/RecYAMLDecoder.cc @@ -73,15 +73,20 @@ SetRecordFromYAMLNode(CfgNode const &field, swoc::Errata &errata) std::string record_name{field.get_record_name()}; RecT rec_type{RecT::RECT_CONFIG}; RecDataT data_type{RecDataT::RECD_NULL}; - + RecCheckT check_type{RecCheckT::RECC_NULL}; + std::string check_expr; // this function (GetRec..) should be generic and possibly getting the value either // from where it gets it currently or a schema file. if (const auto *found = GetRecordElementByName(record_name.c_str()); found) { if (REC_TYPE_IS_STAT(found->type)) { ink_release_assert(REC_TYPE_IS_STAT(found->type)); } - rec_type = found->type; - data_type = found->value_type; + rec_type = found->type; + data_type = found->value_type; + check_type = found->check; + if (found->regex) { + check_expr = found->regex; + } } else { // Not registered in ATS, could be a plugin or an invalid(not registered) records. // Externally registered records should have the type set in each field (!!int, !!float, etc), otherwise we will not be able to @@ -121,6 +126,12 @@ SetRecordFromYAMLNode(CfgNode const &field, swoc::Errata &errata) errata.note(ERRATA_DEBUG, "'{}' was override with '{}' using an env variable", record_name, value_str); } + if (!check_expr.empty() && RecordValidityCheck(value_str.c_str(), check_type, check_expr.c_str()) == false) { + errata.note(ERRATA_WARN, "{} - Validity Check failed. '{}' against '{}'. Default value will be used", record_name, check_expr, + value_str); + return; + } + RecData data; memset(&data, 0, sizeof(RecData)); RecDataSetFromString(data_type, &data, value_str.c_str()); diff --git a/src/records/RecordsConfig.cc b/src/records/RecordsConfig.cc index ca225212f2..ee7bb70e0d 100644 --- a/src/records/RecordsConfig.cc +++ b/src/records/RecordsConfig.cc @@ -805,7 +805,7 @@ static const RecordElement RecordsConfig[] = , // # default the ram cache size to AUTO_SIZE (-1) // # alternatively: 20971520 (20MB) - {RECT_CONFIG, "proxy.config.cache.ram_cache.size", RECD_INT, "-1", RECU_RESTART_TS, RR_NULL, RECC_STR, "^-?[0-9]+$", RECA_NULL} + {RECT_CONFIG, "proxy.config.cache.ram_cache.size", RECD_INT, "-1", RECU_RESTART_TS, RR_NULL, RECC_STR, "^-?[0-9]+[A-Za-z]{0,}$", RECA_NULL} , {RECT_CONFIG, "proxy.config.cache.ram_cache.algorithm", RECD_INT, "1", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} ,
