dovecot-2.0-pigeonhole: Imap4flags: fixed bug in setflag command...

[pigeonhole at rename-it.nl]

details:   http://hg.rename-it.nl/dovecot-2.0-pigeonhole/rev/1c66cb52772d
changeset: 1356:1c66cb52772d
user:      Stephan Bosch <stephan at rename-it.nl>
date:      Tue Aug 03 18:30:14 2010 +0200
description:
Imap4flags: fixed bug in setflag command; when parameter was a stringlist, only the last item was actually set.

diffstat:

 src/lib-sieve/plugins/imap4flags/cmd-flag.c              |  14 +---
 src/lib-sieve/plugins/imap4flags/ext-imap4flags-common.c |  80 ++++++++++++++------
 src/lib-sieve/plugins/imap4flags/ext-imap4flags-common.h |   8 +-
 tests/extensions/imap4flags/basic.svtest                 |  24 ++++++
 4 files changed, 86 insertions(+), 40 deletions(-)

diffs (194 lines):

diff -r 8ab83f81360e -r 1c66cb52772d src/lib-sieve/plugins/imap4flags/cmd-flag.c
--- a/src/lib-sieve/plugins/imap4flags/cmd-flag.c	Tue Aug 03 15:58:36 2010 +0200
+++ b/src/lib-sieve/plugins/imap4flags/cmd-flag.c	Tue Aug 03 18:30:14 2010 +0200
@@ -241,17 +241,7 @@
 		i_unreached();
 	}
 
-	/* Iterate through all flags and perform requested operation */
+	/* Perform requested operation */
 	
-	while ( (ret=sieve_stringlist_next_item(flag_list, &flag_item)) > 0 ) {
-		if ( (ret=flag_op(renv, storage, var_index, flag_item)) <= 0)
-			return ret;
-	}
-
-	if ( ret < 0 ) {	
-		sieve_runtime_trace_error(renv, "invalid flag-list item");
-		return SIEVE_EXEC_BIN_CORRUPT;
-	}
-
-	return SIEVE_EXEC_OK;
+	return flag_op(renv, storage, var_index, flag_list);
 }
diff -r 8ab83f81360e -r 1c66cb52772d src/lib-sieve/plugins/imap4flags/ext-imap4flags-common.c
--- a/src/lib-sieve/plugins/imap4flags/ext-imap4flags-common.c	Tue Aug 03 15:58:36 2010 +0200
+++ b/src/lib-sieve/plugins/imap4flags/ext-imap4flags-common.c	Tue Aug 03 18:30:14 2010 +0200
@@ -457,9 +457,43 @@
 	flags_list_add_flags(flags_list, flags);
 }
 
+static void flags_list_clear_flags
+(string_t *flags_list)
+{
+	str_truncate(flags_list, 0);
+}
+
 int ext_imap4flags_set_flags
 (const struct sieve_runtime_env *renv, struct sieve_variable_storage *storage,
-	unsigned int var_index, string_t *flags)
+	unsigned int var_index, struct sieve_stringlist *flags)
+{
+	string_t *cur_flags;
+	
+	if ( storage != NULL ) {
+		if ( !sieve_variable_get_modifiable(storage, var_index, &cur_flags) )
+			return SIEVE_EXEC_BIN_CORRUPT;
+	} else {
+		cur_flags = _get_flags_string(renv->oprtn->ext, renv->result);
+	}
+	
+	if ( cur_flags != NULL ) {
+		string_t *flags_item;
+		int ret;
+
+		flags_list_clear_flags(cur_flags);
+		while ( (ret=sieve_stringlist_next_item(flags, &flags_item)) > 0 ) {
+			flags_list_add_flags(cur_flags, flags_item);
+		}
+
+		if ( ret < 0 ) return SIEVE_EXEC_BIN_CORRUPT;
+	}
+
+	return SIEVE_EXEC_OK;
+}
+
+int ext_imap4flags_add_flags
+(const struct sieve_runtime_env *renv, struct sieve_variable_storage *storage, 
+	unsigned int var_index, struct sieve_stringlist *flags)
 {
 	string_t *cur_flags;
 	
@@ -469,15 +503,23 @@
 	} else
 		cur_flags = _get_flags_string(renv->oprtn->ext, renv->result);
 
-	if ( cur_flags != NULL )
-		flags_list_set_flags(cur_flags, flags);		
+	if ( cur_flags != NULL ) {
+		string_t *flags_item;
+		int ret;
+
+		while ( (ret=sieve_stringlist_next_item(flags, &flags_item)) > 0 ) {
+			flags_list_add_flags(cur_flags, flags_item);
+		}
+
+		if ( ret < 0 ) return SIEVE_EXEC_BIN_CORRUPT;
+	}
 
 	return SIEVE_EXEC_OK;
 }
 
-int ext_imap4flags_add_flags
+int ext_imap4flags_remove_flags
 (const struct sieve_runtime_env *renv, struct sieve_variable_storage *storage, 
-	unsigned int var_index, string_t *flags)
+	unsigned int var_index, struct sieve_stringlist *flags)
 {
 	string_t *cur_flags;
 	
@@ -487,26 +529,16 @@
 	} else
 		cur_flags = _get_flags_string(renv->oprtn->ext, renv->result);
 	
-	if ( cur_flags != NULL )
-		flags_list_add_flags(cur_flags, flags);
-	
-	return SIEVE_EXEC_OK;	
-}
+	if ( cur_flags != NULL ) {
+		string_t *flags_item;
+		int ret;
 
-int ext_imap4flags_remove_flags
-(const struct sieve_runtime_env *renv, struct sieve_variable_storage *storage, 
-	unsigned int var_index, string_t *flags)
-{
-	string_t *cur_flags;
-	
-	if ( storage != NULL ) {
-		if ( !sieve_variable_get_modifiable(storage, var_index, &cur_flags) )
-			return SIEVE_EXEC_BIN_CORRUPT;
-	} else
-		cur_flags = _get_flags_string(renv->oprtn->ext, renv->result);
-	
-	if ( cur_flags != NULL )
-		flags_list_remove_flags(cur_flags, flags);		
+		while ( (ret=sieve_stringlist_next_item(flags, &flags_item)) > 0 ) {
+			flags_list_remove_flags(cur_flags, flags_item);
+		}
+
+		if ( ret < 0 ) return SIEVE_EXEC_BIN_CORRUPT;
+	}
 
 	return SIEVE_EXEC_OK;
 }
diff -r 8ab83f81360e -r 1c66cb52772d src/lib-sieve/plugins/imap4flags/ext-imap4flags-common.h
--- a/src/lib-sieve/plugins/imap4flags/ext-imap4flags-common.h	Tue Aug 03 15:58:36 2010 +0200
+++ b/src/lib-sieve/plugins/imap4flags/ext-imap4flags-common.h	Tue Aug 03 18:30:14 2010 +0200
@@ -90,17 +90,17 @@
 
 typedef int (*ext_imapflag_flag_operation_t)
 	(const struct sieve_runtime_env *renv, struct sieve_variable_storage *storage,
-		unsigned int var_index, string_t *flags);
+		unsigned int var_index, struct sieve_stringlist *flags);
 
 int ext_imap4flags_set_flags
 	(const struct sieve_runtime_env *renv, struct sieve_variable_storage *storage,
-		unsigned int var_index, string_t *flags);
+		unsigned int var_index, struct sieve_stringlist *flags);
 int ext_imap4flags_add_flags
 	(const struct sieve_runtime_env *renv, struct sieve_variable_storage *storage,
-		unsigned int var_index, string_t *flags);
+		unsigned int var_index, struct sieve_stringlist *flags);
 int ext_imap4flags_remove_flags
 	(const struct sieve_runtime_env *renv, struct sieve_variable_storage *storage,
-		unsigned int var_index, string_t *flags);
+		unsigned int var_index, struct sieve_stringlist *flags);
 
 /* Flags access */
 
diff -r 8ab83f81360e -r 1c66cb52772d tests/extensions/imap4flags/basic.svtest
--- a/tests/extensions/imap4flags/basic.svtest	Tue Aug 03 15:58:36 2010 +0200
+++ b/tests/extensions/imap4flags/basic.svtest	Tue Aug 03 18:30:14 2010 +0200
@@ -104,6 +104,30 @@
 	}
 }
 
+test "Setflag; string list" {
+	setflag ["A B", "C D"];
+
+	if not hasflag "A" {
+		test_fail "hasflag misses A flag";
+	}
+
+	if not hasflag "B" {
+		test_fail "hasflag misses B flag";
+	}
+
+	if not hasflag "C" {
+		test_fail "hasflag misses C flag";
+	}
+
+	if not hasflag "D" {
+		test_fail "hasflag misses D flag";
+	}
+
+	if hasflag :comparator "i;ascii-numeric" :count "ne" "4" {
+		test_fail "hasflag sees incorrect number of flags";
+	}
+}
+
 test "Removal: one" {
 	setflag "\\seen";