From ea223592b4a0a00f493b2e09c9878f1931a9fdaa Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Sun, 3 Aug 2008 23:15:31 +0200
Subject: [PATCH] Fixed replacing match values only when a test succeeds.

---
 TODO                                          |   9 +-
 src/lib-sieve/mcht-matches.c                  |   3 +-
 .../plugins/include/ext-include-common.c      |   2 -
 src/lib-sieve/plugins/regex/mcht-regex.c      |  14 +--
 src/lib-sieve/sieve-match-types.c             | 102 ++++++++++++++----
 src/lib-sieve/sieve-match-types.h             |  10 ++
 tests/extensions/variables/match.svtest       |  31 +++++-
 tests/match-types/matches.svtest              |   7 +-
 8 files changed, 138 insertions(+), 40 deletions(-)

diff --git a/TODO b/TODO
index 0f52f825f..8a5be9c7d 100644
--- a/TODO
+++ b/TODO
@@ -7,12 +7,8 @@ Next (in order of descending priority/precedence):
 	- Imapflags: when keep/fileinto is used multiple times in a script and
 	  duplicate message elimination is performed, the last flag list value
 	  MUST win.
-	- :matches : match values must only be changed when the test matches.
 	- If an address is not syntactically valid, then it will not be matched
 	  by tests specifying ":localpart" or ":domain".
-	- Allow for the existance of dynamic comparators 
-		(i.e. specified by variables). 
-	- Fix/Report issues listed in 'doc/rfc/RFC Controversy.txt'
 * Code cleanup 
 * Full security review. Enforce limits on number of created objects, script 
   size, execution time, etc...
@@ -29,6 +25,11 @@ Next (in order of descending priority/precedence):
 
 * ## MAKE A FIRST RELEASE (0.1.x) ##
 
+* Fix remaining RFC deviations:
+	- Allow for the existance of dynamic comparators (i.e. specified by 
+	  variables). 
+	- Allow for dynamic includes (i.e. specified by variables).
+	- Fix/Report issues listed in 'doc/rfc/RFC Controversy.txt'
 * Imapflags: merge execution of setflags, removeflags and addflags into one 
   common implementation. 
 * Verify outgoing mail addresses at runtime when necessary (e.g. after variables 
diff --git a/src/lib-sieve/mcht-matches.c b/src/lib-sieve/mcht-matches.c
index aeaae9510..aa070174e 100644
--- a/src/lib-sieve/mcht-matches.c
+++ b/src/lib-sieve/mcht-matches.c
@@ -336,10 +336,11 @@ static int mcht_matches_match
 	if (kp == kend && vp == vend) {
 		string_t *matched = str_new_const(pool_datastack_create(), val, val_size);
 		sieve_match_values_set(mvalues, 0, matched);
-
+		sieve_match_values_commit(mctx->interp, &mvalues);
 		return TRUE;
 	}
 
+	sieve_match_values_abort(&mvalues);
 	return FALSE;
 }
 			 
diff --git a/src/lib-sieve/plugins/include/ext-include-common.c b/src/lib-sieve/plugins/include/ext-include-common.c
index cee2790ea..bab9ffec9 100644
--- a/src/lib-sieve/plugins/include/ext-include-common.c
+++ b/src/lib-sieve/plugins/include/ext-include-common.c
@@ -74,8 +74,6 @@ const char *ext_include_get_script_directory
 		return NULL;
 	}
 
-	printf("SIEVE DIR: %s\n", sieve_dir);
-
 	return sieve_dir;
 }
 
diff --git a/src/lib-sieve/plugins/regex/mcht-regex.c b/src/lib-sieve/plugins/regex/mcht-regex.c
index dc4c3ec11..bfd98e4df 100644
--- a/src/lib-sieve/plugins/regex/mcht-regex.c
+++ b/src/lib-sieve/plugins/regex/mcht-regex.c
@@ -161,9 +161,9 @@ bool mcht_regex_validate_context
 struct mcht_regex_context {
 	ARRAY_DEFINE(reg_expressions, regex_t *);
 	int value_index;
-	struct sieve_match_values *mvalues;
 	regmatch_t *pmatch;
 	size_t nmatch;
+	bool with_match_values;
 };
 
 static void mcht_regex_match_init
@@ -175,8 +175,8 @@ static void mcht_regex_match_init
 	t_array_init(&ctx->reg_expressions, 4);
 
 	ctx->value_index = -1;
-	ctx->mvalues = sieve_match_values_start(mctx->interp);
-	if ( ctx->mvalues != NULL ) {
+	ctx->with_match_values = sieve_match_values_are_enabled(mctx->interp);
+	if ( ctx->with_match_values ) {
 		ctx->pmatch = t_new(regmatch_t, MCHT_REGEX_MAX_SUBSTITUTIONS);
 		ctx->nmatch = MCHT_REGEX_MAX_SUBSTITUTIONS;
 	} else {
@@ -207,7 +207,7 @@ static regex_t *mcht_regex_get
 		else
 			return NULL;
 			
-		if ( ctx->mvalues == NULL ) cflags |= REG_NOSUB;
+		if ( !ctx->with_match_values ) cflags |= REG_NOSUB;
 
 		if ( (ret=regcomp(regexp, key, cflags)) != 0 ) {
 			/* FIXME: Do something useful, i.e. report error somewhere */
@@ -246,20 +246,22 @@ static int mcht_regex_match
 		size_t i;
 		int skipped = 0;
 		string_t *subst = t_str_new(32);
+		struct sieve_match_values *mvalues = sieve_match_values_start(mctx->interp);
 		
 		for ( i = 0; i < ctx->nmatch; i++ ) {
 			str_truncate(subst, 0);
 			
 			if ( ctx->pmatch[i].rm_so != -1 ) {
 				if ( skipped > 0 )
-					sieve_match_values_skip(ctx->mvalues, skipped);
+					sieve_match_values_skip(mvalues, skipped);
 					
 				str_append_n(subst, val + ctx->pmatch[i].rm_so, 
 					ctx->pmatch[i].rm_eo - ctx->pmatch[i].rm_so);
-				sieve_match_values_add(ctx->mvalues, subst);
+				sieve_match_values_add(mvalues, subst);
 			} else 
 				skipped++;
 		}
+		sieve_match_values_commit(mctx->interp, &mvalues);
 		return TRUE;
 	}
 	
diff --git a/src/lib-sieve/sieve-match-types.c b/src/lib-sieve/sieve-match-types.c
index 7e64310aa..731ac411d 100644
--- a/src/lib-sieve/sieve-match-types.c
+++ b/src/lib-sieve/sieve-match-types.c
@@ -20,6 +20,16 @@
 
 #include <string.h>
 
+/*
+ * Types
+ */
+ 
+struct sieve_match_values {
+	pool_t pool;
+	ARRAY_DEFINE(values, string_t *);
+	unsigned count;
+};
+
 /* 
  * Default match types
  */ 
@@ -107,6 +117,22 @@ struct mtch_interpreter_context {
 	bool match_values_enabled;
 };
 
+static void mtch_interpreter_free
+(struct sieve_interpreter *interp ATTR_UNUSED, void *context)
+{
+	struct mtch_interpreter_context *mctx = 
+		(struct mtch_interpreter_context *) context;
+	
+	if ( mctx->match_values != NULL ) {
+		pool_unref(&mctx->match_values->pool);
+	}
+}
+
+struct sieve_interpreter_extension mtch_interpreter_extension = {
+	&match_type_extension,
+	mtch_interpreter_free
+};
+
 static inline struct mtch_interpreter_context *
 get_interpreter_context(struct sieve_interpreter *interp)
 {
@@ -122,8 +148,8 @@ mtch_interpreter_context_init(struct sieve_interpreter *interp)
 	
 	ctx = p_new(pool, struct mtch_interpreter_context, 1);
 
-	sieve_interpreter_extension_set_context
-		(interp, ext_this, (void *) ctx);
+	sieve_interpreter_extension_register
+		(interp, &mtch_interpreter_extension, (void *) ctx);
 
 	return ctx;
 }
@@ -131,12 +157,6 @@ mtch_interpreter_context_init(struct sieve_interpreter *interp)
 /*
  * Match values
  */
- 
-struct sieve_match_values {
-	pool_t pool;
-	ARRAY_DEFINE(values, string_t *);
-	unsigned count;
-};
 
 bool sieve_match_values_set_enabled
 (struct sieve_interpreter *interp, bool enable)
@@ -153,29 +173,36 @@ bool sieve_match_values_set_enabled
 	return previous;
 }
 
+bool sieve_match_values_are_enabled
+(struct sieve_interpreter *interp)
+{
+	struct mtch_interpreter_context *ctx = get_interpreter_context(interp);
+		
+	return ctx->match_values_enabled;
+}
+
 struct sieve_match_values *sieve_match_values_start
 (struct sieve_interpreter *interp)
 {
 	struct mtch_interpreter_context *ctx = get_interpreter_context(interp);
+	struct sieve_match_values *match_values;
 	
 	if ( ctx == NULL || !ctx->match_values_enabled )
 		return NULL;
-		
-	if ( ctx->match_values == NULL ) {
-		pool_t pool = sieve_interpreter_pool(interp);
-		
-		ctx->match_values = p_new(pool, struct sieve_match_values, 1);
-		ctx->match_values->pool = pool;
-		p_array_init(&ctx->match_values->values, pool, 4);
-	}
 	
-	ctx->match_values->count = 0;
+	pool_t pool = pool_alloconly_create("sieve_match_values", 1024);
+		
+	match_values = p_new(pool, struct sieve_match_values, 1);
+	match_values->pool = pool;
+	match_values->count = 0;
 	
-	return ctx->match_values;
+	p_array_init(&match_values->values, pool, 4);
+
+	return match_values;
 }
 
 static string_t *sieve_match_values_add_entry
-	(struct sieve_match_values *mvalues) 
+(struct sieve_match_values *mvalues) 
 {
 	string_t *entry;
 	
@@ -210,7 +237,7 @@ void sieve_match_values_set
 }
 	
 void sieve_match_values_add
-	(struct sieve_match_values *mvalues, string_t *value) 
+(struct sieve_match_values *mvalues, string_t *value) 
 {
 	string_t *entry = sieve_match_values_add_entry(mvalues); 
 
@@ -219,7 +246,7 @@ void sieve_match_values_add
 }
 
 void sieve_match_values_add_char
-	(struct sieve_match_values *mvalues, char c) 
+(struct sieve_match_values *mvalues, char c) 
 {
 	string_t *entry = sieve_match_values_add_entry(mvalues); 
 
@@ -228,7 +255,7 @@ void sieve_match_values_add_char
 }
 
 void sieve_match_values_skip
-	(struct sieve_match_values *mvalues, int num) 
+(struct sieve_match_values *mvalues, int num) 
 {
 	int i;
 	
@@ -236,8 +263,37 @@ void sieve_match_values_skip
 		(void) sieve_match_values_add_entry(mvalues); 
 }
 
+void sieve_match_values_commit
+(struct sieve_interpreter *interp, struct sieve_match_values **mvalues)
+{
+	struct mtch_interpreter_context *ctx;
+	
+	if ( (*mvalues) == NULL ) return;
+	
+	ctx = get_interpreter_context(interp);
+	if ( ctx == NULL || !ctx->match_values_enabled )
+		return;	
+		
+	if ( ctx->match_values != NULL ) {
+		pool_unref(&ctx->match_values->pool);
+		ctx->match_values = NULL;
+	}
+
+	ctx->match_values = *mvalues;
+	*mvalues = NULL;
+}
+
+void sieve_match_values_abort
+(struct sieve_match_values **mvalues)
+{		
+	if ( (*mvalues) == NULL ) return;
+	
+	pool_unref(&(*mvalues)->pool);
+	*mvalues = NULL;
+}
+
 void sieve_match_values_get
-	(struct sieve_interpreter *interp, unsigned int index, string_t **value_r) 
+(struct sieve_interpreter *interp, unsigned int index, string_t **value_r) 
 {
 	struct mtch_interpreter_context *ctx = get_interpreter_context(interp);
 	struct sieve_match_values *mvalues;
diff --git a/src/lib-sieve/sieve-match-types.h b/src/lib-sieve/sieve-match-types.h
index 309d8bd25..87da2a3ed 100644
--- a/src/lib-sieve/sieve-match-types.h
+++ b/src/lib-sieve/sieve-match-types.h
@@ -79,6 +79,9 @@ struct sieve_match_values;
 
 bool sieve_match_values_set_enabled
 	(struct sieve_interpreter *interp, bool enable);
+bool sieve_match_values_are_enabled
+	(struct sieve_interpreter *interp);	
+	
 struct sieve_match_values *sieve_match_values_start
 	(struct sieve_interpreter *interp);
 void sieve_match_values_set
@@ -89,6 +92,13 @@ void sieve_match_values_add_char
 	(struct sieve_match_values *mvalues, char c);	
 void sieve_match_values_skip
 	(struct sieve_match_values *mvalues, int num);
+	
+void sieve_match_values_commit
+	(struct sieve_interpreter *interp, struct sieve_match_values **mvalues);
+void sieve_match_values_abort
+	(struct sieve_match_values **mvalues);
+	
+	
 void sieve_match_values_get
 	(struct sieve_interpreter *interp, unsigned int index, string_t **value_r);
 
diff --git a/tests/extensions/variables/match.svtest b/tests/extensions/variables/match.svtest
index 6871924a8..5395af3a6 100644
--- a/tests/extensions/variables/match.svtest
+++ b/tests/extensions/variables/match.svtest
@@ -2,7 +2,6 @@ require "vnd.dovecot.testsuite";
 
 require "variables";
 
-
 /*
  * RFC compliance
  */
@@ -70,6 +69,36 @@ test "RFC - test short-circuit" {
 	}
 }
 
+# Test overwriting only on match 
+test "RFC - values overwrite" {
+	set "sentence1" "the cat jumps off the table";
+	set "sentence2" "the dog barks at the cat in the alley";
+
+	if not string :matches "${sentence1}" "the * jumps off the *" {
+		test_fail "failed to match first sentence";
+	} 
+	
+	if not string :is "${1}:${2}" "cat:table" {
+		test_fail "invalid match values";
+	}
+
+	if string :matches "${sentence2}" "the * barks at the * in the store" {
+		test_fail "should not have matched second sentence";
+	}
+
+	if not string :is "${1}:${2}" "cat:table" {
+		test_fail "should have preserved match values";
+	}
+
+	if not string :matches "${sentence2}" "the * barks at the * in the alley" {
+		test_fail "failed to match the second sentence (second time)";
+	}
+
+	if not string :is "${1}:${2}" "dog:cat" {
+		test_fail "should have overwritten match values";
+	}
+}
+
 test "RFC - example" {
 	test_set "message" text:
 Subject: [acme-users] [fwd] version 1.0 is out
diff --git a/tests/match-types/matches.svtest b/tests/match-types/matches.svtest
index 99a2c06c3..88c6f9636 100644
--- a/tests/match-types/matches.svtest
+++ b/tests/match-types/matches.svtest
@@ -18,10 +18,9 @@ Het werkt!
 ;
 
 /*
- * Matching tests
+ * General conformance testing
  */
 
-# Empty string
 test "Empty string" {
 	if not header :matches "comment" "" {
 		test_fail "failed to match \"\" against \"\"";
@@ -36,7 +35,9 @@ test "Empty string" {
 	}
 }
 
-stop;
+/*
+ * Matching tests
+ */
 
 test "MATCH-A" {
 	if not address :matches "from" "*@d*ksn*ers.com" {
-- 
GitLab