From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 18 Jan 2022 07:44:23 +0000 (+0100)
Subject: BUG/MEDIUM: cli: Never wait for more data on client shutdown
X-Git-Tag: v2.6-dev1~108
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0f727dabf51d4ffead40fd43feb7c07193ebde99;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: cli: Never wait for more data on client shutdown

When a shutdown is detected on the cli, we try to execute all pending
commands first before closing the connection. It is required because
commands execution is serialized. However, when the last part is a partial
command, the cli connection is not closed, waiting for more data. Because
there is no timeout for now on the cli socket, the connection remains
infinitely in this state. And because the maxconn is set to 10, if it
happens several times, the cli socket quickly becomes unresponsive because
all its slots are waiting for more data on a closed connections.

This patch should fix the issue #1512. It must be backported as far as 2.0.
---

diff --git a/src/cli.c b/src/cli.c
index 8dadd5ac06..ec164aebd4 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -2441,20 +2441,11 @@ read_again:
 	/* We don't know yet to which server we will connect */
 	channel_dont_connect(req);
 
-
-	/* we are not waiting for a response, there is no more request and we
-	 * receive a close from the client, we can leave */
-	if (!(ci_data(req)) && req->flags & CF_SHUTR) {
-		channel_shutw_now(&s->res);
-		s->req.analysers &= ~AN_REQ_WAIT_CLI;
-		return 1;
-	}
-
 	req->flags |= CF_READ_DONTWAIT;
 
 	/* need more data */
 	if (!ci_data(req))
-		return 0;
+		goto missing_data;
 
 	/* If there is data available for analysis, log the end of the idle time. */
 	if (c_data(req) && s->logs.t_idle == -1)
@@ -2497,14 +2488,14 @@ read_again:
 		/* we trimmed things but we might have other commands to consume */
 		pcli_write_prompt(s);
 		goto read_again;
-	} else if (to_forward == -1 && errmsg) {
-		/* there was an error during the parsing */
-			pcli_reply_and_close(s, errmsg);
-			s->req.analysers &= ~AN_REQ_WAIT_CLI;
-			return 0;
-	} else if (to_forward == -1 && channel_full(req, global.tune.maxrewrite)) {
-		/* buffer is full and we didn't catch the end of a command */
-		goto send_help;
+	} else if (to_forward == -1) {
+                if (errmsg) {
+                        /* there was an error during the parsing */
+                        pcli_reply_and_close(s, errmsg);
+                        s->req.analysers &= ~AN_REQ_WAIT_CLI;
+                        return 0;
+                }
+                goto missing_data;
 	}
 
 	return 0;
@@ -2514,6 +2505,20 @@ send_help:
 	b_putblk(&req->buf, "help\n", 5);
 	goto read_again;
 
+missing_data:
+        if (req->flags & CF_SHUTR) {
+                /* There is no more request or a only a partial one and we
+                 * receive a close from the client, we can leave */
+                channel_shutw_now(&s->res);
+                s->req.analysers &= ~AN_REQ_WAIT_CLI;
+                return 1;
+        }
+        else if (channel_full(req, global.tune.maxrewrite)) {
+                /* buffer is full and we didn't catch the end of a command */
+                goto send_help;
+        }
+        return 0;
+
 server_disconnect:
 	pcli_reply_and_close(s, "Can't connect to the target CLI!\n");
 	return 0;