ishaaaan17 commented on code in PR #13934:
URL: https://github.com/apache/trafficserver/pull/13934#discussion_r4421316385


##########
plugins/header_rewrite/parser.cc:
##########
@@ -188,8 +188,8 @@ Parser::preprocess(std::vector<std::string> tokens)
 
           while (getline(iss, 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());
+              return false;

Review Comment:
   [P1] propagate duplicate-modifier errors out of parse_config
   
   addressed in 6aae1610f. `Parser` now tracks `_has_duplicate_mods = true;` 
during preprocessing and rejects it via `Parser::validate_mods()`. when 
`Condition::initialize()` or `Operator::initialize()` validates modifiers, it 
detects the duplicate and throws `std::runtime_error("invalid or unknown 
modifier(s)")`. `RulesConfig::parse_config()` catches the exception and returns 
`false`, aborting configuration loading and preventing operators from loading 
unconditionally.
   
   added regression coverage in `test_header_rewrite_bad_set_body.py` to verify 
that startup fails when duplicate modifiers or operatorless conditions are 
present.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to