From 7e4242639828414fea76bbc158f5e54ef13c52d5 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Sun, 31 Aug 2008 15:43:23 +0200
Subject: [PATCH] Imapflags: properly implemented handling of duplicate store
 actions with different :flags.

---
 Makefile.am                                 |  1 +
 TODO                                        |  3 -
 src/lib-sieve/cmd-keep.c                    |  3 +-
 src/lib-sieve/plugins/copy/ext-copy.c       |  2 +-
 src/lib-sieve/plugins/imapflags/tag-flags.c | 18 ++++
 src/lib-sieve/sieve-actions.h               | 10 ++-
 src/lib-sieve/sieve-generator.c             |  4 +
 src/lib-sieve/sieve-result.c                | 99 +++++++++++++++++++--
 8 files changed, 126 insertions(+), 14 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index ca86a326e..8f05f1a84 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -39,6 +39,7 @@ test_cases = \
 	tests/extensions/include/errors.svtest \
 	tests/extensions/imapflags/basic.svtest \
 	tests/extensions/imapflags/hasflag.svtest \
+	tests/extensions/imapflags/execute.svtest \
 	tests/extensions/body/basic.svtest \
 	tests/extensions/body/match-values.svtest \
 	tests/extensions/regex/basic.svtest \
diff --git a/TODO b/TODO
index 14d62bdb8..1745b4462 100644
--- a/TODO
+++ b/TODO
@@ -2,9 +2,6 @@ Next (in order of descending priority/precedence):
 * Fix standards compliance issues:
 	- Body: contains various bugs that need to be resolved for standards
 	  compliance.
-	- Imapflags: when keep/fileinto is used multiple times in a script and
-	  duplicate message elimination is performed, the last flag list value
-	  MUST win.
 	- 'If an address is not syntactically valid, then it will not be matched
 	  by tests specifying ":localpart" or ":domain"'.
 	- Vacation: If the Sieve variables extension is used, the arguments 
diff --git a/src/lib-sieve/cmd-keep.c b/src/lib-sieve/cmd-keep.c
index 537ce6f6a..88dc8f3d1 100644
--- a/src/lib-sieve/cmd-keep.c
+++ b/src/lib-sieve/cmd-keep.c
@@ -66,7 +66,8 @@ static bool cmd_keep_generate
 	/* Emit line number */
     sieve_code_source_line_emit(cgenv->sbin, sieve_command_source_line(ctx));
 
-	return TRUE;
+	/* Generate arguments */
+	return sieve_generate_arguments(cgenv, ctx, NULL);
 }
 
 /* 
diff --git a/src/lib-sieve/plugins/copy/ext-copy.c b/src/lib-sieve/plugins/copy/ext-copy.c
index ba90cbc44..2548c8aa7 100644
--- a/src/lib-sieve/plugins/copy/ext-copy.c
+++ b/src/lib-sieve/plugins/copy/ext-copy.c
@@ -84,7 +84,7 @@ static bool seff_copy_post_commit
 const struct sieve_side_effect copy_side_effect = {
 	SIEVE_OBJECT("copy", &copy_side_effect_operand, 0),
 	&act_store,
-	NULL, NULL,
+	NULL, NULL, NULL,
 	seff_copy_print,
 	NULL, NULL,
 	seff_copy_post_commit, 
diff --git a/src/lib-sieve/plugins/imapflags/tag-flags.c b/src/lib-sieve/plugins/imapflags/tag-flags.c
index a8b9abeee..5eac55963 100644
--- a/src/lib-sieve/plugins/imapflags/tag-flags.c
+++ b/src/lib-sieve/plugins/imapflags/tag-flags.c
@@ -49,6 +49,10 @@ static bool seff_flags_read_context
 	(const struct sieve_side_effect *seffect, 
 		const struct sieve_runtime_env *renv, sieve_size_t *address,
 		void **se_context);
+static int seff_flags_merge
+	(const struct sieve_runtime_env *renv, const struct sieve_action *action, 
+		const struct sieve_side_effect *seffect, 
+		void **old_context, void *new_context);
 static void seff_flags_print
 	(const struct sieve_side_effect *seffect, const struct sieve_action *action,
 		const struct sieve_result_print_env *rpenv, void *se_context, bool *keep);
@@ -63,6 +67,7 @@ const struct sieve_side_effect flags_side_effect = {
 
 	seff_flags_dump_context,
 	seff_flags_read_context,
+	seff_flags_merge,
 	seff_flags_print,
 	seff_flags_pre_execute, 
 	NULL, NULL, NULL
@@ -260,6 +265,19 @@ static struct seff_flags_context *seff_flags_get_implicit_context
 	return ctx;
 }
 
+/* Result verification */
+
+static int seff_flags_merge
+(const struct sieve_runtime_env *renv ATTR_UNUSED, 
+	const struct sieve_action *action ATTR_UNUSED, 
+	const struct sieve_side_effect *seffect ATTR_UNUSED, 
+	void **old_context, void *new_context)
+{
+	*old_context = new_context;
+	
+	return 1;
+}
+
 /* Result printing */
 
 static void seff_flags_print
diff --git a/src/lib-sieve/sieve-actions.h b/src/lib-sieve/sieve-actions.h
index f845f14b0..915aca842 100644
--- a/src/lib-sieve/sieve-actions.h
+++ b/src/lib-sieve/sieve-actions.h
@@ -82,8 +82,9 @@ struct sieve_side_effect {
 	struct sieve_object object;
 	
 	/* The action it is supposed to link to */
-	const struct sieve_action *to_action;
 	
+	const struct sieve_action *to_action;
+		
 	/* Context coding */
 	
 	bool (*dump_context)
@@ -96,8 +97,11 @@ struct sieve_side_effect {
 		
 	/* Result verification */
 	
-	/* ... */
-	
+	int (*merge)
+		(const struct sieve_runtime_env *renv, const struct sieve_action *action, 
+			const struct sieve_side_effect *seffect, 
+			void **old_context, void *new_context);
+
 	/* Result printing */	
 			
 	void (*print)
diff --git a/src/lib-sieve/sieve-generator.c b/src/lib-sieve/sieve-generator.c
index 937ea5652..afac539c9 100644
--- a/src/lib-sieve/sieve-generator.c
+++ b/src/lib-sieve/sieve-generator.c
@@ -272,6 +272,10 @@ bool sieve_generate_arguments
 
 		arg = sieve_ast_argument_next(arg);
 	}
+
+	/* Mark end of optional list if it is still open */
+	if ( state == ARG_OPTIONAL )
+		sieve_binary_emit_byte(cgenv->sbin, 0);
 	
 	if ( last_arg != NULL )
 		*last_arg = arg;
diff --git a/src/lib-sieve/sieve-result.c b/src/lib-sieve/sieve-result.c
index e77828639..c2c1f9029 100644
--- a/src/lib-sieve/sieve-result.c
+++ b/src/lib-sieve/sieve-result.c
@@ -225,6 +225,87 @@ void sieve_result_add_implicit_side_effect
 	sieve_side_effects_list_add(actctx->seffects, seffect, context);
 }
 
+static int sieve_result_side_effects_merge
+(const struct sieve_runtime_env *renv, const struct sieve_action *action, 
+	struct sieve_side_effects_list *old_seffects,
+	struct sieve_side_effects_list *new_seffects)
+{
+	int ret;
+	struct sieve_result_side_effect *rsef, *nrsef;
+
+	/* Allow side-effects to merge with existing copy */
+		
+	/* Merge existing side effects */
+	rsef = old_seffects != NULL ? old_seffects->first_effect : NULL;
+	while ( rsef != NULL ) {
+		const struct sieve_side_effect *seffect = rsef->seffect;
+		bool found = FALSE;
+		
+		if ( seffect->merge != NULL ) {
+
+			/* Try to find it among the new */
+			nrsef = new_seffects != NULL ? new_seffects->first_effect : NULL;
+			while ( nrsef != NULL ) {
+				if ( nrsef->seffect == seffect ) {
+					if ( seffect->merge
+						(renv, action, seffect, &rsef->context, nrsef->context) < 0 )
+						return -1;
+			
+					found = TRUE;
+					break;
+				}
+		
+				nrsef = nrsef->next;
+			}
+	
+			/* Not found? */
+			if ( !found && seffect->merge
+				(renv, action, seffect, &rsef->context, NULL) < 0 )
+				return -1;
+		}
+	
+		rsef = rsef->next;
+	}
+
+	/* Merge new Side effects */
+	nrsef = new_seffects != NULL ? new_seffects->first_effect : NULL;
+	while ( nrsef != NULL ) {
+		const struct sieve_side_effect *seffect = nrsef->seffect;
+		bool found = FALSE;
+		
+		if ( seffect->merge != NULL ) {
+		
+			/* Try to find it in among the exising */
+			rsef = old_seffects != NULL ? old_seffects->first_effect : NULL;
+			while ( rsef != NULL ) {
+				if ( rsef->seffect == seffect ) {
+					found = TRUE;
+					break;
+				}
+				rsef = rsef->next;
+			}
+	
+			/* Not found? */
+			if ( !found ) {
+				void *new_context = NULL; 
+		
+				if ( (ret=seffect->merge
+					(renv, action, seffect, &new_context, nrsef->context)) < 0 ) 
+					return -1;
+					
+				if ( ret != 0 ) {
+					/* Add side effect */
+					sieve_side_effects_list_add(new_seffects, seffect, new_context);
+				}
+			}
+		}
+	
+		nrsef = nrsef->next;
+	}
+	
+	return 1;
+}
+
 int sieve_result_add_action
 (const struct sieve_runtime_env *renv,
 	const struct sieve_action *action, struct sieve_side_effects_list *seffects,
@@ -249,10 +330,15 @@ int sieve_result_add_action
 			if ( action->check_duplicate != NULL ) {
 				if ( (ret=action->check_duplicate
 					(renv, action, context, raction->context,
-						location, raction->location)) != 0 )
+						location, raction->location)) < 0 )
 					return ret;
-			} else 
-				return 1; 
+				
+				/* Duplicate */	
+				if ( ret == 1 ) {
+					return sieve_result_side_effects_merge
+						(renv, action, raction->seffects, seffects);
+				}
+			}
 		} else {
 			/* Check conflict */
 			if ( action->check_conflict != NULL &&
@@ -270,14 +356,15 @@ int sieve_result_add_action
 
 	/* Check policy limit on total number of actions */
 	if ( sieve_max_actions > 0 && result->action_count >= sieve_max_actions ) {
-		sieve_runtime_error(renv, location, "total number of actions exceeds policy limit");
+		sieve_runtime_error(renv, location, 
+			"total number of actions exceeds policy limit");
 		return -1;
 	}
 
 	/* Check policy limit on number of this class of actions */
 	if ( instance_limit > 0 && instance_count >= instance_limit ) {
-		sieve_runtime_error(renv, location, "number of %s actions exceeds policy limit",
-			action->name);
+		sieve_runtime_error(renv, location, 
+			"number of %s actions exceeds policy limit", action->name);
 		return -1;
 	}	
 		
-- 
GitLab