From a26cb52c7050fc3fca6043460236851a66754ed6 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Sun, 27 Jul 2008 13:08:33 +0200
Subject: [PATCH] Imapflags: resolved problem of hasflags encountering
 duplicate flags in flag lists contained in a variable.

---
 TODO                                          |  2 -
 .../plugins/imapflags/ext-imapflags-common.c  |  9 ++-
 src/lib-sieve/plugins/imapflags/tag-flags.c   |  1 +
 src/lib-sieve/plugins/relational/mcht-count.c | 13 ++--
 src/testsuite/Makefile.am                     |  1 +
 .../tests/extensions/imapflags/basic.svtest   | 68 ++++---------------
 6 files changed, 30 insertions(+), 64 deletions(-)

diff --git a/TODO b/TODO
index d20b76be5..8be5f5f4d 100644
--- a/TODO
+++ b/TODO
@@ -1,7 +1,5 @@
 Next (in order of descending priority/precedence):
 * Resolve problems in variables extension: scope uses hash the wrong way.
-* Resolve problems in imapflags extension: variables should not be trusted to 
-  contain a valid flags list.
 
 * Full standards compliance review for the engine and all fully implemented 
   sieve extensions. Issues discovered so far:
diff --git a/src/lib-sieve/plugins/imapflags/ext-imapflags-common.c b/src/lib-sieve/plugins/imapflags/ext-imapflags-common.c
index 4d4406e5d..c8bc024f6 100644
--- a/src/lib-sieve/plugins/imapflags/ext-imapflags-common.c
+++ b/src/lib-sieve/plugins/imapflags/ext-imapflags-common.c
@@ -468,8 +468,13 @@ void ext_imapflags_get_flags_init
 {
 	string_t *cur_flags;
 	
-	if ( storage != NULL ) 
-		sieve_variable_get_modifiable(storage, var_index, &cur_flags);
+	if ( storage != NULL ) {
+		string_t *raw_flags;
+		cur_flags = t_str_new(256);
+		
+		sieve_variable_get_modifiable(storage, var_index, &raw_flags);
+		flags_list_set_flags(cur_flags, raw_flags);
+	}
 	else
 		cur_flags = _get_flags_string(renv->result);
 	
diff --git a/src/lib-sieve/plugins/imapflags/tag-flags.c b/src/lib-sieve/plugins/imapflags/tag-flags.c
index e05fa4353..f36101b34 100644
--- a/src/lib-sieve/plugins/imapflags/tag-flags.c
+++ b/src/lib-sieve/plugins/imapflags/tag-flags.c
@@ -174,6 +174,7 @@ static bool seff_flags_read_context
 				/* keyword */
 				const char *keyword = p_strdup(pool, flag);
 
+				/* FIXME: should check for duplicates (cannot trust variables) */
 				array_append(&ctx->keywords, &keyword, 1);
 
 			} else {
diff --git a/src/lib-sieve/plugins/relational/mcht-count.c b/src/lib-sieve/plugins/relational/mcht-count.c
index 268ced240..874f22b01 100644
--- a/src/lib-sieve/plugins/relational/mcht-count.c
+++ b/src/lib-sieve/plugins/relational/mcht-count.c
@@ -92,17 +92,22 @@ static int mcht_count_match_deinit(struct sieve_match_context *mctx)
 
 	string_t *value = t_str_new(20);
 	str_printfa(value, "%d", val_count);
-	
+
     /* Match to all key values */
     key_index = 0;
     key_item = NULL;
     while ( (ok=sieve_coded_stringlist_next_item(mctx->key_list, &key_item)) 
 		&& key_item != NULL )
     {
-        if ( mcht_value_match
+		int ret = mcht_value_match
 			(mctx, str_c(value), str_len(value), str_c(key_item), 
-			str_len(key_item), key_index) )
-            return TRUE;
+				str_len(key_item), key_index);
+        
+		if ( ret > 0 )   
+			return TRUE;
+	
+		if ( ret < 0 )
+			return ret;
 
         key_index++;
     }
diff --git a/src/testsuite/Makefile.am b/src/testsuite/Makefile.am
index dec21d440..b6c135eaa 100644
--- a/src/testsuite/Makefile.am
+++ b/src/testsuite/Makefile.am
@@ -78,6 +78,7 @@ test_cases = \
 	tests/extensions/variables/basic.svtest \
 	tests/extensions/include/variables.svtest \
 	tests/extensions/imapflags/basic.svtest \
+	tests/extensions/imapflags/rfc.svtest \
 	tests/compile/compile.svtest \
 	tests/compile/compile-examples.svtest
 
diff --git a/src/testsuite/tests/extensions/imapflags/basic.svtest b/src/testsuite/tests/extensions/imapflags/basic.svtest
index 2339d59af..3bec20ee9 100644
--- a/src/testsuite/tests/extensions/imapflags/basic.svtest
+++ b/src/testsuite/tests/extensions/imapflags/basic.svtest
@@ -5,12 +5,16 @@ require "relational";
 require "variables";
 require "comparator-i;ascii-numeric";
 
+/*
+ * Basic functionality tests
+ */
+
 test "Hasflag empty" {
 	if hasflag "\\Seen" {
 		test_fail "hasflag sees flags were there should be none";
 	}
 
-	if hasflag :count "ge" "1" {
+	if hasflag :comparator "i;ascii-numeric" :count "ge" "1" {
 		test_fail "hasflag sees flags were there should be none";
 	}
 }
@@ -22,7 +26,7 @@ test "Setflag; Hasflag one" {
 		test_fail "flag not set of hasflag fails to see it";
 	}
 
-	if not hasflag :count "eq" "1" {
+	if not hasflag :comparator "i;ascii-numeric" :count "eq" "1" {
 		test_fail "flag not set of hasflag fails to see it";
 	}
 
@@ -31,62 +35,14 @@ test "Setflag; Hasflag one" {
 	}
 }
 
-/*
- * RFC examples
- */ 
-
-
-test "RFC hasflag example - :is" {
-	setflag "A B";
-
-	if not hasflag ["b","A"] {
-		test_fail "list representation did not match";
-	}
- 
-	if not hasflag :is "b A" {
-		test_fail "string representation did not match";
-	}
-}
-
-test "RFC hasflag example - :contains variable" {
-	set "MyVar" "NonJunk Junk gnus-forward $Forwarded NotJunk JunkRecorded $Junk $NotJunk";
-
-	if not hasflag :contains "MyVar" "Junk" {
-		test_fail "failed true example 1";
-	}
+test "Hasflag; duplicates" {
+	set "Flags" "A B C D E F A B C D E F";
 
-    if not hasflag :contains "MyVar" "forward" {
-		test_fail "failed true example 2";
+	if hasflag :comparator "i;ascii-numeric" :count "gt" "Flags" "6" {
+		test_fail "hasflag ignores duplicates";
 	}
-      
-	if not hasflag :contains "MyVar" ["label", "forward"] {
-		test_fail "failed true example 3";
-	}
-     
-	if not hasflag :contains "MyVar" ["junk", "forward"] {
-		test_fail "failed true example 4";
-	}
-
-	if not hasflag :contains "MyVar" "junk forward" {
-		test_fail "failed true example 4 (rewrite 1)";
-	}
-
-    if not hasflag :contains "MyVar" "forward junk" {
-		test_fail "failed true example 4 (rewrite 2)";
-	}
-
-	if hasflag :contains "MyVar" "label" {
-		test_fail "failed false example 1";
-	}
-
-	if hasflag :contains "MyVar" ["label1", "label2"] {
-		test_fail "failed false example 2";
-	}
-}
 
-test "RFC hasflag example - :count variable" {
-	set "MyFlags" "A B";
-	if not hasflag :count "ge" :comparator "i;ascii-numeric" "MyFlags" "2" {
-		test_fail "failed count \"ge\" comparison";
+	if not hasflag :comparator "i;ascii-numeric" :count "eq" "Flags" "6" {
+		test_fail "hasflag :count gives strange results";
 	}
 }
-- 
GitLab