RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2025-02-20 20:57:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MEDIUM: cli: Warn if pipelined commands are delimited by a \n

Browse Source 
This was broken since commit 0011c25144 ("BUG/MINOR: cli: avoid O(bufsize)
parsing cost on pipelined commands"). It is not really a bug fix but it is
labelled as is to make it more visible.

Before, a full line was first retrieved from the request buffer before
extracting the first command to eval it. Now, only one command is retrieved.
But we rely on the request buffer state to interrupt processing in
non-interactive mode. After a command processing, if output of the request
buffer is empty, we leave. Before the above commit, this was not a problem.
But since then, it is obviously a bad statement. First because some input
data may still be there. It is not true today, but it might change. Then,
there is no warranty to receive all commands in same time. For small list of
commands, it will be most of time the case, but it is a dangerous
assumption. For long list of commands, it is almost always false.

To be an issue, commands must be chunked exactly between two commands. But
in this case, remaining commands are skipped. A good way to reproduce the
issue is to wait a bit between two commands, for instance:

    (printf "show info;"; sleep 2; printf "show stat\n") | socat ...

In fact, to properly fix the issue, we should exit on the first command
finished by a newline. Indeed, as stated in the documentation, in
non-interactive mode, a single line is processed. To pipeline commands,
commands must be separated by a semi-colon. Unfortunately, the above commit
introduced another change. It is possible to pipeline commands delimited by
a newline. It was pushed 2 years ago and backported to all stable versions.
Several scripts may rely on this behavior.

So, on stable version, the bug will not be fixed. However a warning will be
emitted to notify users their scripts don't respect the documentation and
they must adapt it. Mainly because the cli behavior on this point will be
changed in 3.0 to stick to the doc. This warning will only be emitted once
over the whole worker process life. Idea is to not flood the logs with the
same warning for every offending commands.

This commit should probably be backported to all stable versions. But with
some cautions because the CLI was often modified.
This commit is contained in:
Christopher Faulet 2024-02-20 18:30:16 +01:00
parent e018e8a419
commit 598c7f164c
2 changed files with 24 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/haproxy/cli-t.h
View File

@ -45,6 +45,7 @@
#define APPCTX_CLI_ST1_PAYLOAD (1 << 1)
#define APPCTX_CLI_ST1_NOLF (1 << 2)
#define APPCTX_CLI_ST1_TIMED (1 << 3)
#define APPCTX_CLI_ST1_SHUT_EXPECTED (1 << 4)
#define CLI_PREFIX_KW_NB 5
#define CLI_MAX_MATCHES 5

23
src/cli.c
View File

@ -813,6 +813,22 @@ static int cli_parse_request(struct appctx *appctx)
if (!**args)
return 0;
if (appctx->st1 & APPCTX_CLI_ST1_SHUT_EXPECTED) {
/* The previous command line was finished by a \n in non-interactive mode.
* It should not be followed by another command line. In non-interactive mode,
* only one line should be processed. Because of a bug, it is not respected.
* So emit a warning, only once in the process life, to warn users their script
* must be updated.
*/
appctx->st1 &= ~APPCTX_CLI_ST1_SHUT_EXPECTED;
if (ONLY_ONCE()) {
ha_warning("Commands sent to the CLI were chained using a new line character while in non-interactive mode."
" This is not reliable, not officially supported and will not be supported anymore in future versions. "
"Please use ';' to delimit commands instead.");
}
}
kw = cli_find_kw(args);
if (!kw ||
(kw->level & ~appctx->cli_level & ACCESS_MASTER_ONLY) ||
@ -916,6 +932,7 @@ static void cli_io_handler(struct appctx *appctx)
struct bind_conf *bind_conf = strm_li(__sc_strm(sc))->bind_conf;
int reql;
int len;
int lf = 0;
if (unlikely(se_fl_test(appctx->sedesc, (SE_FL_EOS|SE_FL_ERROR)))) {
co_skip(sc_oc(sc), co_data(sc_oc(sc)));
@ -987,6 +1004,8 @@ static void cli_io_handler(struct appctx *appctx)
continue;
}
if (str[reql-1] == '\n')
lf = 1;
/* now it is time to check that we have a full line,
* remove the trailing \n and possibly \r, then cut the
@ -1029,6 +1048,8 @@ static void cli_io_handler(struct appctx *appctx)
*/
appctx->st1 &= ~APPCTX_CLI_ST1_PAYLOAD;
if (!(appctx->st1 & APPCTX_CLI_ST1_PROMPT) && lf)
appctx->st1 |= APPCTX_CLI_ST1_SHUT_EXPECTED;
}
}
}
@ -1067,6 +1088,8 @@ static void cli_io_handler(struct appctx *appctx)
/* no payload, the command is complete: parse the request */
cli_parse_request(appctx);
chunk_reset(appctx->chunk);
if (!(appctx->st1 & APPCTX_CLI_ST1_PROMPT) && lf)
appctx->st1 |= APPCTX_CLI_ST1_SHUT_EXPECTED;
}
}