Re: Invalid blank line in master socket output when run in non-admin level

[Daniel MacDougall]

That's great to hear, thanks very much for the fast turnaround!

On Mon, Jul 1, 2019 at 6:56 AM William Lallemand 
wrote:

>
> On Mon, Jul 01, 2019 at 11:16:21AM +0200, William Lallemand wrote:
> > Hi Daniel,
> >
> > On Sat, Jun 29, 2019 at 03:16:06PM +0200, William Lallemand wrote:
> > >
> > > I'll take a look at the code on monday to see if it's easily fixable.
> > >
> >
> > The attached patch should fix your problem.
> >
> > Regards,
> >
>
> I pushed the fix in master, 2.0 and 1.9.
>
>
> --
> William Lallemand
>

Re: Invalid blank line in master socket output when run in non-admin level

[William Lallemand]

On Mon, Jul 01, 2019 at 11:16:21AM +0200, William Lallemand wrote:
> Hi Daniel,
> 
> On Sat, Jun 29, 2019 at 03:16:06PM +0200, William Lallemand wrote:
> > 
> > I'll take a look at the code on monday to see if it's easily fixable.
> > 
> 
> The attached patch should fix your problem.
> 
> Regards,
> 
 
I pushed the fix in master, 2.0 and 1.9.


-- 
William Lallemand

Re: Invalid blank line in master socket output when run in non-admin level

[William Lallemand]

Hi Daniel,

On Sat, Jun 29, 2019 at 03:16:06PM +0200, William Lallemand wrote:
> 
> I'll take a look at the code on monday to see if it's easily fixable.
> 

The attached patch should fix your problem.

Regards,

-- 
William Lallemand
>From 3dffa809d35e6380a958d657e10b9c1885836070 Mon Sep 17 00:00:00 2001
From: William Lallemand 
Date: Mon, 1 Jul 2019 10:56:15 +0200
Subject: [PATCH] BUG/MINOR: mworker/cli: don't output a \n before the response

When using a level lower than admin on the master CLI, a \n is output
before the response, this is caused by the response of the "operator" or
"user" that are sent before the actual command.

To fix this problem we introduce the flag APPCTX_CLI_ST1_NOLF which ask
a command response to not be followed by the final \n.
This patch made a special case with the command operator and user
followed by a - so they are not followed by \n.

This patch must be backported to 2.0.
---
 include/types/applet.h |  1 +
 src/cli.c  | 16 +++-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/types/applet.h b/include/types/applet.h
index c9e02d173..1f3a4983a 100644
--- a/include/types/applet.h
+++ b/include/types/applet.h
@@ -50,6 +50,7 @@ struct applet {
 
 #define APPCTX_CLI_ST1_PROMPT  (1 << 0)
 #define APPCTX_CLI_ST1_PAYLOAD (1 << 1)
+#define APPCTX_CLI_ST1_NOLF(1 << 2)
 
 /* Context of a running applet. */
 struct appctx {
diff --git a/src/cli.c b/src/cli.c
index 44ddc7bfb..9a9f80f90 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -821,7 +821,7 @@ static void cli_io_handler(struct appctx *appctx)
 		prompt = "\n> ";
 }
 else {
-	if (!(appctx->st1 & APPCTX_CLI_ST1_PAYLOAD))
+	if (!(appctx->st1 & (APPCTX_CLI_ST1_PAYLOAD|APPCTX_CLI_ST1_NOLF)))
 		prompt = "\n";
 }
 
@@ -848,6 +848,8 @@ static void cli_io_handler(struct appctx *appctx)
 
 			/* switch state back to GETREQ to read next requests */
 			appctx->st0 = CLI_ST_GETREQ;
+			/* reactivate the \n at the end of the response for the next command */
+			appctx->st1 &= ~APPCTX_CLI_ST1_NOLF;
 		}
 	}
 
@@ -1442,6 +1444,10 @@ static int cli_parse_show_lvl(char **args, char *payload, struct appctx *appctx,
 /* parse and set the CLI level dynamically */
 static int cli_parse_set_lvl(char **args, char *payload, struct appctx *appctx, void *private)
 {
+	/* this will ask the applet to not output a \n after the command */
+	if (!strcmp(args[1], "-"))
+	appctx->st1 |= APPCTX_CLI_ST1_NOLF;
+
 	if (!strcmp(args[0], "operator")) {
 		if (!cli_has_level(appctx, ACCESS_LVL_OPER)) {
 			return 1;
@@ -2097,11 +2103,11 @@ int pcli_parse_request(struct stream *s, struct channel *req, char **errmsg, int
 		if (pcli_has_level(s, ACCESS_LVL_ADMIN)) {
 			goto end;
 		} else if (pcli_has_level(s, ACCESS_LVL_OPER)) {
-			ci_insert_line2(req, 0, "operator", strlen("operator"));
-			ret += strlen("operator") + 2;
+			ci_insert_line2(req, 0, "operator -", strlen("operator -"));
+			ret += strlen("operator -") + 2;
 		} else if (pcli_has_level(s, ACCESS_LVL_USER)) {
-			ci_insert_line2(req, 0, "user", strlen("user"));
-			ret += strlen("user") + 2;
+			ci_insert_line2(req, 0, "user -", strlen("user -"));
+			ret += strlen("user -") + 2;
 		}
 	}
 end:
-- 
2.21.0

Re: Invalid blank line in master socket output when run in non-admin level

[William Lallemand]

Hi Daniel,

On Fri, Jun 28, 2019 at 02:26:33PM -0700, Daniel MacDougall wrote:
> *# At operator or user level, the output is preceded by an extra blank
> line.*
> > sudo haproxy -W -S /run/haproxy/master.sock,mode,666,level,*operator* -f
> haproxy.cfg
> > socat /run/haproxy/master.sock stdio
> show cli level
> 
> operator
> 
> @1 show info
> 
> Name: HAProxy
> Version: 2.0.1-1ppa1~bionic
> Release_date: 2019/06/27
> Nbthread: 8
> Nbproc: 1
> ...

Hi Daniel,

That's indeed a bug, and it's a side effect of the implementation of the level
parameter of the master CLI.

To lower the rights of the master CLI, a command is sent before the command you
ask, and that's probably this which returns a \n.

For example, if you have a master CLI with level operator:

`@1 show info` will send `operator;show info` to the first process.

I'll take a look at the code on monday to see if it's easily fixable.

Regards,

-- 
William Lallemand

Invalid blank line in master socket output when run in non-admin level

[Daniel MacDougall]

Hi,

I ran across what seems like a bug in the master socket output. When run in
a non-admin level (i.e., operator or user), it outputs an extra newline
before printing the normal output. This seems to break the following
promise from the management guide
:

Since multiple commands may be issued at once, haproxy uses the empty line
as a
delimiter to mark an end of output for each command, and takes care of
ensuring
that no command can emit an empty line on output. A script can thus easily
parse the output even when multiple commands were pipelined on a single
line.


Here's a brief demonstration:

*# At admin level, the output is normal.*
> sudo haproxy -W -S /run/haproxy/master.sock,mode,666,level,*admin* -f
haproxy.cfg
> socat /run/haproxy/master.sock stdio
show cli level
admin

@1 show info
Name: HAProxy
Version: 2.0.1-1ppa1~bionic
Release_date: 2019/06/27
Nbthread: 8
Nbproc: 1
...

*# At operator or user level, the output is preceded by an extra blank
line.*
> sudo haproxy -W -S /run/haproxy/master.sock,mode,666,level,*operator* -f
haproxy.cfg
> socat /run/haproxy/master.sock stdio
show cli level

operator

@1 show info

Name: HAProxy
Version: 2.0.1-1ppa1~bionic
Release_date: 2019/06/27
Nbthread: 8
Nbproc: 1
...

Note that this behavior is only present on the master socket, not on worker
sockets.

For completeness, this is the haproxy.cfg file I'm using:

global
log stdout  format raw  daemon  info

stats socket /run/haproxy/worker.sock mode 666 level operator

chroot /var/lib/haproxy
user haproxy
group haproxy

Is this indeed a bug, or am I missing something? If it is a bug, would it
be appropriate to file an issue on GitHub?

Thanks!
Daniel