From 8691151dbd0194d776e66dcfdc6f4d6e9c1e014c Mon Sep 17 00:00:00 2001
From: Maria Matejka <mq@ucw.cz>
Date: Mon, 11 Jul 2022 13:53:09 +0200
Subject: [PATCH] CLI fix for long-lived sessions during high loads

When there is a continuos stream of CLI commands, cli_get_command()
always returns 1 (there is a new command). Anyway, the socket receive
buffer was reset only when there was no command at all, leading to a
strange behavior: after a while, the CLI receive buffer came to its end,
then read() was called with zero size buffer, it returned 0 which was
interpreted as EOF.

Fixing this by:
* resetting the buffer any time CLI RX gets to EOL
* explicitly refusing to pipeline

In future, we may implement CLI pipelining, yet to make it conveniently,
a push-parser may come handy instead of the current implementation.
---
 doc/reply_codes    |  1 +
 nest/cli.c         | 21 ++++++++++++++-------
 nest/cli.h         | 11 +++++++++--
 sysdep/unix/main.c | 18 ++++++++++--------
 test/birdtest.c    |  2 +-
 5 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/doc/reply_codes b/doc/reply_codes
index 02f4e656..6d268494 100644
--- a/doc/reply_codes
+++ b/doc/reply_codes
@@ -75,3 +75,4 @@ Reply codes of BIRD command-line interface
 9000	Command too long
 9001	Parse error
 9002	Invalid symbol type
+9003	Pipelining not supported
diff --git a/nest/cli.c b/nest/cli.c
index b54a0d76..7b517cd2 100644
--- a/nest/cli.c
+++ b/nest/cli.c
@@ -293,13 +293,20 @@ cli_event(void *data)
     c->cont(c);
   else
     {
-      err = cli_get_command(c);
-      if (!err)
-	return;
-      if (err < 0)
-	cli_printf(c, 9000, "Command too long");
-      else
-	cli_command(c);
+      switch (cli_get_command(c))
+      {
+	case CGC_OK:
+	  cli_command(c);
+	  break;
+	case CGC_INCOMPLETE:
+	  return;
+	case CGC_TOO_LONG:
+	  cli_printf(c, 9000, "Command too long");
+	  break;
+	case CGC_TRAILING:
+	  cli_printf(c, 9003, "Pipelining not supported");
+	  break;
+      }
     }
 
   cli_write_trigger(c);
diff --git a/nest/cli.h b/nest/cli.h
index 8a3294c5..90442544 100644
--- a/nest/cli.h
+++ b/nest/cli.h
@@ -29,7 +29,7 @@ typedef struct cli {
   node n;				/* Node in list of all log hooks */
   pool *pool;
   void *priv;				/* Private to sysdep layer */
-  byte *rx_buf, *rx_pos, *rx_aux;	/* sysdep */
+  byte *rx_buf, *rx_pos;		/* sysdep */
   struct cli_out *tx_buf, *tx_pos, *tx_write;
   event *event;
   void (*cont)(struct cli *c);
@@ -81,6 +81,13 @@ static inline int cli_access_restricted(void)
 /* Functions provided by sysdep layer */
 
 void cli_write_trigger(cli *);
-int cli_get_command(cli *);
+enum cli_get_command_result {
+  CGC_OK,
+  CGC_INCOMPLETE,
+  CGC_TOO_LONG,
+  CGC_TRAILING,
+};
+
+enum cli_get_command_result cli_get_command(cli *);
 
 #endif
diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c
index 392aff9d..02ee0ec6 100644
--- a/sysdep/unix/main.c
+++ b/sysdep/unix/main.c
@@ -430,11 +430,11 @@ cli_tx(sock *s)
   cli_write(s->data);
 }
 
-int
+enum cli_get_command_result
 cli_get_command(cli *c)
 {
   sock *s = c->priv;
-  byte *t = c->rx_aux ? : s->rbuf;
+  byte *t = s->rbuf;
   byte *tend = s->rpos;
   byte *d = c->rx_pos;
   byte *dend = c->rx_buf + CLI_RX_BUF_SIZE - 2;
@@ -445,18 +445,21 @@ cli_get_command(cli *c)
 	t++;
       else if (*t == '\n')
 	{
-	  t++;
+	  if (++t < tend)
+	    return CGC_TRAILING;
+
+	  s->rpos = s->rbuf;
 	  c->rx_pos = c->rx_buf;
-	  c->rx_aux = t;
 	  *d = 0;
-	  return (d < dend) ? 1 : -1;
+	  return (d < dend) ? CGC_OK : CGC_TOO_LONG;
 	}
       else if (d < dend)
 	*d++ = *t++;
     }
-  c->rx_aux = s->rpos = s->rbuf;
+
+  s->rpos = s->rbuf;
   c->rx_pos = d;
-  return 0;
+  return CGC_INCOMPLETE;
 }
 
 static int
@@ -493,7 +496,6 @@ cli_connect(sock *s, uint size UNUSED)
   s->pool = c->pool;		/* We need to have all the socket buffers allocated in the cli pool */
   s->fast_rx = 1;
   c->rx_pos = c->rx_buf;
-  c->rx_aux = NULL;
   rmove(s, c->pool);
   return 1;
 }
diff --git a/test/birdtest.c b/test/birdtest.c
index 6ad743ce..2207ca6e 100644
--- a/test/birdtest.c
+++ b/test/birdtest.c
@@ -549,6 +549,6 @@ int sysdep_commit(struct config *new UNUSED, struct config *old UNUSED) { return
 void sysdep_shutdown_done(void) {}
 
 #include "nest/cli.h"
-int cli_get_command(cli *c UNUSED) { return 0; }
+enum cli_get_command_result cli_get_command(cli *c UNUSED) { return CGC_OK; }
 void cli_write_trigger(cli *c UNUSED) {}
 cli *cmd_reconfig_stored_cli;