From ac77b22b73928e6b4c524e5d4a05dfd68d7b8894 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Fri, 25 Mar 2016 01:47:49 +0100
Subject: [PATCH] lib-sieve: Improved handling of extension conflicts.

Conflicts are now always tested from both sides. This is mainly important for the "ihave" test.
---
 .../plugins/duplicate/ext-duplicate.c         | 44 +++++++------
 .../plugins/imap4flags/ext-imapflags.c        | 28 +++++----
 src/lib-sieve/plugins/mime/ext-extracttext.c  | 24 +++----
 src/lib-sieve/plugins/notify/ext-notify.c     | 38 ++++++-----
 .../plugins/spamvirustest/ext-spamvirustest.c | 31 +++++----
 src/lib-sieve/sieve-types.h                   |  2 +-
 src/lib-sieve/sieve-validator.c               | 63 ++++++++++++++++---
 src/lib-sieve/sieve-validator.h               | 14 +++--
 src/plugins/sieve-extprograms/ext-pipe.c      | 23 +++----
 9 files changed, 162 insertions(+), 105 deletions(-)

diff --git a/src/lib-sieve/plugins/duplicate/ext-duplicate.c b/src/lib-sieve/plugins/duplicate/ext-duplicate.c
index 98ef24f15..2fac5ac4d 100644
--- a/src/lib-sieve/plugins/duplicate/ext-duplicate.c
+++ b/src/lib-sieve/plugins/duplicate/ext-duplicate.c
@@ -58,14 +58,16 @@ const struct sieve_extension_def vnd_duplicate_extension = {
  * Validation
  */
 
-static bool ext_duplicate_validator_extension_validate
-	(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-		void *context, struct sieve_ast_argument *require_arg);
-
-const struct sieve_validator_extension duplicate_validator_extension = {
-	&vnd_duplicate_extension,
-	ext_duplicate_validator_extension_validate,
-	NULL
+static bool ext_duplicate_validator_check_conflict
+	(const struct sieve_extension *ext,
+		struct sieve_validator *valdtr, void *context,
+		struct sieve_ast_argument *require_arg,
+		const struct sieve_extension *ext_other);
+
+const struct sieve_validator_extension
+duplicate_validator_extension = {
+	.ext = &vnd_duplicate_extension,
+	.check_conflict = ext_duplicate_validator_check_conflict
 };
 
 static bool ext_duplicate_validator_load
@@ -84,22 +86,18 @@ static bool ext_duplicate_validator_load
 	return TRUE;
 }
 
-static bool ext_duplicate_validator_extension_validate
-(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-	void *context ATTR_UNUSED, struct sieve_ast_argument *require_arg)
+static bool ext_duplicate_validator_check_conflict
+(const struct sieve_extension *ext ATTR_UNUSED,
+	struct sieve_validator *valdtr, void *context ATTR_UNUSED,
+	struct sieve_ast_argument *require_arg,
+	const struct sieve_extension *ext_other)
 {
-	const struct sieve_extension *ext_dupl;
-
-	if ( (ext_dupl=sieve_extension_get_by_name
-		(ext->svinst, "duplicate")) != NULL ) {
-
-		/* Check for conflict with duplicate extension */
-		if ( sieve_validator_extension_loaded(valdtr, ext_dupl) ) {
-			sieve_argument_validate_error(valdtr, require_arg,
-				"the (deprecated) vnd.dovecot.duplicate extension cannot be used "
-				"together with the duplicate extension");
-			return FALSE;
-		}
+	/* Check for conflict with duplicate extension */
+	if ( sieve_extension_name_is(ext_other, "duplicate") ) {
+		sieve_argument_validate_error(valdtr, require_arg,
+			"the (deprecated) vnd.dovecot.duplicate extension "
+			"cannot be used together with the duplicate extension");
+		return FALSE;
 	}
 
 	return TRUE;
diff --git a/src/lib-sieve/plugins/imap4flags/ext-imapflags.c b/src/lib-sieve/plugins/imap4flags/ext-imapflags.c
index 1d1368715..18b8b11fc 100644
--- a/src/lib-sieve/plugins/imap4flags/ext-imapflags.c
+++ b/src/lib-sieve/plugins/imap4flags/ext-imapflags.c
@@ -101,14 +101,16 @@ static bool ext_imapflags_load
  * Validator
  */
 
-static bool ext_imapflags_validator_extension_validate
-	(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-		void *context, struct sieve_ast_argument *require_arg);
-
-const struct sieve_validator_extension imapflags_validator_extension = {
-	&imapflags_extension,
-	ext_imapflags_validator_extension_validate,
-	NULL
+static bool ext_imapflags_validator_check_conflict
+	(const struct sieve_extension *ext,
+		struct sieve_validator *valdtr, void *context,
+		struct sieve_ast_argument *require_arg,
+		const struct sieve_extension *other_ext);
+
+const struct sieve_validator_extension
+imapflags_validator_extension = {
+	.ext = &imapflags_extension,
+	.check_conflict = ext_imapflags_validator_check_conflict
 };
 
 static bool ext_imapflags_validator_load
@@ -135,14 +137,16 @@ static bool ext_imapflags_validator_load
 	return TRUE;
 }
 
-static bool ext_imapflags_validator_extension_validate
-(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-	void *context ATTR_UNUSED, struct sieve_ast_argument *require_arg)
+static bool ext_imapflags_validator_check_conflict
+(const struct sieve_extension *ext,
+	struct sieve_validator *valdtr, void *context ATTR_UNUSED,
+	struct sieve_ast_argument *require_arg,
+	const struct sieve_extension *ext_other)
 {
 	const struct sieve_extension *master_ext =
 		(const struct sieve_extension *) ext->context;
 
-	if ( sieve_validator_extension_loaded(valdtr, master_ext) ) {
+	if ( ext_other == master_ext ) {
 		sieve_argument_validate_error(valdtr, require_arg,
 			"the (deprecated) imapflags extension cannot be used "
 			"together with the imap4flags extension");
diff --git a/src/lib-sieve/plugins/mime/ext-extracttext.c b/src/lib-sieve/plugins/mime/ext-extracttext.c
index defd7c819..7f30c15f8 100644
--- a/src/lib-sieve/plugins/mime/ext-extracttext.c
+++ b/src/lib-sieve/plugins/mime/ext-extracttext.c
@@ -75,14 +75,15 @@ static void ext_extracttext_unload
  * Extension validation
  */
 
-static bool ext_extracttext_validator_extension_validate
-	(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-		void *context, struct sieve_ast_argument *require_arg);
-
-const struct sieve_validator_extension extracttext_validator_extension = {
-	&extracttext_extension,
-	ext_extracttext_validator_extension_validate,
-	NULL
+static bool ext_extracttext_validator_validate
+	(const struct sieve_extension *ext,
+		struct sieve_validator *valdtr, void *context,
+		struct sieve_ast_argument *require_arg);
+
+const struct sieve_validator_extension
+extracttext_validator_extension = {
+	.ext = &extracttext_extension,
+	.validate = ext_extracttext_validator_validate
 };
 
 static bool ext_extracttext_validator_load
@@ -98,9 +99,10 @@ static bool ext_extracttext_validator_load
 	return TRUE;
 }
 
-static bool ext_extracttext_validator_extension_validate
-(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-	void *context ATTR_UNUSED, struct sieve_ast_argument *require_arg)
+static bool ext_extracttext_validator_validate
+(const struct sieve_extension *ext,
+	struct sieve_validator *valdtr, void *context ATTR_UNUSED,
+	struct sieve_ast_argument *require_arg)
 {
 	struct ext_extracttext_context *ectx =
 		(struct ext_extracttext_context *)ext->context;
diff --git a/src/lib-sieve/plugins/notify/ext-notify.c b/src/lib-sieve/plugins/notify/ext-notify.c
index 7ab6b9e8e..4fa5d948b 100644
--- a/src/lib-sieve/plugins/notify/ext-notify.c
+++ b/src/lib-sieve/plugins/notify/ext-notify.c
@@ -50,14 +50,15 @@ const struct sieve_extension_def notify_extension = {
  * Extension validation
  */
 
-static bool ext_notify_validator_extension_validate
-	(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-		void *context, struct sieve_ast_argument *require_arg);
+static bool ext_notify_validator_check_conflict
+	(const struct sieve_extension *ext,
+		struct sieve_validator *valdtr, void *context,
+		struct sieve_ast_argument *require_arg,
+		const struct sieve_extension *ext_other);
 
 const struct sieve_validator_extension notify_validator_extension = {
-	&notify_extension,
-	ext_notify_validator_extension_validate,
-	NULL
+	.ext = &notify_extension,
+	.check_conflict = ext_notify_validator_check_conflict
 };
 
 static bool ext_notify_validator_load
@@ -74,21 +75,18 @@ static bool ext_notify_validator_load
 	return TRUE;
 }
 
-static bool ext_notify_validator_extension_validate
-(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-	void *context ATTR_UNUSED, struct sieve_ast_argument *require_arg)
+static bool ext_notify_validator_check_conflict
+(const struct sieve_extension *ext ATTR_UNUSED,
+	struct sieve_validator *valdtr, void *context ATTR_UNUSED,
+	struct sieve_ast_argument *require_arg,
+	const struct sieve_extension *ext_other)
 {
-	const struct sieve_extension *ext_entfy;
-
-	if ( (ext_entfy=sieve_extension_get_by_name(ext->svinst, "enotify")) != NULL ) {
-
-		/* Check for conflict with enotify */
-		if ( sieve_validator_extension_loaded(valdtr, ext_entfy) ) {
-			sieve_argument_validate_error(valdtr, require_arg,
-				"the (deprecated) notify extension cannot be used "
-				"together with the enotify extension");
-			return FALSE;
-		}
+	/* Check for conflict with enotify */
+	if ( sieve_extension_name_is(ext_other, "enotify") ) {
+		sieve_argument_validate_error(valdtr, require_arg,
+			"the (deprecated) notify extension cannot be used "
+			"together with the enotify extension");
+		return FALSE;
 	}
 
 	return TRUE;
diff --git a/src/lib-sieve/plugins/spamvirustest/ext-spamvirustest.c b/src/lib-sieve/plugins/spamvirustest/ext-spamvirustest.c
index c4f63c69d..148af056a 100644
--- a/src/lib-sieve/plugins/spamvirustest/ext-spamvirustest.c
+++ b/src/lib-sieve/plugins/spamvirustest/ext-spamvirustest.c
@@ -95,14 +95,15 @@ const struct sieve_extension_def virustest_extension = {
  * Implementation
  */
 
-static bool ext_spamtest_validator_extension_validate
-	(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-		void *context, struct sieve_ast_argument *require_arg);
+static bool ext_spamtest_validator_check_conflict
+	(const struct sieve_extension *ext,
+		struct sieve_validator *valdtr, void *context,
+		struct sieve_ast_argument *require_arg,
+		const struct sieve_extension *ext_other);
 
 const struct sieve_validator_extension spamtest_validator_extension = {
-	&spamtest_extension,
-	ext_spamtest_validator_extension_validate,
-	NULL
+	.ext = &spamtest_extension,
+	.check_conflict = ext_spamtest_validator_check_conflict
 };
 
 static bool ext_spamvirustest_validator_load
@@ -125,18 +126,16 @@ static bool ext_spamvirustest_validator_load
 	return TRUE;
 }
 
-static bool ext_spamtest_validator_extension_validate
-(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-	void *context ATTR_UNUSED, struct sieve_ast_argument *require_arg)
+static bool ext_spamtest_validator_check_conflict
+(const struct sieve_extension *ext ATTR_UNUSED,
+	struct sieve_validator *valdtr, void *context ATTR_UNUSED,
+	struct sieve_ast_argument *require_arg,
+	const struct sieve_extension *ext_other)
 {
-	const struct sieve_extension *ext_spamtestplus =
-		sieve_extension_get_by_name(ext->svinst, "spamtestplus");
-
-	if ( ext_spamtestplus != NULL &&
-		sieve_validator_extension_loaded(valdtr, ext_spamtestplus) ) {
+	if ( sieve_extension_name_is(ext_other, "spamtestplus") ) {
 		sieve_argument_validate_warning(valdtr, require_arg,
-			"the spamtest and spamtestplus extensions should not be specified "
-			"at the same time");
+			"the spamtest and spamtestplus extensions should "
+			"not be specified at the same time");
 	}
 
 	return TRUE;
diff --git a/src/lib-sieve/sieve-types.h b/src/lib-sieve/sieve-types.h
index ade4c5d24..08985bd45 100644
--- a/src/lib-sieve/sieve-types.h
+++ b/src/lib-sieve/sieve-types.h
@@ -28,7 +28,7 @@ struct sieve_exec_status;
 
 enum sieve_flag {
 	/* Relative paths are resolved to HOME */
-	SIEVE_FLAG_HOME_RELATIVE = (1 << 0),
+	SIEVE_FLAG_HOME_RELATIVE = (1 << 0)
 };
 
 /* Sieve evaluation can be performed at various different points as messages
diff --git a/src/lib-sieve/sieve-validator.c b/src/lib-sieve/sieve-validator.c
index a013975f8..21a4deb34 100644
--- a/src/lib-sieve/sieve-validator.c
+++ b/src/lib-sieve/sieve-validator.c
@@ -550,12 +550,50 @@ static struct sieve_tag_registration *sieve_validator_command_tag_get
  * Extension support
  */
 
+static bool sieve_validator_extensions_check_conficts
+(struct sieve_validator *valdtr,
+	struct sieve_ast_argument *ext_arg,
+	const struct sieve_extension *ext)
+{
+	struct sieve_validator_extension_reg *ext_reg = NULL;
+	struct sieve_validator_extension_reg *regs;
+	unsigned int count, i;
+
+	if ( ext->id >= 0 ) {
+		ext_reg = array_idx_modifiable
+			(&valdtr->extensions, (unsigned int) ext->id);
+	}
+
+	regs = array_get_modifiable(&valdtr->extensions, &count);
+	for ( i = 0; i < count; i++ ) {
+		if (regs[i].ext == NULL)
+			continue;
+
+		/* Check this extension vs other extension */
+		if ( ext_reg != NULL && ext_reg->valext != NULL &&
+			ext_reg->valext->check_conflict != NULL ) {
+			if ( !ext_reg->valext->check_conflict(ext, valdtr,
+				ext_reg->context, ext_arg, regs[i].ext) )
+				return FALSE;
+		}
+
+		/* Check other extension vs this extension */
+		if ( regs[i].valext != NULL &&
+			regs[i].valext->check_conflict != NULL ) {
+			if ( !regs[i].valext->check_conflict(regs[i].ext,
+				valdtr, regs[i].context, regs[i].arg, ext) )
+				return FALSE;
+		}
+	}
+	return TRUE;
+}
+
 bool sieve_validator_extension_load
 (struct sieve_validator *valdtr, struct sieve_command *cmd,
 	struct sieve_ast_argument *ext_arg, const struct sieve_extension *ext)
 {
 	const struct sieve_extension_def *extdef = ext->def;
-	struct sieve_validator_extension_reg *reg;
+	struct sieve_validator_extension_reg *reg = NULL;
 
 	if ( ext->global && (valdtr->flags & SIEVE_COMPILE_FLAG_NOGLOBAL) != 0 ) {
 		if ( cmd != NULL && ext_arg != NULL ) {
@@ -568,6 +606,15 @@ bool sieve_validator_extension_load
 		return FALSE;
 	}
 
+	if ( ext->id >= 0 ) {
+		reg = array_idx_modifiable
+			(&valdtr->extensions, (unsigned int) ext->id);
+		i_assert(reg->ext == NULL || reg->ext == ext);
+		reg->ext = ext;
+		if ( reg->arg == NULL )
+			reg->arg = ext_arg;
+	}
+
 	if ( !sieve_ast_extension_link(valdtr->ast, ext) ) {
 		/*if ( cmd != NULL && ext_arg != NULL ) {
 			sieve_argument_validate_warning(valdtr, ext_arg,
@@ -588,14 +635,15 @@ bool sieve_validator_extension_load
 		}
 	}
 
+	/* Check conflicts with other extensions */
+	if ( !sieve_validator_extensions_check_conficts
+		(valdtr, ext_arg, ext) )
+		return FALSE;
+
 	/* Register extension no matter what and store the AST argument registering it
 	 */
-	if ( ext->id >= 0 ) {
-		reg = array_idx_modifiable(&valdtr->extensions, (unsigned int) ext->id);
-		if ( reg->arg == NULL )
-			reg->arg = ext_arg;
+	if ( reg != NULL )
 		reg->loaded = TRUE;
-	}
 
 	return TRUE;
 }
@@ -669,8 +717,9 @@ void sieve_validator_extension_register
 	if ( ext->id < 0 ) return;
 
 	reg = array_idx_modifiable(&valdtr->extensions, (unsigned int) ext->id);
-	reg->valext = valext;
+	i_assert(reg->ext == NULL || reg->ext == ext);
 	reg->ext = ext;
+	reg->valext = valext;
 	reg->context = context;
 }
 
diff --git a/src/lib-sieve/sieve-validator.h b/src/lib-sieve/sieve-validator.h
index 857b7b2ff..c859c02d0 100644
--- a/src/lib-sieve/sieve-validator.h
+++ b/src/lib-sieve/sieve-validator.h
@@ -127,13 +127,19 @@ bool sieve_validate_tag_parameter
 struct sieve_validator_extension {
 	const struct sieve_extension_def *ext;
 
+	bool (*check_conflict)
+		(const struct sieve_extension *ext,
+			struct sieve_validator *valdtr, void *context,
+			struct sieve_ast_argument *require_arg,
+			const struct sieve_extension *ext_other);
 	bool (*validate)
-		(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-			void *context, struct sieve_ast_argument *require_arg);
+		(const struct sieve_extension *ext,
+			struct sieve_validator *valdtr, void *context,
+			struct sieve_ast_argument *require_arg);
 
 	void (*free)
-		(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-			void *context);
+		(const struct sieve_extension *ext,
+			struct sieve_validator *valdtr, void *context);
 };
 
 bool sieve_validator_extension_load
diff --git a/src/plugins/sieve-extprograms/ext-pipe.c b/src/plugins/sieve-extprograms/ext-pipe.c
index 01ae8158c..d71aee429 100644
--- a/src/plugins/sieve-extprograms/ext-pipe.c
+++ b/src/plugins/sieve-extprograms/ext-pipe.c
@@ -70,14 +70,14 @@ static void ext_pipe_unload(const struct sieve_extension *ext)
  * Validation
  */
 
-static bool ext_pipe_validator_extension_validate
-	(const struct sieve_extension *ext, struct sieve_validator *valdtr, 
-		void *context, struct sieve_ast_argument *require_arg);
+static bool ext_pipe_validator_validate
+	(const struct sieve_extension *ext,
+		struct sieve_validator *valdtr, void *context,
+		struct sieve_ast_argument *require_arg);
 
 const struct sieve_validator_extension pipe_validator_extension = {
-	&vnd_pipe_extension,
-	ext_pipe_validator_extension_validate,
-	NULL
+	.ext = &vnd_pipe_extension,
+	.validate = ext_pipe_validator_validate
 };
 
 static bool ext_pipe_validator_load
@@ -93,17 +93,18 @@ static bool ext_pipe_validator_load
 	return TRUE;
 }
 
-static bool ext_pipe_validator_extension_validate
-(const struct sieve_extension *ext, struct sieve_validator *valdtr,
-	void *context ATTR_UNUSED, struct sieve_ast_argument *require_arg ATTR_UNUSED)
+static bool ext_pipe_validator_validate
+(const struct sieve_extension *ext,
+	struct sieve_validator *valdtr, void *context ATTR_UNUSED,
+	struct sieve_ast_argument *require_arg ATTR_UNUSED)
 {
 	struct sieve_extprograms_config *ext_config =
 		(struct sieve_extprograms_config *) ext->context;
 
 	if ( ext_config != NULL && ext_config->copy_ext != NULL ) {
 		/* Register :copy command tag */
-		sieve_ext_copy_register_tag
-			(valdtr, ext_config->copy_ext, cmd_pipe.identifier);
+		sieve_ext_copy_register_tag(valdtr,
+			ext_config->copy_ext, cmd_pipe.identifier);
 	}
 	return TRUE;
 }
-- 
GitLab