bneradt commented on PR #13426:
URL: https://github.com/apache/trafficserver/pull/13426#issuecomment-5174181089
I agree that removing these redundant overrides would be a useful separate
cleanup. My concern does not require that cleanup in this PR, though.
`initialize_hooks()` is virtual, so when `Statement::initialize()` invokes
it for `OperatorSetPluginCntl` or one of the state operators, C++ calls the
derived override instead of `Statement::initialize_hooks()`. The new POST_REMAP
entry in the base implementation therefore has no effect on those operators.
For example, this currently fails configuration parsing:
```text
cond %{POST_REMAP_HOOK}
set-state-flag 0 true
```
`RuleSet::add_operator()` rejects it because
`OperatorSetStateFlag::_allowed_hooks` does not contain
`TS_HTTP_POST_REMAP_HOOK`. The session operators are affected too because the
factory implements them using the same state classes.
The existing AuTest does not expose this because `set-header` inherits the
base hook list.
If removing the overrides is out of scope, that is fine. The minimal fix
here is to add `TS_HTTP_POST_REMAP_HOOK` to the four overrides marked "allowed
everywhere": `OperatorSetPluginCntl`, `OperatorSetStateFlag`,
`OperatorSetStateInt8`, and `OperatorSetStateInt16`. Adding `set-state-flag 0
true` to the existing POST_REMAP test ruleset would cover the parsing path.
--
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]
