dovecot-2.0-pigeonhole: Fixed assertion failure in the keep/file...

pigeonhole at rename-it.nl pigeonhole at rename-it.nl
Thu Jul 8 01:09:05 EEST 2010


details:   http://hg.rename-it.nl/dovecot-2.0-pigeonhole/rev/703f82bb2b09
changeset: 1320:703f82bb2b09
user:      Stephan Bosch <stephan at rename-it.nl>
date:      Thu Jul 08 00:06:42 2010 +0200
description:
Fixed assertion failure in the keep/fileinto store actions and added testsuite item.

diffstat:

 src/lib-sieve/plugins/mailbox/tag-mailbox-create.c |   7 ++-
 src/lib-sieve/sieve-actions.c                      |  64 +++++++++-----------
 tests/execute/errors.svtest                        |  24 ++++++++
 tests/execute/errors/fileinto.sieve                |   3 +
 4 files changed, 62 insertions(+), 36 deletions(-)

diffs (214 lines):

diff -r e6c049bf72a9 -r 703f82bb2b09 src/lib-sieve/plugins/mailbox/tag-mailbox-create.c
--- a/src/lib-sieve/plugins/mailbox/tag-mailbox-create.c	Mon Jul 05 17:18:23 2010 +0200
+++ b/src/lib-sieve/plugins/mailbox/tag-mailbox-create.c	Thu Jul 08 00:06:42 2010 +0200
@@ -125,7 +125,7 @@
 	enum mail_error error;
 	
 	/* Check whether creation is necessary */
-	if ( trans->box == NULL || trans->redundant || trans->disabled )
+	if ( trans->box == NULL || trans->disabled )
 		return TRUE;
 
 	/* Check whether creation has a chance of working */
@@ -133,6 +133,10 @@
 		trans->error_code != MAIL_ERROR_NOTFOUND )
 		return FALSE;
 
+	trans->error = NULL;
+	trans->error_code = MAIL_ERROR_NONE;
+	
+
 	*storage = mailbox_get_storage(trans->box);
 
 	/* Create mailbox */
@@ -143,6 +147,7 @@
 			return FALSE;
 		}
 	}
+
 	/* Subscribe to it if necessary */
 	if ( aenv->scriptenv->mailbox_autosubscribe ) {
 		(void)mailbox_list_set_subscribed
diff -r e6c049bf72a9 -r 703f82bb2b09 src/lib-sieve/sieve-actions.c
--- a/src/lib-sieve/sieve-actions.c	Mon Jul 05 17:18:23 2010 +0200
+++ b/src/lib-sieve/sieve-actions.c	Thu Jul 08 00:06:42 2010 +0200
@@ -331,8 +331,8 @@
 /* Action implementation */
 
 static bool act_store_mailbox_open
-(const struct sieve_action_exec_env *aenv, const char **mailbox,
- struct mailbox **box_r, const char **error_r)
+(const struct sieve_action_exec_env *aenv, const char *mailbox,
+	struct mailbox **box_r, const char **error_r)
 {
 	struct mail_storage **storage = &(aenv->exec_status->last_storage);
 	struct mail_deliver_save_open_context save_ctx;
@@ -340,10 +340,9 @@
 
 	*box_r = NULL;
 
-	if ( !uni_utf8_str_is_valid(*mailbox) ) {
+	if ( !uni_utf8_str_is_valid(mailbox) ) {
 		/* FIXME: check utf-8 validity at compiletime/runtime */
-		*error_r = t_strdup_printf("mailbox name not utf-8: %s",
-					   *mailbox);
+		*error_r = t_strdup_printf("mailbox name not utf-8: %s", mailbox);
 		return FALSE;
 	}
 
@@ -352,15 +351,11 @@
 	save_ctx.lda_mailbox_autocreate = aenv->scriptenv->mailbox_autocreate;
 	save_ctx.lda_mailbox_autosubscribe = aenv->scriptenv->mailbox_autosubscribe;
 
-	if (mail_deliver_save_open(&save_ctx, *mailbox, box_r, &error) < 0) {
-		*error_r = t_strdup_printf("failed to save mail to mailbox '%s': %s",
-					   *mailbox, error);
+	if (mail_deliver_save_open(&save_ctx, mailbox, box_r, &error) < 0) {
+		*error_r = error;
 		return FALSE;
 	}
 
-	/* FIXME: is box freed too early for this? is it even useful to update
-	   this name now that box is set? */
-	*mailbox = mailbox_get_vname(*box_r);
 	*storage = mailbox_get_storage(*box_r);
 	return TRUE;
 }
@@ -371,12 +366,11 @@
 {  
 	struct act_store_context *ctx = (struct act_store_context *) action->context;
 	const struct sieve_script_env *senv = aenv->scriptenv;
-	const struct sieve_message_data *msgdata = aenv->msgdata;
 	struct act_store_transaction *trans;
 	struct mailbox *box = NULL;
 	pool_t pool = sieve_result_pool(aenv->result);
 	const char *error = NULL;
-	bool disabled = FALSE, redundant = FALSE, open_failed = FALSE;
+	bool disabled = FALSE, open_failed = FALSE;
 
 	/* If context is NULL, the store action is the result of (implicit) keep */	
 	if ( ctx == NULL ) {
@@ -390,16 +384,8 @@
 	 * to NULL. This implementation will then skip actually storing the message.
 	 */
 	if ( senv->user != NULL ) {
-		if ( !act_store_mailbox_open(aenv, &ctx->mailbox, &box, &error) ) {
+		if ( !act_store_mailbox_open(aenv, ctx->mailbox, &box, &error) ) {
 			open_failed = TRUE;
-
-		/* Check whether we are trying to store the message in the folder it
-		 * originates from. In that case we skip actually storing it.
-		 */
-		} else if ( mailbox_backends_equal(box, msgdata->mail->box) ) {
-			mailbox_free(&box);
-			box = NULL;
-			redundant = TRUE;
 		}
 	} else {
 		disabled = TRUE;
@@ -413,11 +399,14 @@
 	trans->flags = 0;
 
 	trans->disabled = disabled;
-	trans->redundant = redundant;
-	trans->error_code = MAIL_ERROR_NONE;
 
-	if ( open_failed && error != NULL )
-		trans->error = p_strdup(sieve_result_pool(aenv->result), error);
+	if ( open_failed  ) {
+		trans->error = error;
+		(void)mail_storage_get_last_error
+			(mailbox_get_storage(trans->box), &trans->error_code);
+	} else {
+		trans->error_code = MAIL_ERROR_NONE;
+	}
 
 	*tr_context = (void *)trans;
 
@@ -464,10 +453,16 @@
 	/* Check whether we need to do anything */
 	if ( trans->disabled ) return TRUE;
 
+	/* Exit early if mailbox is not available */
+	if ( trans->box == NULL || trans->error_code != MAIL_ERROR_NONE ) 
+		return FALSE;
+
 	/* If the message originates from the target mailbox, only update the flags 
 	 * and keywords 
 	 */
-	if ( trans->redundant ) {
+	if ( mailbox_backends_equal(trans->box, msgdata->mail->box) ) {
+		trans->redundant = TRUE;
+
 		if ( trans->flags_altered ) {
 			keywords = act_store_keywords_create
 				(aenv, &trans->keywords, msgdata->mail->box);
@@ -483,10 +478,6 @@
 		return TRUE;
 	}
 
-	/* Exit early if mailbox is not available */
-	if ( trans->box == NULL ) 
-		return FALSE;
-
 	/* Mark attempt to store in default mailbox */
 	if ( strcmp(trans->context->mailbox, 
 		SIEVE_SCRIPT_DEFAULT_MAILBOX(aenv->scriptenv)) == 0 ) 
@@ -529,12 +520,15 @@
 }
 
 static void act_store_log_status
-(struct act_store_transaction *trans, 
-	const struct sieve_action_exec_env *aenv, bool rolled_back, bool status )
+(struct act_store_transaction *trans, const struct sieve_action_exec_env *aenv,
+	bool rolled_back, bool status )
 {
 	const char *mailbox_name;
-	
-	mailbox_name = str_sanitize(trans->context->mailbox, 128);
+
+	if ( trans->box != NULL )
+		mailbox_name = str_sanitize(mailbox_get_vname(trans->box), 128);
+	else
+		mailbox_name = str_sanitize(trans->context->mailbox, 128);
 
 	/* Store disabled? */
 	if ( trans->disabled ) {
diff -r e6c049bf72a9 -r 703f82bb2b09 tests/execute/errors.svtest
--- a/tests/execute/errors.svtest	Mon Jul 05 17:18:23 2010 +0200
+++ b/tests/execute/errors.svtest	Thu Jul 08 00:06:42 2010 +0200
@@ -80,3 +80,27 @@
 		test_fail "unexpected error reported";
 	}
 }
+
+test "Fileinto missing folder" {
+	if not test_script_compile "errors/fileinto.sieve" {
+		test_fail "compile failed";
+	}
+
+	test_mailbox :create "INBOX";
+
+	if not test_script_run {
+		test_fail "execution failed";
+	}
+
+    if test_result_execute {
+        test_fail "execution of result should have failed";
+    }	
+
+	if test_error :count "gt" :comparator "i;ascii-numeric" "1" {
+		test_fail "too many runtime errors reported";
+	}
+	
+/*	if not test_error :index 1 :contains "number of redirect actions exceeds policy limit"{
+		test_fail "unexpected error reported";
+	}*/
+}
diff -r e6c049bf72a9 -r 703f82bb2b09 tests/execute/errors/fileinto.sieve
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/execute/errors/fileinto.sieve	Thu Jul 08 00:06:42 2010 +0200
@@ -0,0 +1,3 @@
+require "fileinto";
+
+fileinto "FROP";


More information about the dovecot-cvs mailing list