ishaaaan17 opened a new pull request, #13934:
URL: https://github.com/apache/trafficserver/pull/13934
### summary
fixes #12660 (milestone 11.0.0).
in `header_rewrite` and `regex_remap`, syntax errors or invalid
configuration rules were ignored or skipped with `continue;`. because of this,
ats could start or reload with broken or missing rules, and did not fail with
`TS_ERROR`.
this pr makes the loaders fail when there are configuration errors, unknown
modifiers, invalid options or regexes that cannot be parsed. they return
`TS_ERROR` from `TSRemapNewInstance` or abort `TSPluginInit`.
### changes
#### `header_rewrite`:
1. **rule completion check**: in `validate_rule_completion()`, rules that
start with `CondClause::COND` must now contain at least one operator. before,
only `CondClause::OPER` was checked, so condition-only rules (such as `cond
%{PSEUDO__REMAP_HOOK}`) passed without any error.
2. **modifier check**: `Parser::validate_mods()` is now enforced in
`Condition::initialize()` and `Operator::initialize()`. it throws
`std::runtime_error("unknown modifier(s)")` on unknown modifiers, and the
return value is not ignored anymore.
3. **duplicate modifiers**: in `Parser::preprocess()`, return `false` on
duplicate modifiers (this takes care of the v11 todo comment).
4. **global config loading**: in `TSPluginInit()`, the registration is
aborted if any configuration file fails to parse.
5. **unit test**: added a duplicate modifier test case to
`test_header_rewrite`.
#### `regex_remap`:
1. **remap instance creation**: in `TSRemapNewInstance()`, return `TS_ERROR`
and clean up `remap_info` when invalid options are given in `argv` or when
loading the configuration fails.
2. **rule option check**: in `RemapRegex::initialize()`, return `false` for
malformed option values (for example `@status=`) or unknown options.
3. **rule compilation**: in `compile_rule_set()`, return `nullptr` when
regex compilation, syntax validation or rule initialization fails, instead of
skipping the invalid rule with `continue`.
### testing
- built both `header_rewrite.so` and `regex_remap.so` with no compiler
warnings and no errors.
- ran the `test_header_rewrite` unit test suite, and all the parser,
tokenizer and cidr checks pass.
- checked the formatting with the repository's `.clang-format`.
### ai disclosure
ai was used to help trace the configuration loading paths and find where
parsing errors were silently skipped in `header_rewrite` and `regex_remap`.
i reviewed all the changes by hand:
- checked the memory cleanup and the `TS_ERROR` returns in
`TSRemapNewInstance`, so failed instances do not leak resources.
- checked the operator requirement in `validate_rule_completion()`, so rules
without operators cannot load.
- checked the modifier validation and how failures are passed up in the
`Condition` and `Operator` initialization.
- built both plugin modules and checked that the unit tests
(`test_header_rewrite`) pass.
- checked the formatting with the project's `.clang-format`.
--
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]
