From 2ff8eeeddfdc3106480004ffc68049b0ff09caeb Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Fri, 25 Jul 2008 12:12:33 +0200
Subject: [PATCH] Fixed code emission for extension-defined variables and
 removed hardcoded paths from include extension.

---
 TODO                                          |  4 +-
 src/lib-sieve/plugins/include/cmd-include.c   | 21 ++++--
 .../plugins/include/ext-include-binary.c      |  9 +--
 .../plugins/include/ext-include-common.c      | 41 ++++++++---
 .../plugins/include/ext-include-common.h      |  2 +-
 .../variables/ext-variables-operands.c        | 68 +++++++------------
 src/lib-sieve/sieve-script.c                  | 54 ++++++++++++---
 src/lib-sieve/sieve-script.h                  |  4 ++
 src/testsuite/Makefile.am                     |  1 +
 src/testsuite/testsuite.c                     | 16 ++++-
 10 files changed, 139 insertions(+), 81 deletions(-)

diff --git a/TODO b/TODO
index 01adf3dec..321aa1130 100644
--- a/TODO
+++ b/TODO
@@ -1,7 +1,6 @@
 Next (in order of descending priority/precedence):
 * Improve handling of old/corrupt binaries.
-* Resolve problems in variables extension: scope uses hash the wrong way and
-  extension ids are emitted directly.
+* Resolve problems in variables extension: scope uses hash the wrong way.
 
 * Full standards compliance review for the engine and all fully implemented 
   sieve extensions. Issues discovered so far:
@@ -12,7 +11,6 @@ Next (in order of descending priority/precedence):
     - Header test does not strip trailing whitespace
 * Code cleanup 
 * Make sure cmusieve can be replaced seamlessly with the new plugin. Issues:
-    - Include: global and personal script directories are still hardcoded.  
 * Full security review. Enforce limits on number of created objects, script 
   size, execution time, etc...
 * Make simple test suite for the base functionality
diff --git a/src/lib-sieve/plugins/include/cmd-include.c b/src/lib-sieve/plugins/include/cmd-include.c
index 2f4014fba..47a14bc2b 100644
--- a/src/lib-sieve/plugins/include/cmd-include.c
+++ b/src/lib-sieve/plugins/include/cmd-include.c
@@ -174,22 +174,31 @@ static bool cmd_include_validate(struct sieve_validator *validator,
 	struct cmd_include_context_data *ctx_data = 
 		(struct cmd_include_context_data *) cmd->data;
 	struct sieve_script *script;
-	const char *script_path, *script_name;
+	const char *script_dir, *script_name;
 	
 	if ( !sieve_validate_positional_argument
 		(validator, cmd, arg, "value", 1, SAAT_STRING) ) {
 		return FALSE;
 	}
-		
+
 	/* Find the script */
+
 	script_name = sieve_ast_argument_strc(arg);
-	script_path = ext_include_get_script_path(ctx_data->location, script_name);
-	if ( script_path == NULL )
+		
+	script_dir = ext_include_get_script_directory
+		(ctx_data->location, script_name);
+	if ( script_dir == NULL ) {
+		 sieve_command_validate_error(validator, cmd,
+            "specified location for included script '%s' is unavalable "
+			"(system logs should provide more information)",
+			script_name);
 		return FALSE;
+	}
 	
 	/* Create script object */
-	if ( (script = sieve_script_create(script_path, script_name, 
-		sieve_validator_error_handler(validator), NULL)) == NULL ) 
+	script = sieve_script_create_in_directory(script_dir, script_name, 
+		sieve_validator_error_handler(validator), NULL);
+	if ( script == NULL ) 
 		return FALSE;	
 		
 	sieve_ast_link_object(cmd->ast_node, &cmd_include_ast_object);
diff --git a/src/lib-sieve/plugins/include/ext-include-binary.c b/src/lib-sieve/plugins/include/ext-include-binary.c
index cec8dcb55..00e012384 100644
--- a/src/lib-sieve/plugins/include/ext-include-binary.c
+++ b/src/lib-sieve/plugins/include/ext-include-binary.c
@@ -189,7 +189,7 @@ static bool ext_include_binary_open(struct sieve_binary *sbin)
 		unsigned int block_id;
 		enum ext_include_script_location location;
 		string_t *script_name;
-		const char *script_path;
+		const char *script_dir;
 		struct sieve_script *script;
 		
 		if ( 
@@ -211,9 +211,10 @@ static bool ext_include_binary_open(struct sieve_binary *sbin)
 		}		
 		
 		/* Can we find/open the script dependency ? */
-		script_path = ext_include_get_script_path(location, str_c(script_name));		
-		if ( script_path == NULL || 
-			!(script=sieve_script_create(script_path, str_c(script_name), NULL, NULL)) ) {
+		script_dir = ext_include_get_script_directory(location, str_c(script_name));		
+		if ( script_dir == NULL || 
+			!(script=sieve_script_create_in_directory
+				(script_dir, str_c(script_name), NULL, NULL)) ) {
 			/* No, recompile */
 			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 5de51281e..45d67e8a0 100644
--- a/src/lib-sieve/plugins/include/ext-include-common.c
+++ b/src/lib-sieve/plugins/include/ext-include-common.c
@@ -1,3 +1,6 @@
+#include "lib.h"
+#include "str-sanitize.h"
+
 #include "sieve-common.h"
 #include "sieve-error.h"
 #include "sieve-script.h"
@@ -11,6 +14,8 @@
 #include "ext-include-binary.h"
 #include "ext-include-variables.h"
 
+#include <stdlib.h>
+
 /*
  * Forward declarations
  */
@@ -44,23 +49,37 @@ struct ext_include_interpreter_context {
  * Script access 
  */
 
-#define HARDCODED_PERSONAL_DIR "src/lib-sieve/plugins/include/"
-#define HARDCODED_GLOBAL_DIR "src/lib-sieve/plugins/include/"
-
-const char *ext_include_get_script_path
+const char *ext_include_get_script_directory
 (enum ext_include_script_location location, const char *script_name)
 {
-	/* FIXME: Hardcoded */	
+	const char *sieve_dir;
+
 	switch ( location ) {
 	case EXT_INCLUDE_LOCATION_PERSONAL:
-		return t_strconcat(HARDCODED_PERSONAL_DIR, script_name, ".sieve", NULL);
-	case EXT_INCLUDE_LOCATION_GLOBAL:
-		return t_strconcat(HARDCODED_GLOBAL_DIR, script_name, ".sieve", NULL);
-	default:
+        sieve_dir = getenv("SIEVE_DIR");
+
+        if (sieve_dir == NULL)
+            sieve_dir = getenv("HOME");
+        if (sieve_dir == NULL) {
+            sieve_sys_error("include: sieve_dir and home not set "
+                   "(wanted script %s)", str_sanitize(script_name, 80));
+            return NULL;
+        }
 		break;
+   	case EXT_INCLUDE_LOCATION_GLOBAL:
+		sieve_dir = getenv("SIEVE_GLOBAL_DIR");
+
+        if (sieve_dir == NULL) {
+            sieve_sys_warning("include: sieve_global_dir not set "
+                   "(wanted script %s)", str_sanitize(script_name, 80));
+            return NULL;
+        }
+		break;
+	default:
+		return NULL;
 	}
-	
-	return NULL;
+
+	return sieve_dir;
 }
 
 /* 
diff --git a/src/lib-sieve/plugins/include/ext-include-common.h b/src/lib-sieve/plugins/include/ext-include-common.h
index 80a78cff3..a793d2fc2 100644
--- a/src/lib-sieve/plugins/include/ext-include-common.h
+++ b/src/lib-sieve/plugins/include/ext-include-common.h
@@ -40,7 +40,7 @@ enum ext_include_script_location {
 
 /* Script access */
 
-const char *ext_include_get_script_path
+const char *ext_include_get_script_directory
 	(enum ext_include_script_location location, const char *script_name);
 
 /* Generator */
diff --git a/src/lib-sieve/plugins/variables/ext-variables-operands.c b/src/lib-sieve/plugins/variables/ext-variables-operands.c
index ed8a37798..a9ff1709a 100644
--- a/src/lib-sieve/plugins/variables/ext-variables-operands.c
+++ b/src/lib-sieve/plugins/variables/ext-variables-operands.c
@@ -42,7 +42,7 @@ const struct sieve_operand variable_operand = {
 };
 
 void ext_variables_opr_variable_emit
-	(struct sieve_binary *sbin, struct sieve_variable *var) 
+(struct sieve_binary *sbin, struct sieve_variable *var) 
 {
 	if ( var->ext == NULL ) {
 		/* Default variable storage */
@@ -52,60 +52,45 @@ void ext_variables_opr_variable_emit
 		return;
 	} 
 
-	/* FIXME: never directly emit *ext->id !! */
 	(void) sieve_operand_emit_code(sbin, &variable_operand);
-	(void) sieve_binary_emit_byte(sbin, 1 +	*var->ext->id);
+	(void) sieve_binary_emit_extension(sbin, var->ext, 1);
 	(void) sieve_binary_emit_integer(sbin, var->index);
 }
 
 static bool opr_variable_dump
 	(const struct sieve_dumptime_env *denv, sieve_size_t *address) 
 {
-	unsigned int code;
-	int ext_id;
 	sieve_size_t index = 0;
-	const struct sieve_extension *ext = NULL;
-	
-	if ( !sieve_binary_read_byte(denv->sbin, address, &code) ) {
-		return FALSE;
-	} 
+	const struct sieve_extension *ext;
+	unsigned int code = 1; /* Initially set to offset value */
 
-	ext_id = code - 1;
-	
-	if ( ext_id >= 0 && (ext = sieve_extension_get_by_id(ext_id)) == NULL ) {
+	if ( !sieve_binary_read_extension(denv->sbin, address, &code, &ext) )
 		return FALSE;
-	}
 	
-	if ( !sieve_binary_read_integer(denv->sbin, address, &index) ) {
+	if ( !sieve_binary_read_integer(denv->sbin, address, &index) )
 		return FALSE;
-	}
 		
 	if ( ext == NULL )
 		sieve_code_dumpf(denv, "VAR: %ld", (long) index);
 	else
-		sieve_code_dumpf(denv, "VAR: [%d:%s] %ld", ext_id, ext->name, (long) index);
+		sieve_code_dumpf(denv, "VAR: [%d:%s] %ld", *ext->id, ext->name, (long) index);
 	return TRUE;
 }
 
 static bool opr_variable_read_value
   (const struct sieve_runtime_env *renv, sieve_size_t *address, string_t **str)
 { 
-	unsigned int code;
-	int ext_id;
-	const struct sieve_extension *ext = NULL;
+	const struct sieve_extension *ext;
+	unsigned int code = 1; /* Initially set to offset value */
 	struct sieve_variable_storage *storage;
 	sieve_size_t index = 0;
 	
-	if ( !sieve_binary_read_byte(renv->sbin, address, &code) ) {
+	if ( !sieve_binary_read_extension(renv->sbin, address, &code, &ext) )
 		return FALSE;
-	}
-	ext_id = code - 1;
-	
-	if ( ext_id >= 0 ) 
-		ext = sieve_extension_get_by_id(ext_id);
 
 	storage = sieve_ext_variables_get_storage(renv->interp, ext);
-	if ( storage == NULL ) return FALSE;
+	if ( storage == NULL ) 
+		return FALSE;
 	
 	if (sieve_binary_read_integer(renv->sbin, address, &index) ) {
 		/* Parameter str can be NULL if we are requested to only skip and not 
@@ -127,33 +112,26 @@ bool sieve_variable_operand_read_data
 	sieve_size_t *address, struct sieve_variable_storage **storage, 
 	unsigned int *var_index)
 {
-	unsigned int code;
-	int ext_id;
-	const struct sieve_extension *ext = NULL;
+	const struct sieve_extension *ext;
+	unsigned int code = 1; /* Initially set to offset value */
 	sieve_size_t idx = 0;
 
 	if ( operand != &variable_operand ) {
 		return FALSE;
 	}
 
-	if ( !sieve_binary_read_byte(renv->sbin, address, &code) ) {
-		return FALSE;
-	}
-	ext_id = code - 1;
-
-	if ( ext_id >= 0 )
-        ext = sieve_extension_get_by_id(ext_id);
+	if ( !sieve_binary_read_extension(renv->sbin, address, &code, &ext) )
+        return FALSE;
 		
 	*storage = sieve_ext_variables_get_storage(renv->interp, ext);
-	if ( *storage == NULL )	return FALSE;
-	
-	if (sieve_binary_read_integer(renv->sbin, address, &idx) ) {
-		*var_index = idx;
-		
-		return TRUE;
-	}
+	if ( *storage == NULL )	
+		return FALSE;
 	
-	return FALSE;
+	if ( !sieve_binary_read_integer(renv->sbin, address, &idx) )
+		return FALSE;		
+
+	*var_index = idx;
+	return TRUE;
 }
 
 bool sieve_variable_operand_read
diff --git a/src/lib-sieve/sieve-script.c b/src/lib-sieve/sieve-script.c
index 6c6761936..f94915540 100644
--- a/src/lib-sieve/sieve-script.c
+++ b/src/lib-sieve/sieve-script.c
@@ -13,6 +13,30 @@
 
 #define SIEVE_READ_BLOCK_SIZE (1024*8)
 
+static inline const char *_sieve_scriptfile_get_basename(const char *filename)
+{
+	const char *ext;
+
+	/* Extract the script name */
+	ext = strrchr(filename, '.');
+	if ( ext == NULL || ext == filename || strncmp(ext,".sieve",6) != 0 )
+		return filename;
+	
+	return t_strdup_until(filename, ext);	
+}
+
+static inline const char *_sieve_scriptfile_from_name(const char *name)
+{
+    const char *ext;
+
+    /* See if it ends in .sieve already */
+    ext = strrchr(name, '.');
+    if ( ext == NULL || ext == name || strncmp(ext,".sieve",6) != 0 )
+        return t_strconcat(name, ".sieve", NULL);
+
+    return name;
+}
+
 /* Script object */
 struct sieve_script *sieve_script_init
 (struct sieve_script *script, const char *path, const char *name, 
@@ -39,14 +63,7 @@ struct sieve_script *sieve_script_init
 		}
 		
 		if ( name == NULL || *name == '\0' ) {
-			const char *ext;
-			
-			/* Extract the script name */
-			ext = strrchr(filename, '.');
-			if ( ext == NULL || ext == filename || strncmp(ext,".sieve",6) != 0 )
-				basename = filename;
-			else
-				basename = t_strdup_until(filename, ext);
+			basename = _sieve_scriptfile_get_basename(filename);
 		} else {
 			basename = name;
 		}
@@ -115,12 +132,29 @@ struct sieve_script *sieve_script_init
 	return script;
 }
 
-struct sieve_script *sieve_script_create(const char *path, const char *name,
-    struct sieve_error_handler *ehandler, bool *exists_r)
+struct sieve_script *sieve_script_create
+(const char *path, const char *name, 
+	struct sieve_error_handler *ehandler, bool *exists_r)
 {
 	return sieve_script_init(NULL, path, name, ehandler, exists_r);
 }
 
+struct sieve_script *sieve_script_create_in_directory
+(const char *dirpath, const char *name,
+    struct sieve_error_handler *ehandler, bool *exists_r)
+{
+	const char *path;
+
+	if ( dirpath[strlen(dirpath)-1] == '/' )
+		path = t_strconcat(dirpath, 
+			_sieve_scriptfile_from_name(name), NULL);
+	else
+		path = t_strconcat(dirpath, "/",
+			_sieve_scriptfile_from_name(name), NULL);
+
+    return sieve_script_init(NULL, path, name, ehandler, exists_r);
+}
+
 void sieve_script_ref(struct sieve_script *script)
 {
 	script->refcount++;
diff --git a/src/lib-sieve/sieve-script.h b/src/lib-sieve/sieve-script.h
index 8747f9bcb..78c37e34d 100644
--- a/src/lib-sieve/sieve-script.h
+++ b/src/lib-sieve/sieve-script.h
@@ -9,6 +9,10 @@ struct sieve_script *sieve_script_create
 	(const char *path, const char *name, 
 		struct sieve_error_handler *ehandler, bool *exists_r);
 
+struct sieve_script *sieve_script_create_in_directory
+	(const char *dirpath, const char *name,
+    	struct sieve_error_handler *ehandler, bool *exists_r);
+
 void sieve_script_ref(struct sieve_script *script);
 void sieve_script_unref(struct sieve_script **script);
 
diff --git a/src/testsuite/Makefile.am b/src/testsuite/Makefile.am
index a25a30c02..e0b6887b2 100644
--- a/src/testsuite/Makefile.am
+++ b/src/testsuite/Makefile.am
@@ -75,6 +75,7 @@ test_cases = \
 	tests/match-types/matches.svtest \
 	tests/address-parts/subaddress.svtest \
 	tests/extensions/variables/basic.svtest \
+	tests/extensions/include/variables.svtest \
 	tests/compile/compile.svtest \
 	tests/compile/compile-examples.svtest
 
diff --git a/src/testsuite/testsuite.c b/src/testsuite/testsuite.c
index 5d9adb162..e862480a4 100644
--- a/src/testsuite/testsuite.c
+++ b/src/testsuite/testsuite.c
@@ -6,11 +6,15 @@
 #include "ostream.h"
 #include "hostpid.h"
 #include "mail-storage.h"
+#include "env-util.h"
 
 #include "mail-raw.h"
 #include "namespaces.h"
+
 #include "sieve.h"
 #include "sieve-extensions.h"
+#include "sieve-script.h"
+#include "sieve-binary.h"
 #include "sieve-result.h"
 #include "sieve-interpreter.h"
 
@@ -92,7 +96,7 @@ static struct sieve_binary *_compile_sieve_script(const char *filename)
 
 	if ( (sbin = sieve_compile(filename, ehandler)) == NULL ) {
 		sieve_error_handler_unref(&ehandler);
-		i_fatal("Failed to compile sieve script\n");
+		i_fatal("Failed to compile test script %s\n", filename);
 	}
 
 	sieve_error_handler_unref(&ehandler);
@@ -162,6 +166,7 @@ int main(int argc, char **argv)
 	int i;
 	pool_t namespaces_pool;
 	struct sieve_binary *sbin;
+	const char *sieve_dir;
 	struct sieve_script_env scriptenv;
 
 	testsuite_init();
@@ -189,6 +194,15 @@ int main(int argc, char **argv)
 	}
 
 	printf("Test case: %s:\n\n", scriptfile);
+
+	/* Initialize environment */
+	sieve_dir = strrchr(scriptfile, '/');
+    if ( sieve_dir == NULL )
+        sieve_dir="./";
+	else
+		sieve_dir = t_strdup_until(scriptfile, sieve_dir);
+
+	env_put(t_strconcat("SIEVE_DIR=", sieve_dir, NULL));
 	
 	/* Compile sieve script */
 	sbin = _compile_sieve_script(scriptfile);
-- 
GitLab