From b71803b2c9012726e4a47a4c0d5f61e6b568cb9c Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Mon, 23 Aug 2010 18:50:58 +0200
Subject: [PATCH] Enforced ManageSieve protocol syntax better with some of the
 commands; some commands still allowed spurious extra arguments.

---
 TODO                                 |  2 --
 src/managesieve/cmd-capability.c     |  4 ++++
 src/managesieve/cmd-deletescript.c   |  2 +-
 src/managesieve/cmd-getscript.c      |  2 +-
 src/managesieve/cmd-havespace.c      |  8 +------
 src/managesieve/cmd-listscripts.c    |  4 ++++
 src/managesieve/cmd-logout.c         |  4 ++++
 src/managesieve/cmd-noop.c           | 13 +++++-----
 src/managesieve/cmd-renamescript.c   |  2 +-
 src/managesieve/cmd-setactive.c      |  2 +-
 src/managesieve/managesieve-client.c | 36 +++++++++++++++++-----------
 src/managesieve/managesieve-client.h | 10 ++++++--
 12 files changed, 53 insertions(+), 36 deletions(-)

diff --git a/TODO b/TODO
index 95355fffe..6a1f322c1 100644
--- a/TODO
+++ b/TODO
@@ -10,8 +10,6 @@ Current activities:
 
 Next (in order of descending priority/precedence):
 
-* Enforce ManageSieve protocol syntax better with some of the commands. Some 
-  commands still allow spurious extra arguments.
 * Code cleanup:
 	- Review all FIXMEs 
 
diff --git a/src/managesieve/cmd-capability.c b/src/managesieve/cmd-capability.c
index 38761610c..33638f658 100644
--- a/src/managesieve/cmd-capability.c
+++ b/src/managesieve/cmd-capability.c
@@ -17,6 +17,10 @@ bool cmd_capability(struct client_command_context *cmd)
 	const char *sievecap, *notifycap;
 	unsigned int max_redirects;
 
+	/* no arguments */
+	if ( !client_read_no_args(cmd) )
+		return FALSE;
+
 	o_stream_cork(client->output);
 
 	T_BEGIN {
diff --git a/src/managesieve/cmd-deletescript.c b/src/managesieve/cmd-deletescript.c
index 60b3fec60..414f5b721 100644
--- a/src/managesieve/cmd-deletescript.c
+++ b/src/managesieve/cmd-deletescript.c
@@ -17,7 +17,7 @@ bool cmd_deletescript(struct client_command_context *cmd)
 	struct sieve_script *script;
 
 	/* <scrip name>*/
-	if (!client_read_string_args(cmd, 1, &scriptname))
+	if ( !client_read_string_args(cmd, 1, TRUE, &scriptname) )
 		return FALSE;
 
 	script = sieve_storage_script_init(storage, scriptname);
diff --git a/src/managesieve/cmd-getscript.c b/src/managesieve/cmd-getscript.c
index 570032c57..cb1653d2c 100644
--- a/src/managesieve/cmd-getscript.c
+++ b/src/managesieve/cmd-getscript.c
@@ -90,7 +90,7 @@ bool cmd_getscript(struct client_command_context *cmd)
 	enum sieve_error error;
 
 	/* <scriptname> */
-	if (!client_read_string_args(cmd, 1, &scriptname))
+	if ( !client_read_string_args(cmd, 1, TRUE, &scriptname) )
 		return FALSE;
 
 	ctx = p_new(cmd->pool, struct cmd_getscript_context, 1);
diff --git a/src/managesieve/cmd-havespace.c b/src/managesieve/cmd-havespace.c
index 0a51260b3..7fec4a9a2 100644
--- a/src/managesieve/cmd-havespace.c
+++ b/src/managesieve/cmd-havespace.c
@@ -20,17 +20,11 @@ bool cmd_havespace(struct client_command_context *cmd)
 	struct managesieve_arg *args;
 	const char *scriptname;
 	uoff_t size;
-	int ret;
 
 	/* <scriptname> <size> */
-	if (!(ret=client_read_args(cmd, 2, 0, &args)))
+	if ( !client_read_args(cmd, 2, 0, TRUE, &args) )
 	  return FALSE;
 
-	if ( ret > 2 ) {
-		client_send_no(client, "Too many arguments");
-		return TRUE;
-	}
-
 	if ( (scriptname = managesieve_arg_string(&args[0])) == NULL ) {
 		client_send_no(client, "Invalid string for scriptname.");
 		return TRUE;
diff --git a/src/managesieve/cmd-listscripts.c b/src/managesieve/cmd-listscripts.c
index 7737bfdb5..363f26bad 100644
--- a/src/managesieve/cmd-listscripts.c
+++ b/src/managesieve/cmd-listscripts.c
@@ -20,6 +20,10 @@ bool cmd_listscripts(struct client_command_context *cmd)
 	bool active;
 	string_t *str;
 
+	/* no arguments */
+	if ( !client_read_no_args(cmd) )
+		return FALSE;
+
 	if ( (ctx = sieve_storage_list_init(client->storage))
 		== NULL ) {
 		client_send_storage_error(client, client->storage);
diff --git a/src/managesieve/cmd-logout.c b/src/managesieve/cmd-logout.c
index 996ea7997..8949511ae 100644
--- a/src/managesieve/cmd-logout.c
+++ b/src/managesieve/cmd-logout.c
@@ -11,6 +11,10 @@ bool cmd_logout(struct client_command_context *cmd)
 {
 	struct client *client = cmd->client;
 
+	/* no arguments */
+	if ( !client_read_no_args(cmd) )
+		return FALSE;
+
 	client_send_line(client, "OK \"Logout completed.\"");
 	client_disconnect(client, "Logged out");
 	return TRUE;
diff --git a/src/managesieve/cmd-noop.c b/src/managesieve/cmd-noop.c
index 5cc34a388..26cf914ee 100644
--- a/src/managesieve/cmd-noop.c
+++ b/src/managesieve/cmd-noop.c
@@ -17,17 +17,11 @@ bool cmd_noop(struct client_command_context *cmd)
 	struct managesieve_arg *args;
 	const char *text;
 	string_t *resp_code;
-	int ret;
 
 	/* [<echo string>] */
-	if (!(ret=client_read_args(cmd, 0, 0, &args)))
+	if ( !client_read_args(cmd, 0, 0, FALSE, &args) )
 		return FALSE;
 
-	if ( ret > 1 ) {
-		client_send_no(client, "Too many arguments");
-		return TRUE;
-	}
-
 	if ( args[0].type == MANAGESIEVE_ARG_EOL ) {
 		client_send_ok(client, "NOOP Completed");
 		return TRUE;
@@ -38,6 +32,11 @@ bool cmd_noop(struct client_command_context *cmd)
 		return TRUE;
 	}
 
+	if ( args[1].type != MANAGESIEVE_ARG_EOL ) {
+		client_send_command_error(cmd, "Too many arguments.");
+		return TRUE;
+	}
+
 	resp_code = t_str_new(256);
 	str_append(resp_code, "TAG ");
 	managesieve_quote_append_string(resp_code, text, FALSE);
diff --git a/src/managesieve/cmd-renamescript.c b/src/managesieve/cmd-renamescript.c
index 5ebc378bc..57edac30b 100644
--- a/src/managesieve/cmd-renamescript.c
+++ b/src/managesieve/cmd-renamescript.c
@@ -18,7 +18,7 @@ bool cmd_renamescript(struct client_command_context *cmd)
 	struct sieve_script *script;
 
 	/* <oldname> <newname> */
-	if (!client_read_string_args(cmd, 2, &scriptname, &newname))
+	if (!client_read_string_args(cmd, 2, TRUE, &scriptname, &newname))
 		return FALSE;
 
 	script = sieve_storage_script_init(storage, scriptname);
diff --git a/src/managesieve/cmd-setactive.c b/src/managesieve/cmd-setactive.c
index 288d99d2a..ce34ad135 100644
--- a/src/managesieve/cmd-setactive.c
+++ b/src/managesieve/cmd-setactive.c
@@ -18,7 +18,7 @@ bool cmd_setactive(struct client_command_context *cmd)
 	int ret;
 
 	/* <scriptname> */
-	if (!client_read_string_args(cmd, 1, &scriptname))
+	if ( !client_read_string_args(cmd, 1, TRUE, &scriptname) )
 		return FALSE;
 
 	if ( *scriptname != '\0' ) {
diff --git a/src/managesieve/managesieve-client.c b/src/managesieve/managesieve-client.c
index e505f8750..be8db5786 100644
--- a/src/managesieve/managesieve-client.c
+++ b/src/managesieve/managesieve-client.c
@@ -412,14 +412,28 @@ void client_send_storage_error
 }
 
 bool client_read_args(struct client_command_context *cmd, unsigned int count,
-		      unsigned int flags, struct managesieve_arg **args_r)
+	unsigned int flags, bool no_more, struct managesieve_arg **args_r)
 {
+	struct managesieve_arg *dummy_args_r = NULL;
 	int ret;
 
+	if ( args_r == NULL ) args_r = &dummy_args_r; 
+
 	i_assert(count <= INT_MAX);
 
-	ret = managesieve_parser_read_args(cmd->client->parser, count, flags, args_r);
-	if (ret >= (int)count) {
+	ret = managesieve_parser_read_args
+		(cmd->client->parser, ( no_more ? 0 : count ), flags, args_r);
+	if ( ret >= 0 ) {
+		if ( count > 0 || no_more ) {
+			if ( ret < (int)count ) {
+				client_send_command_error(cmd, "Missing arguments.");
+				return FALSE;
+			} else if ( no_more && ret > (int)count ) {
+				client_send_command_error(cmd, "Too many arguments.");
+				return FALSE;
+			}
+		}
+	
 		/* all parameters read successfully */
 		return TRUE;
 	} else if (ret == -2) {
@@ -430,15 +444,14 @@ bool client_read_args(struct client_command_context *cmd, unsigned int count,
 		}
 		return FALSE;
 	} else {
-		/* error, or missing arguments */
-		client_send_command_error(cmd, ret < 0 ? NULL :
-					  "Missing arguments");
+		/* error */
+		client_send_command_error(cmd, NULL);
 		return FALSE;
 	}
 }
 
 bool client_read_string_args(struct client_command_context *cmd,
-			     unsigned int count, ...)
+			     unsigned int count, bool no_more, ...)
 {
 	struct managesieve_arg *managesieve_args;
 	va_list va;
@@ -446,10 +459,10 @@ bool client_read_string_args(struct client_command_context *cmd,
 	unsigned int i;
 	bool result = TRUE;
 
-	if (!client_read_args(cmd, count, 0, &managesieve_args))
+	if (!client_read_args(cmd, count, 0, no_more, &managesieve_args))
 		return FALSE;
 
-	va_start(va, count);
+	va_start(va, no_more);
 	for (i = 0; i < count; i++) {
 		const char **ret = va_arg(va, const char **);
 
@@ -471,11 +484,6 @@ bool client_read_string_args(struct client_command_context *cmd,
 	}
 	va_end(va);
 
-	if (result && managesieve_args[i].type != MANAGESIEVE_ARG_EOL) {
-		client_send_command_error(cmd, "Too many arguments.");
-		result = FALSE;
-	}
-
 	return result;
 }
 
diff --git a/src/managesieve/managesieve-client.h b/src/managesieve/managesieve-client.h
index 32db5e02e..1c31d7ef6 100644
--- a/src/managesieve/managesieve-client.h
+++ b/src/managesieve/managesieve-client.h
@@ -114,11 +114,17 @@ void client_send_storage_error(struct client *client,
 /* Read a number of arguments. Returns TRUE if everything was read or
    FALSE if either needs more data or error occurred. */
 bool client_read_args(struct client_command_context *cmd, unsigned int count,
-		      unsigned int flags, struct managesieve_arg **args_r);
+		      unsigned int flags, bool no_more, struct managesieve_arg **args_r);
 /* Reads a number of string arguments. ... is a list of pointers where to
    store the arguments. */
 bool client_read_string_args(struct client_command_context *cmd,
-			     unsigned int count, ...);
+			     unsigned int count, bool no_more, ...);
+
+static inline bool client_read_no_args
+(struct client_command_context *cmd)
+{
+	return client_read_args(cmd, 0, 0, TRUE, NULL);
+}
 
 void _client_reset_command(struct client *client);
 void client_input(struct client *client);
-- 
GitLab