From 76dfa64d6dba768bb46c728ae6821e98ff7b15e1 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Wed, 23 Dec 2009 15:12:37 +0100
Subject: [PATCH] Include extension: global command may now appear anywhere in
 a script.

---
 TODO                                          |  1 -
 src/lib-sieve/plugins/include/cmd-global.c    | 36 +++++++------------
 .../plugins/include/ext-include-variables.c   | 20 ++++++++---
 tests/extensions/include/errors.svtest        |  2 +-
 .../extensions/include/errors/variables.sieve | 17 ++++++++-
 tests/extensions/include/variables.svtest     |  7 ++--
 6 files changed, 49 insertions(+), 34 deletions(-)

diff --git a/TODO b/TODO
index 1bc9ae1a8..0a2b37242 100644
--- a/TODO
+++ b/TODO
@@ -13,7 +13,6 @@ Current activities:
 Next (in order of descending priority/precedence):
 
 * Update include extension to latest draft:
-	- Allow placing the global command anywhere in the script
 	- Implement required ManageSieve behavior
 * Implement mechanism for implicitly including an account's aliases in the
   vacation command's :addresses list.
diff --git a/src/lib-sieve/plugins/include/cmd-global.c b/src/lib-sieve/plugins/include/cmd-global.c
index 8a5f5322d..b9b507410 100644
--- a/src/lib-sieve/plugins/include/cmd-global.c
+++ b/src/lib-sieve/plugins/include/cmd-global.c
@@ -111,29 +111,19 @@ static bool cmd_global_validate
 	struct sieve_ast_argument *arg = cmd->first_positional;
 	struct sieve_command *prev = sieve_command_prev(cmd);
 
-	/* Check valid command placement */
-	if ( !sieve_command_is_toplevel(cmd) ||
-		( !sieve_command_is_first(cmd) && prev != NULL &&
-			!sieve_command_is(prev, cmd_require) ) ) {
-
-		if ( sieve_command_is(cmd, cmd_global) ) {
-			if ( !sieve_command_is(prev, cmd_global) ) {
-				sieve_command_validate_error(valdtr, cmd, 
-					"a global command can only be placed at top level "
-					"at the beginning of the file after any require or "
-					"other global commands");
-				return FALSE;
-			}
-		} else {
-			if ( !sieve_command_is(prev, cmd_import) && 
-				!sieve_command_is(prev, cmd_export) ) {
-				sieve_command_validate_error(valdtr, cmd,
-					"the DEPRECATED %s command can only be placed at top level "
-					"at the beginning of the file after any require or "
-					"import/export commands",
-					sieve_command_identifier(cmd));
-				return FALSE;
-				}
+	/* DEPRECATED: Check valid command placement */
+	if ( !sieve_command_is(cmd, cmd_global) ) {
+		if ( !sieve_command_is_toplevel(cmd) ||
+			( !sieve_command_is_first(cmd) && prev != NULL &&
+				!sieve_command_is(prev, cmd_require) && 
+				!sieve_command_is(prev, cmd_import) && 
+				!sieve_command_is(prev, cmd_export) ) ) {
+			sieve_command_validate_error(valdtr, cmd,
+				"the DEPRECATED %s command can only be placed at top level "
+				"at the beginning of the file after any require or "
+				"import/export commands",
+				sieve_command_identifier(cmd));
+			return FALSE;
 		}
 	}
 
diff --git a/src/lib-sieve/plugins/include/ext-include-variables.c b/src/lib-sieve/plugins/include/ext-include-variables.c
index c9083627a..76c9a990a 100644
--- a/src/lib-sieve/plugins/include/ext-include-variables.c
+++ b/src/lib-sieve/plugins/include/ext-include-variables.c
@@ -32,27 +32,37 @@ struct sieve_variable *ext_include_variable_import_global
 		ext_include_get_ast_context(this_ext, ast);
 	struct ext_include_context *ectx = ext_include_get_context(this_ext);
 	struct sieve_variable_scope *main_scope;
-	struct sieve_variable *var = NULL;
+	struct sieve_variable *global_var = NULL, *local_var;
 
 	/* Sanity safeguard */	
 	i_assert ( ctx->global_vars != NULL );
 
 	/* Get/Declare the variable in the global scope */
-	var = sieve_variable_scope_get_variable(ctx->global_vars, variable, TRUE);
+	global_var = sieve_variable_scope_get_variable
+		(ctx->global_vars, variable, TRUE);
 
 	/* Check whether scope is over its size limit */
-	if ( var == NULL ) {
+	if ( global_var == NULL ) {
 		sieve_command_validate_error(valdtr, cmd,
 			"declaration of new global variable '%s' exceeds the limit "
 			"(max variables: %u)", 
 			variable, SIEVE_VARIABLES_MAX_SCOPE_SIZE);
+		return NULL;
 	}
 	
 	/* Import the global variable into the local script scope */
 	main_scope = sieve_ext_variables_get_main_scope(ectx->var_ext, valdtr);
-	(void)sieve_variable_scope_import(main_scope, var);
 
-	return var;	
+	local_var = sieve_variable_scope_get_variable(main_scope, variable, FALSE);
+	if ( local_var != NULL && local_var->ext != this_ext ) {
+		/* FIXME: indicate location of conflicting set statement */
+		sieve_command_validate_error(valdtr, cmd,
+			"declaration of new global variable '%s' conflicts with earlier local "
+			"use", variable);
+		return NULL;
+	}
+
+	return sieve_variable_scope_import(main_scope, global_var);
 }
 
 /*
diff --git a/tests/extensions/include/errors.svtest b/tests/extensions/include/errors.svtest
index 27ad9ffd7..65f4a4c70 100644
--- a/tests/extensions/include/errors.svtest
+++ b/tests/extensions/include/errors.svtest
@@ -70,7 +70,7 @@ test "Variables" {
 		test_fail "compile should have failed";
 	}
 
-	if not test_error :count "eq" :comparator "i;ascii-numeric" "2" {
+	if not test_error :count "eq" :comparator "i;ascii-numeric" "3" {
 		test_fail "wrong number of errors reported";
 	}
 }
diff --git a/tests/extensions/include/errors/variables.sieve b/tests/extensions/include/errors/variables.sieve
index 277ca0c4a..eac99f8c8 100644
--- a/tests/extensions/include/errors/variables.sieve
+++ b/tests/extensions/include/errors/variables.sieve
@@ -1,8 +1,23 @@
 require "include";
 require "variables";
 
+# Duplicate global declaration (not an error)
+global "frml";
+global "frml";
+
 keep;
 
-# Global after command not being require or global
+# Global after command not being require or global (not an error)
 global "friep";
 
+# DEPRECATED: import/export after command not being require or import/export
+export "friep";
+import "friep";
+
+# Marking local variable as global
+set "frutsels" "frop";
+global "frutsels";
+set "frutsels" "frop";
+
+
+
diff --git a/tests/extensions/include/variables.svtest b/tests/extensions/include/variables.svtest
index 3ee0fb54c..5c4f8d8ab 100644
--- a/tests/extensions/include/variables.svtest
+++ b/tests/extensions/include/variables.svtest
@@ -4,11 +4,10 @@ require "include";
 require "variables";
 
 global ["value1", "value2"];
-global ["value3", "value4"];
-global "result";
-
 set "value1" "Works";
 set "value2" "fine.";
+
+global ["value3", "value4"];
 set "value3" "Yeah";
 set "value4" "it does.";
 
@@ -16,6 +15,8 @@ include "variables-included1";
 include "variables-included2";
 include "variables-included3";
 
+global "result";
+
 test "Basic" {
 	if not string :is "${result}" "Works fine. Yeah it does." {
 		test_fail "invalid result: ${result}";
-- 
GitLab