From: Gaetan Rivet Date: Wed, 26 Feb 2020 15:19:40 +0000 (+0100) Subject: MEDIUM: checks: rewind to the first inverse expect rule of a chain on new data X-Git-Tag: v2.2-dev7~176 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4038b94706f601c6a721377ae94d18ef3a723fd4;p=thirdparty%2Fhaproxy.git MEDIUM: checks: rewind to the first inverse expect rule of a chain on new data When receiving additional data while chaining multiple tcp-check expects, previous inverse expects might have a different result with the new data. They need to be evaluated again against the new data. Add a pointer to the first inverse expect rule of the current expect chain (possibly of length one) to each expect rule. When receiving new data, the currently evaluated tcp-check rule is set back to this pointed rule. Fonctionnaly speaking, it is a bug and it exists since the introduction of the feature. But there is no way for now to hit it because when an expect rule does not match, we wait for more data, independently on the inverse flag. The only way to move to the following rule is to be sure no more data will be received. This patch depends on the commit "MINOR: mini-clist: Add functions to iterate backward on a list". [Cf: I slightly updated the patch. First, it only concerns inverse expect rule. Normal expect rules are not concerned. Then, I removed the BUG tag because, for now, it is not possible to move to the following rule when the current one does not match while more data can be received.] --- diff --git a/include/types/checks.h b/include/types/checks.h index dbbf96ddd2..294adc6dd0 100644 --- a/include/types/checks.h +++ b/include/types/checks.h @@ -236,6 +236,7 @@ struct tcpcheck_rule { int inverse; /* 0 = regular match, 1 = inverse match */ unsigned short port; /* port to connect to */ unsigned short conn_opts; /* options when setting up a new connection */ + struct tcpcheck_rule *expect_head; /* first expect of a chain. */ }; #endif /* _TYPES_CHECKS_H */ diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c index 4117ea9b68..be3ae037d8 100644 --- a/src/cfgparse-listen.c +++ b/src/cfgparse-listen.c @@ -3191,6 +3191,7 @@ stats_error_parsing: } } else if (strcmp(args[1], "expect") == 0) { + struct tcpcheck_rule *tcpcheck, *prev_check; const char *ptr_arg; int cur_arg; int inverse = 0; @@ -3216,7 +3217,6 @@ stats_error_parsing: * exclamation mark, and cur_arg is the argument which holds this word. */ if (strcmp(ptr_arg, "binary") == 0) { - struct tcpcheck_rule *tcpcheck; char *err = NULL; if (!*(args[cur_arg + 1])) { @@ -3249,12 +3249,8 @@ stats_error_parsing: } tcpcheck->comment = strdup(args[cur_arg + 1]); } - - LIST_ADDQ(&curproxy->tcpcheck_rules, &tcpcheck->list); } else if (strcmp(ptr_arg, "string") == 0) { - struct tcpcheck_rule *tcpcheck; - if (!*(args[cur_arg + 1])) { ha_alert("parsing [%s:%d] : '%s %s %s' expects as an argument.\n", file, linenum, args[0], args[1], ptr_arg); @@ -3281,12 +3277,8 @@ stats_error_parsing: } tcpcheck->comment = strdup(args[cur_arg + 1]); } - - LIST_ADDQ(&curproxy->tcpcheck_rules, &tcpcheck->list); } else if (strcmp(ptr_arg, "rstring") == 0) { - struct tcpcheck_rule *tcpcheck; - if (!*(args[cur_arg + 1])) { ha_alert("parsing [%s:%d] : '%s %s %s' expects as an argument.\n", file, linenum, args[0], args[1], ptr_arg); @@ -3320,8 +3312,6 @@ stats_error_parsing: } tcpcheck->comment = strdup(args[cur_arg + 1]); } - - LIST_ADDQ(&curproxy->tcpcheck_rules, &tcpcheck->list); } else { ha_alert("parsing [%s:%d] : '%s %s' only supports [!] 'binary', 'string', 'rstring', found '%s'.\n", @@ -3329,6 +3319,21 @@ stats_error_parsing: err_code |= ERR_ALERT | ERR_FATAL; goto out; } + + /* All tcp-check expect points back to the first inverse expect rule + * in a chain of one or more expect rule, potentially itself. + */ + tcpcheck->expect_head = tcpcheck; + list_for_each_entry_rev(prev_check, &curproxy->tcpcheck_rules, list) { + if (prev_check->action == TCPCHK_ACT_EXPECT) { + if (prev_check->inverse) + tcpcheck->expect_head = prev_check; + continue; + } + if (prev_check->action != TCPCHK_ACT_COMMENT) + break; + } + LIST_ADDQ(&curproxy->tcpcheck_rules, &tcpcheck->list); } else { ha_alert("parsing [%s:%d] : '%s' only supports 'comment', 'connect', 'send' or 'expect'.\n", file, linenum, args[0]); diff --git a/src/checks.c b/src/checks.c index 4470c8f4a3..ea534f5122 100644 --- a/src/checks.c +++ b/src/checks.c @@ -3121,6 +3121,9 @@ static int tcpcheck_main(struct check *check) } } + /* Having received new data, reset the expect chain to its head. */ + check->current_step = check->current_step->expect_head; + /* mark the step as started */ check->last_started_step = check->current_step; @@ -3158,7 +3161,6 @@ static int tcpcheck_main(struct check *check) tcpcheck_expect: if (!done && (check->current_step->string != NULL) && (b_data(&check->bi) < check->current_step->string_len) ) continue; /* try to read more */ - if (check->current_step->string != NULL) ret = my_memmem(contentptr, b_data(&check->bi), check->current_step->string, check->current_step->string_len) != NULL; else if (check->current_step->expect_regex != NULL) @@ -3462,7 +3464,7 @@ int init_email_alert(struct mailers *mls, struct proxy *p, char **err) static int add_tcpcheck_expect_str(struct list *list, const char *str) { - struct tcpcheck_rule *tcpcheck; + struct tcpcheck_rule *tcpcheck, *prev_check; if ((tcpcheck = pool_alloc(pool_head_tcpcheck_rule)) == NULL) return 0; @@ -3476,6 +3478,19 @@ static int add_tcpcheck_expect_str(struct list *list, const char *str) return 0; } + /* All tcp-check expect points back to the first inverse expect rule + * in a chain of one or more expect rule, potentially itself. + */ + tcpcheck->expect_head = tcpcheck; + list_for_each_entry_rev(prev_check, list, list) { + if (prev_check->action == TCPCHK_ACT_EXPECT) { + if (prev_check->inverse) + tcpcheck->expect_head = prev_check; + continue; + } + if (prev_check->action != TCPCHK_ACT_COMMENT) + break; + } LIST_ADDQ(list, &tcpcheck->list); return 1; }