From e6995fc0ffbb091d64bc01838900b70eb91067bb Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Thu, 24 Jul 2008 17:35:47 +0200
Subject: [PATCH] Imapflags: improved handling of invalid flags.

---
 TODO                                          |  2 +-
 src/lib-sieve/cmd-redirect.c                  |  2 +-
 src/lib-sieve/plugins/imapflags/cmd-addflag.c |  8 +---
 .../plugins/imapflags/cmd-removeflag.c        |  8 +---
 src/lib-sieve/plugins/imapflags/cmd-setflag.c |  8 +---
 .../plugins/imapflags/ext-imapflags-common.c  | 39 +++++++++++++++++--
 .../plugins/imapflags/ext-imapflags-common.h  |  8 ++++
 .../plugins/imapflags/ext-imapflags.c         |  8 ----
 .../plugins/imapflags/imapflags-errors.sieve  |  4 +-
 .../plugins/imapflags/imapflags.sieve         |  2 +-
 src/lib-sieve/plugins/imapflags/tag-flags.c   | 26 ++++++++++++-
 src/lib-sieve/plugins/imapflags/tst-hasflag.c |  9 +----
 src/lib-sieve/sieve-commands.h                |  2 +
 src/lib-sieve/sieve-result.c                  | 10 +++++
 src/lib-sieve/sieve-result.h                  |  3 ++
 15 files changed, 93 insertions(+), 46 deletions(-)

diff --git a/TODO b/TODO
index e565a0626..5f8352620 100644
--- a/TODO
+++ b/TODO
@@ -1,7 +1,7 @@
 Next (in order of descending priority/precedence):
 * Review sieve-address parsing implementation for past-end checks
 * Improve handling of old/corrupt binaries.
-* Move to 1.2 and start using mailbox_keyword_is_valid() and new const str. 
+* Move to 1.2 and start using and new const str. 
 * Resolve problems in variables extension: scope uses hash the wrong way and
   extension ids are emitted directly.
 
diff --git a/src/lib-sieve/cmd-redirect.c b/src/lib-sieve/cmd-redirect.c
index 575d02eeb..90d6ceddf 100644
--- a/src/lib-sieve/cmd-redirect.c
+++ b/src/lib-sieve/cmd-redirect.c
@@ -118,7 +118,7 @@ static bool cmd_redirect_validate
 
 	/* We can only assess the validity of the outgoing address when it is 
 	 * a string literal. For runtime-generated strings this needs to be 
-	 * done at runtime 
+	 * done at runtime (FIXME!)
      */
 	if ( sieve_argument_is_string_literal(arg) ) {
 		string_t *address = sieve_ast_argument_str(arg);
diff --git a/src/lib-sieve/plugins/imapflags/cmd-addflag.c b/src/lib-sieve/plugins/imapflags/cmd-addflag.c
index 17c11aa02..d65fa1b7e 100644
--- a/src/lib-sieve/plugins/imapflags/cmd-addflag.c
+++ b/src/lib-sieve/plugins/imapflags/cmd-addflag.c
@@ -75,13 +75,9 @@ static bool cmd_addflag_operation_execute
 	
 	sieve_runtime_trace(renv, "ADDFLAG command");
 	
-	t_push();
-	
 	if ( !ext_imapflags_command_operands_read
-		(renv, address, &flag_list, &storage, &var_index) ) {
-		t_pop();
+		(renv, address, &flag_list, &storage, &var_index) )
 		return FALSE;
-	}
 	
 	/* Iterate through all added flags */	
 	while ( (result=sieve_coded_stringlist_next_item(flag_list, &flag_item)) && 
@@ -89,7 +85,5 @@ static bool cmd_addflag_operation_execute
 		ext_imapflags_add_flags(renv, storage, var_index, flag_item);
 	}
 
-	t_pop();
-	
 	return result;
 }
diff --git a/src/lib-sieve/plugins/imapflags/cmd-removeflag.c b/src/lib-sieve/plugins/imapflags/cmd-removeflag.c
index 1ccb4846d..c2cce0839 100644
--- a/src/lib-sieve/plugins/imapflags/cmd-removeflag.c
+++ b/src/lib-sieve/plugins/imapflags/cmd-removeflag.c
@@ -76,21 +76,15 @@ static bool cmd_removeflag_operation_execute
 	
 	sieve_runtime_trace(renv, "REMOVEFLAG command");
 	
-	t_push();
-	
 	if ( !ext_imapflags_command_operands_read
-		(renv, address, &flag_list, &storage, &var_index) ) {
-		t_pop();
+		(renv, address, &flag_list, &storage, &var_index) )
 		return FALSE;
-	}
 	
 	/* Iterate through all flags to remove */
 	while ( (result=sieve_coded_stringlist_next_item(flag_list, &flag_item)) && 
 		flag_item != NULL ) {
 		ext_imapflags_remove_flags(renv, storage, var_index, flag_item);
 	}
-
-	t_pop();
 	
 	return result;
 }
diff --git a/src/lib-sieve/plugins/imapflags/cmd-setflag.c b/src/lib-sieve/plugins/imapflags/cmd-setflag.c
index 74e03c6b8..2d166e974 100644
--- a/src/lib-sieve/plugins/imapflags/cmd-setflag.c
+++ b/src/lib-sieve/plugins/imapflags/cmd-setflag.c
@@ -74,13 +74,9 @@ static bool cmd_setflag_operation_execute
 	
 	sieve_runtime_trace(renv, "SETFLAG command");
 	
-	t_push();
-	
 	if ( !ext_imapflags_command_operands_read
-		(renv, address, &flag_list, &storage, &var_index) ) {
-		t_pop();
+		(renv, address, &flag_list, &storage, &var_index) ) 
 		return FALSE;
-	}
 			
 	/* Iterate through all flags to set */
 	while ( (result=sieve_coded_stringlist_next_item(flag_list, &flag_item)) && 
@@ -88,8 +84,6 @@ static bool cmd_setflag_operation_execute
 		ext_imapflags_set_flags(renv, storage, var_index, flag_item);
 	}
 
-	t_pop();
-	
 	return result;
 }
 
diff --git a/src/lib-sieve/plugins/imapflags/ext-imapflags-common.c b/src/lib-sieve/plugins/imapflags/ext-imapflags-common.c
index 3ebe32520..f8454f2ce 100644
--- a/src/lib-sieve/plugins/imapflags/ext-imapflags-common.c
+++ b/src/lib-sieve/plugins/imapflags/ext-imapflags-common.c
@@ -18,6 +18,12 @@
 extern const struct sieve_argument tag_flags;
 extern const struct sieve_argument tag_flags_implicit;
 
+/*
+ * Forward declarations
+ */
+
+static bool flag_is_valid(const char *flag);
+
 /* Common functions */
 
 bool ext_imapflags_command_validate
@@ -77,7 +83,7 @@ bool ext_imapflags_command_validate
 			sieve_ast_argument_type(arg2) != SAAT_STRING_LIST ) 
 		{
 			sieve_command_validate_error(validator, cmd, 
-				"the %s %s command expects a string list (list of flags) as "
+				"the %s %s expects a string list (list of flags) as "
 				"second argument when two arguments are specified, "
 				"but %s was found",
 				cmd->command->identifier, sieve_command_type_name(cmd->command),
@@ -87,7 +93,28 @@ bool ext_imapflags_command_validate
 	} else
 		arg2 = arg;
 
-	return sieve_validator_argument_activate(validator, cmd, arg2, FALSE);	
+	if ( !sieve_validator_argument_activate(validator, cmd, arg2, FALSE) )
+		return FALSE;
+
+	if ( cmd->command != &tst_hasflag && sieve_argument_is_string_literal(arg2) ) {
+		struct ext_imapflags_iter fiter;
+		const char *flag;
+		
+		/* Warn the user about validity of verifiable flags */
+		ext_imapflags_iter_init(&fiter, sieve_ast_argument_str(arg));
+
+		while ( (flag=ext_imapflags_iter_get_flag(&fiter)) != NULL ) {
+			if ( !flag_is_valid(flag) ) {
+				sieve_command_validate_warning(validator, cmd,
+                	"IMAP flag '%s' specified for the %s command is invalid "
+					"and will be ignored (only first invalid is reported)",					
+					flag, cmd->command->identifier);
+				break;
+			}
+		}
+	}
+
+	return TRUE;
 }
 
 bool ext_imapflags_command_operands_dump
@@ -231,7 +258,13 @@ static bool flag_is_valid(const char *flag)
     	return FALSE;
     }
 	} else {
-		/* Custom keyword: currently nothing to validate */					
+		/* Custom keyword:
+		 *
+		 * The validity of the keyword cannot be validated until the 
+		 * target mailbox for the message is known. Meaning that the 
+		 * verfication of keyword can only be performed when the
+		 * action side effect is about to be executed.
+		 */					
 	}
 
 	return TRUE;  
diff --git a/src/lib-sieve/plugins/imapflags/ext-imapflags-common.h b/src/lib-sieve/plugins/imapflags/ext-imapflags-common.h
index 7be3b498d..3df0f58c0 100644
--- a/src/lib-sieve/plugins/imapflags/ext-imapflags-common.h
+++ b/src/lib-sieve/plugins/imapflags/ext-imapflags-common.h
@@ -18,6 +18,14 @@ enum ext_imapflags_opcode {
 	EXT_IMAPFLAGS_OPERATION_HASFLAG
 };
 
+/* Commands */
+
+extern const struct sieve_command cmd_setflag;
+extern const struct sieve_command cmd_addflag;
+extern const struct sieve_command cmd_removeflag;
+
+extern const struct sieve_command tst_hasflag;
+
 bool ext_imapflags_command_validate
 	(struct sieve_validator *validator, struct sieve_command_context *cmd);
 
diff --git a/src/lib-sieve/plugins/imapflags/ext-imapflags.c b/src/lib-sieve/plugins/imapflags/ext-imapflags.c
index 3a785f428..c09afcafd 100644
--- a/src/lib-sieve/plugins/imapflags/ext-imapflags.c
+++ b/src/lib-sieve/plugins/imapflags/ext-imapflags.c
@@ -32,14 +32,6 @@ static bool ext_imapflags_validator_load(struct sieve_validator *valdtr);
 static bool ext_imapflags_runtime_load
 	(const struct sieve_runtime_env *renv);
 
-/* Commands */
-
-extern const struct sieve_command cmd_setflag;
-extern const struct sieve_command cmd_addflag;
-extern const struct sieve_command cmd_removeflag;
-
-extern const struct sieve_command tst_hasflag;
-
 /* Operations */
 
 extern const struct sieve_operation setflag_operation;
diff --git a/src/lib-sieve/plugins/imapflags/imapflags-errors.sieve b/src/lib-sieve/plugins/imapflags/imapflags-errors.sieve
index c3fae9ac1..02400c919 100644
--- a/src/lib-sieve/plugins/imapflags/imapflags-errors.sieve
+++ b/src/lib-sieve/plugins/imapflags/imapflags-errors.sieve
@@ -1,4 +1,4 @@
-require "imapflags";
+require "imap4flags";
 require "fileinto";
 
 setflag;
@@ -16,3 +16,5 @@ if hasflag 3 {
 if hasflag "flagvar" ["$MDNRequired", "\\Seen"] {
     removeflag "$MDNRequired";
 }
+
+removeflag "\\frop";
diff --git a/src/lib-sieve/plugins/imapflags/imapflags.sieve b/src/lib-sieve/plugins/imapflags/imapflags.sieve
index 53c0fdc18..2503134b6 100644
--- a/src/lib-sieve/plugins/imapflags/imapflags.sieve
+++ b/src/lib-sieve/plugins/imapflags/imapflags.sieve
@@ -25,4 +25,4 @@ if hasflag :count "ge" :comparator "i;ascii-numeric" "2" {
 	fileinto "imap-twoflags";
 }
 
-fileinto :flags "MDNRequired \\Draft" "INBOX";
+fileinto :flags "MDNRequired \\Draft 3[\"]\\4" "INBOX";
diff --git a/src/lib-sieve/plugins/imapflags/tag-flags.c b/src/lib-sieve/plugins/imapflags/tag-flags.c
index 2b867fe2e..e05fa4353 100644
--- a/src/lib-sieve/plugins/imapflags/tag-flags.c
+++ b/src/lib-sieve/plugins/imapflags/tag-flags.c
@@ -13,6 +13,8 @@
 
 #include "ext-imapflags-common.h"
 
+#include <ctype.h>
+
 static bool tag_flags_validate
 	(struct sieve_validator *validator,	struct sieve_ast_argument **arg, 
 		struct sieve_command_context *cmd);
@@ -274,7 +276,7 @@ static void seff_flags_print
 				str_printfa(flags, " %s", *keyword);
 			}
 
-			sieve_result_seffect_printf(rpenv, "add flags:%s", str_c(flags));
+			sieve_result_seffect_printf(rpenv, "add IMAP flags:%s", str_c(flags));
 		} T_END;
 	}
 }
@@ -296,12 +298,32 @@ static bool seff_flags_pre_execute
 
 	/* Assign mail keywords for subsequent mailbox_copy() */
 	if ( array_count(&ctx->keywords) > 0 ) {
+		unsigned int i;
+
 		if ( !array_is_created(&trans->keywords) ) {
 			pool_t pool = sieve_result_pool(aenv->result); 
 			p_array_init(&trans->keywords, pool, 2);
 		}
 		
-		array_append_array(&trans->keywords, &ctx->keywords);
+		for ( i = 0; i < array_count(&ctx->keywords); i++ ) {		
+			const char *const *keyword = array_idx(&ctx->keywords, i);
+			const char *kw_error;
+
+			if ( trans->box != NULL && 
+				mailbox_keyword_is_valid(trans->box, *keyword, &kw_error) )
+				array_append(&trans->keywords, keyword, 1);
+			else {
+				char *error = "";
+				if ( kw_error != NULL && *kw_error != '\0' ) {
+					error = t_strdup_noconst(kw_error);
+					error[0] = i_tolower(error[0]);
+				}
+				
+				sieve_result_warning(aenv, 
+					"specified IMAP keyword '%s' is invalid (ignored): %s", 
+					*keyword, error);
+			}
+		}
 	}
 
 	/* Assign mail flags for subsequent mailbox_copy() */
diff --git a/src/lib-sieve/plugins/imapflags/tst-hasflag.c b/src/lib-sieve/plugins/imapflags/tst-hasflag.c
index d221bcf41..9f1d55ee1 100644
--- a/src/lib-sieve/plugins/imapflags/tst-hasflag.c
+++ b/src/lib-sieve/plugins/imapflags/tst-hasflag.c
@@ -188,14 +188,9 @@ static bool tst_hasflag_operation_execute
 		}
 	}
 
-	t_push();
-		
 	if ( !ext_imapflags_command_operands_read
-		(renv, address, &flag_list, &storage, &var_index) ) {
-		t_pop();
+		(renv, address, &flag_list, &storage, &var_index) )
 		return FALSE;
-	}
-
 
 	matched = FALSE;
 	mctx = sieve_match_begin(renv->interp, mtch, cmp, flag_list); 	
@@ -209,8 +204,6 @@ static bool tst_hasflag_operation_execute
 
 	matched = sieve_match_end(mctx) || matched; 	
 	
-	t_pop();
-	
 	sieve_interpreter_set_test_result(renv->interp, matched);
 	
 	return TRUE;
diff --git a/src/lib-sieve/sieve-commands.h b/src/lib-sieve/sieve-commands.h
index 7e5b2ab2d..cb27d82c8 100644
--- a/src/lib-sieve/sieve-commands.h
+++ b/src/lib-sieve/sieve-commands.h
@@ -94,6 +94,8 @@ const char *sieve_command_type_name(const struct sieve_command *command);
 		
 #define sieve_command_validate_error(validator, context, ...) \
 	sieve_validator_error(validator, (context)->ast_node, __VA_ARGS__)
+#define sieve_command_validate_warning(validator, context, ...) \
+	sieve_validator_warning(validator, (context)->ast_node, __VA_ARGS__)
 #define sieve_command_validate_critical(validator, context, ...) \
 	sieve_validator_critical(validator, (context)->ast_node, __VA_ARGS__)
 
diff --git a/src/lib-sieve/sieve-result.c b/src/lib-sieve/sieve-result.c
index 8382c4cde..c44c31ca5 100644
--- a/src/lib-sieve/sieve-result.c
+++ b/src/lib-sieve/sieve-result.c
@@ -157,6 +157,16 @@ void sieve_result_error
 	va_end(args);
 }
 
+void sieve_result_warning
+	(const struct sieve_action_exec_env *aenv, const char *fmt, ...)
+{
+	va_list args;
+	
+	va_start(args, fmt);	
+	sieve_vwarning(aenv->result->ehandler, _get_location(aenv), fmt, args); 
+	va_end(args);
+}
+
 void sieve_result_log
 	(const struct sieve_action_exec_env *aenv, const char *fmt, ...)
 {
diff --git a/src/lib-sieve/sieve-result.h b/src/lib-sieve/sieve-result.h
index 79f016e3e..e7860ab69 100644
--- a/src/lib-sieve/sieve-result.h
+++ b/src/lib-sieve/sieve-result.h
@@ -40,6 +40,9 @@ bool sieve_result_print(struct sieve_result *result, struct ostream *stream);
 void sieve_result_log
 	(const struct sieve_action_exec_env *aenv, const char *fmt, ...)
 		ATTR_FORMAT(2, 3);
+void sieve_result_warning
+	(const struct sieve_action_exec_env *aenv, const char *fmt, ...)
+		ATTR_FORMAT(2, 3);
 void sieve_result_error
 	(const struct sieve_action_exec_env *aenv, const char *fmt, ...)
 		ATTR_FORMAT(2, 3);
-- 
GitLab