From: Willy Tarreau <w@1wt.eu>
Date: Wed, 4 Oct 2017 13:58:52 +0000 (+0200)
Subject: BUG/MINOR: tcp-check: don't initialize then break a connection starting with a comment
X-Git-Tag: v1.8-dev3~58
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=894c642fbf33e635fdd9422ca0e640dc61202342;p=thirdparty%2Fhaproxy.git

BUG/MINOR: tcp-check: don't initialize then break a connection starting with a comment

The following config :

    backend tcp9000
        option tcp-check
        tcp-check comment "this is a comment"
        tcp-check connect port 10000
        server srv 127.0.0.1:9000 check inter 1s

will result in a connection being first made to port 9000 then immediately
destroyed and re-created on port 10000, because the first rule is a comment
and doesn't match the test for the first rule being a connect(). It's
mostly harmless (unless the server really must not receive empty
connections) and the workaround simply consists in removing the comment.

Let's proceed like in other places where we simply skip leading comments.
A new function was made to make this lookup les boring. The fix should be
backported to 1.7 and 1.6.
---

diff --git a/src/checks.c b/src/checks.c
index 04780de257..01059bb925 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -1447,6 +1447,20 @@ static struct task *server_warmup(struct task *t)
 	return t;
 }
 
+/* returns the first NON-COMMENT tcp-check rule from list <list> or NULL if
+ * none was found.
+ */
+static struct tcpcheck_rule *get_first_tcpcheck_rule(struct list *list)
+{
+	struct tcpcheck_rule *r;
+
+	list_for_each_entry(r, list, list) {
+		if (r->action != TCPCHK_ACT_COMMENT)
+			return r;
+	}
+	return NULL;
+}
+
 /*
  * establish a server health-check that makes use of a connection.
  *
@@ -1544,18 +1558,18 @@ static int connect_conn_chk(struct task *t)
 	/* only plain tcp-check supports quick ACK */
 	quickack = check->type == 0 || check->type == PR_O2_TCPCHK_CHK;
 
-	if (check->type == PR_O2_TCPCHK_CHK && !LIST_ISEMPTY(check->tcpcheck_rules)) {
-		struct tcpcheck_rule *r;
-
-		r = LIST_NEXT(check->tcpcheck_rules, struct tcpcheck_rule *, list);
+	if (check->type == PR_O2_TCPCHK_CHK) {
+		struct tcpcheck_rule *r = get_first_tcpcheck_rule(check->tcpcheck_rules);
 
-		/* if first step is a 'connect', then tcpcheck_main must run it */
-		if (r->action == TCPCHK_ACT_CONNECT) {
-			tcpcheck_main(conn);
-			return SF_ERR_UP;
+		if (r) {
+			/* if first step is a 'connect', then tcpcheck_main must run it */
+			if (r->action == TCPCHK_ACT_CONNECT) {
+				tcpcheck_main(conn);
+				return SF_ERR_UP;
+			}
+			if (r->action == TCPCHK_ACT_EXPECT)
+				quickack = 0;
 		}
-		if (r->action == TCPCHK_ACT_EXPECT)
-			quickack = 0;
 	}
 
 	ret = SF_ERR_INTERNAL;