This is an automated email from the ASF dual-hosted git repository.
zwoop 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 855eb10eab Produce warnings when bad mods are used (#12749)
855eb10eab is described below
commit 855eb10eabad7f7cd086e56d910574c8c6beca51
Author: Leif Hedstrom <[email protected]>
AuthorDate: Mon Jan 5 10:41:23 2026 -0700
Produce warnings when bad mods are used (#12749)
---
plugins/header_rewrite/condition.cc | 24 ++++++++++++------------
plugins/header_rewrite/operator.cc | 8 +++++---
plugins/header_rewrite/parser.cc | 27 ++++++++++++++++++++++++++-
plugins/header_rewrite/parser.h | 14 ++++++++++++--
4 files changed, 55 insertions(+), 18 deletions(-)
diff --git a/plugins/header_rewrite/condition.cc
b/plugins/header_rewrite/condition.cc
index da657c59d9..3d0de68477 100644
--- a/plugins/header_rewrite/condition.cc
+++ b/plugins/header_rewrite/condition.cc
@@ -80,17 +80,17 @@ Condition::initialize(Parser &p)
{
Statement::initialize(p);
- if (p.mod_exist("OR")) {
- if (p.mod_exist("AND")) {
+ if (p.consume_mod("OR")) {
+ if (p.consume_mod("AND")) {
TSError("[%s] Can't have both AND and OR in mods", PLUGIN_NAME);
} else {
_mods |= CondModifiers::OR;
}
- } else if (p.mod_exist("AND")) {
+ } else if (p.consume_mod("AND")) {
_mods |= CondModifiers::AND;
}
- if (p.mod_exist("NOT")) {
+ if (p.consume_mod("NOT")) {
_mods |= CondModifiers::NOT;
}
@@ -98,25 +98,25 @@ Condition::initialize(Parser &p)
// strings and regexes.
int _substr_seen = 0;
- if (p.mod_exist("NOCASE")) {
+ if (p.consume_mod("NOCASE")) {
_mods |= CondModifiers::MOD_NOCASE;
- } else if (p.mod_exist("CASE")) {
+ } else if (p.consume_mod("CASE")) {
// Nothing to do — default is case-sensitive, but still allow this string
for clearness.
}
- if (p.mod_exist("EXT")) {
+ if (p.consume_mod("EXT")) {
_mods |= CondModifiers::MOD_EXT;
_substr_seen++;
}
- if (p.mod_exist("SUF")) {
+ if (p.consume_mod("SUF")) {
_mods |= CondModifiers::MOD_SUF;
_substr_seen++;
}
- if (p.mod_exist("PRE")) {
+ if (p.consume_mod("PRE")) {
_mods |= CondModifiers::MOD_PRE;
_substr_seen++;
}
- if (p.mod_exist("MID")) {
+ if (p.consume_mod("MID")) {
_mods |= CondModifiers::MOD_MID;
_substr_seen++;
}
@@ -125,10 +125,10 @@ Condition::initialize(Parser &p)
throw std::runtime_error("Only one substring modifier (EXT, SUF, PRE, MID)
may be used.");
}
- // Deal with the "last" modifier as well.
- if (p.mod_exist("L")) {
+ if (p.consume_mod("L")) {
_mods |= CondModifiers::MOD_L;
}
_cond_op = parse_matcher_op(p.get_arg());
+ p.validate_mods();
}
diff --git a/plugins/header_rewrite/operator.cc
b/plugins/header_rewrite/operator.cc
index 4cd337ca4b..6643bb8059 100644
--- a/plugins/header_rewrite/operator.cc
+++ b/plugins/header_rewrite/operator.cc
@@ -38,17 +38,19 @@ Operator::initialize(Parser &p)
{
Statement::initialize(p);
- if (p.mod_exist("L") || p.mod_exist("LAST")) {
+ if (p.consume_mod("L") || p.consume_mod("LAST")) {
_mods = static_cast<OperModifiers>(_mods | OPER_LAST);
}
- if (p.mod_exist("QSA")) {
+ if (p.consume_mod("QSA")) {
_mods = static_cast<OperModifiers>(_mods | OPER_QSA);
}
- if (p.mod_exist("I") || p.mod_exist("INV")) {
+ if (p.consume_mod("I") || p.consume_mod("INV")) {
_mods = static_cast<OperModifiers>(_mods | OPER_INV);
}
+
+ p.validate_mods();
}
void
diff --git a/plugins/header_rewrite/parser.cc b/plugins/header_rewrite/parser.cc
index 12f0c15d85..745f7d0882 100644
--- a/plugins/header_rewrite/parser.cc
+++ b/plugins/header_rewrite/parser.cc
@@ -181,7 +181,12 @@ Parser::preprocess(std::vector<std::string> tokens)
std::string t;
while (getline(iss, t, ',')) {
- _mods.push_back(t);
+ if (std::find(_mods.begin(), _mods.end(), t) != _mods.end()) {
+ // This produces an error, but it's not fatal for load / reload.
ToDo: ATS v11 fix.
+ TSError("[%s] Duplicate modifier: %s", PLUGIN_NAME, t.c_str());
+ } else {
+ _mods.push_back(t);
+ }
}
} else {
_mods.push_back(m);
@@ -303,6 +308,26 @@ Parser::cond_is_hook(TSHttpHookID &hook) const
return false;
}
+// This is a TSError() here, but where this is called from does not treat
"false" as a
+// load failure. This is a systemic problem in a some of the parsing. ToDo:
ATS v11 fix.
+bool
+Parser::validate_mods() const
+{
+ if (_mods.empty()) {
+ return true;
+ }
+
+ auto it = _mods.begin();
+ std::string list = *it++;
+
+ for (; it != _mods.end(); ++it) {
+ list += ", ";
+ list += *it;
+ }
+ TSError("[%s] Unknown modifier(s): [%s]", PLUGIN_NAME, list.c_str());
+ return false;
+}
+
HRWSimpleTokenizer::HRWSimpleTokenizer(const std::string &line)
{
ParserState state = PARSER_DEFAULT;
diff --git a/plugins/header_rewrite/parser.h b/plugins/header_rewrite/parser.h
index 634df4694b..854ba45231 100644
--- a/plugins/header_rewrite/parser.h
+++ b/plugins/header_rewrite/parser.h
@@ -195,12 +195,22 @@ public:
return _val;
}
+ // Check if the modifier exists and consume it from the list.
bool
- mod_exist(const std::string &m) const
+ consume_mod(const std::string &m)
{
- return std::find(_mods.begin(), _mods.end(), m) != _mods.end();
+ auto it = std::find(_mods.begin(), _mods.end(), m);
+
+ if (it != _mods.end()) {
+ _mods.erase(it);
+ return true;
+ }
+ return false;
}
+ // Validate that all modifiers were consumed; logs error and returns false
if not
+ bool validate_mods() const;
+
bool cond_is_hook(TSHttpHookID &hook) const;
const std::vector<std::string> &
