..Le 21/11/2020 à 06:15, Willy Tarreau a écrit :
I looked at the patch, it's OK regarding this and most of the other
stuff, except one thing, it mixes ACT_HTTP_* and PAT_MATCH_* and there
is some breakage there, because the HTTP action being set to PAT_MATCH_STR
for example sets it to value 5 which is ACT_HTTP_SET_NICE, etc. So you
must only have ACT_* values in your rule->action field.
Maciej, there are two possibilities here. Either you duplicate the delete
action to have DEL_STR, DEL_BEG, DEL_SUB etc, and you just set the right
action based on the matching method you found during parsing, or you
extend act_rule.arg.http to add a matching method for the delete action.
I tend to consider that the former is cleaner and simpler, given that
we're really performing a few different actions there, which are all
derivatives of the delete action.
Hi,
rule->action value may be set to any value if an action function is defined
(rule->action_ptr). ACT_* are only used for action without action function, to
perform a switch case on it. That's documented this way :
unsigned int action; /* ACT_* or any meaningful value if action_ptr is defined
*/
And with Maciej patches, ACT_HTTP_DEL_HDR can be removed.
Maciej, there is an issue in the parsing function (parse_http_del_header) :
- LIST_INIT(&rule->arg.http.fmt);
+ cur_arg++;
+ if (*args[cur_arg]) {
+ if (strcmp(args[cur_arg], "-m") == 0) {
[...]
+ } else {
+ memprintf(err, "'%s' flag was set but currently only
'-m' is supported", args[cur_arg]);
+ return ACT_RET_PRS_ERR;
+ }
+ }
Here you should only increment cur_arg value if "-m" argument is found. And if
not, you should not report an error. Otherwise you will have trouble if an ACL
is used. For instance :
http-request del_header x-hdr if { ... }
Otherwise, your patches are good. If you want, you can add the support for "-m
reg" method too and store the regex in "rule->arg.http.re". Many thanks to
handle this feature. And thanks Tim for the review.
--
Christopher Faulet
