diff --git a/TODO b/TODO
index 1bc9ae1a842932ae0c5d1dfe969b7a022e0d8745..0a2b372422d39a6bf718af2aa8405ff5b6f05ae3 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 8a5f5322d05a67df85e84363aece5bb082225021..b9b5074100fd61b82e5e399a8bab8cfb58cff3d5 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 c9083627a5e77f621b09048b84df83cf25b5efce..76c9a990a6d5bab6477afe694ee17eb5aa213bec 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 27ad9ffd7e7484bff1b2ee516d8d6140c81df1b9..65f4a4c7071938d4387b1ac84a49e6d73b488771 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 277ca0c4adffb14159db91a43cca652ff04ba5b6..eac99f8c81bb85e00eae34fb3116672af4c0e60b 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 3ee0fb54c9b5a37876b205b94c332077ce3ebc06..5c4f8d8ab86407aa834fadbd0ae157185a721706 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}";