This is an automated email from the ASF dual-hosted git repository.
dmeden pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 608ec5b300 records.yaml - Make sure we validate the input value with
the configured check expr. (#11161)
608ec5b300 is described below
commit 608ec5b300678bf32f22647fddeeee4790a9338f
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.
---
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 5e461fbc18..607abe23e0 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}
,
