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 dbbf96ddd2dc271d36e377d10f4c1a4c47d41213..294adc6dd0f23533ad4604f2f72f4f2cea219c74 100644 (file)
--- 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 4117ea9b689e1cd7128d6239c409178e1c09c52f..be3ae037d8f477ca0c21559659429b7736e453f4 100644 (file)
--- 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 <string> 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 <regex> 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 4470c8f4a3921cff6b49c4276761e66968c6208d..ea534f5122053aa9094776a99c76d3d7ff755454 100644 (file)
--- 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;
 }