From c459d4698a505c305d31855caefcc074959c228d Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Sun, 19 Oct 2008 18:35:17 +0200
Subject: [PATCH] Clarified errors occurring when colon is missing.

---
 src/lib-sieve/sieve-validator.c  | 70 ++++++++++++++++++++++++--------
 tests/compile/errors.svtest      | 14 ++++++-
 tests/compile/errors/typos.sieve | 29 +++++++++++++
 3 files changed, 93 insertions(+), 20 deletions(-)
 create mode 100644 tests/compile/errors/typos.sieve

diff --git a/src/lib-sieve/sieve-validator.c b/src/lib-sieve/sieve-validator.c
index ed1e18c19..3a70e7cd0 100644
--- a/src/lib-sieve/sieve-validator.c
+++ b/src/lib-sieve/sieve-validator.c
@@ -443,16 +443,15 @@ static void sieve_validator_register_unknown_tag
 }
 
 static const struct sieve_argument *sieve_validator_find_tag
-(struct sieve_validator *validator, struct sieve_command_context *cmd, 
+(struct sieve_validator *valdtr, struct sieve_command_context *cmd, 
 	struct sieve_ast_argument *arg, int *id_code) 
 {
-	const char *tag;
-	unsigned int i;
 	struct sieve_command_registration *cmd_reg = cmd->cmd_reg;
-		
-	tag = sieve_ast_argument_tag(arg);
-	
-	*id_code = 0;
+	const char *tag = sieve_ast_argument_tag(arg);
+	unsigned int i;
+			
+	if ( id_code != NULL )
+		*id_code = 0;
 	
 	/* First check normal tags */
 	if ( array_is_created(&cmd_reg->normal_tags) ) {
@@ -461,7 +460,9 @@ static const struct sieve_argument *sieve_validator_find_tag
 				array_idx(&cmd_reg->normal_tags, i);
 
 			if ( (*reg)->tag != NULL && strcasecmp((*reg)->identifier,tag) == 0) {
-				*id_code = (*reg)->id_code;
+				if ( id_code != NULL )				
+					*id_code = (*reg)->id_code;
+
 				return (*reg)->tag;
 			}
 		}
@@ -474,9 +475,10 @@ static const struct sieve_argument *sieve_validator_find_tag
 				array_idx(&cmd_reg->instanced_tags, i);
   	
 			if ( (*reg)->tag != NULL && 
-				(*reg)->tag->is_instance_of(validator, cmd, arg) ) 
-			{
-				*id_code = (*reg)->id_code;
+				(*reg)->tag->is_instance_of(valdtr, cmd, arg) ) {
+				if ( id_code != NULL )
+					*id_code = (*reg)->id_code;
+				
 				return (*reg)->tag;
 			}
 		}
@@ -485,6 +487,20 @@ static const struct sieve_argument *sieve_validator_find_tag
 	return NULL;
 }
 
+static const struct sieve_argument *sieve_validator_find_tag_by_identifier
+(struct sieve_validator *valdtr, struct sieve_command_context *cmd, 
+	const char *tag) 
+{
+	struct sieve_ast_argument *arg;
+
+	/* Construct dummy argument */
+	arg = t_new(struct sieve_ast_argument, 1);
+	arg->type = SAAT_TAG;
+	arg->_value.tag = tag; 
+
+	return sieve_validator_find_tag(valdtr, cmd, arg, NULL);  
+}
+
 /* 
  * Extension support 
  */
@@ -877,12 +893,24 @@ static bool sieve_validate_command_subtests
 				ctype = cmd_reg->command->type;
 
 			switch ( ctype ) {
-			case SCT_TEST:
+			case SCT_TEST: /* Spurious test */
 				sieve_command_validate_error(valdtr, cmd, 
 					"the %s %s accepts no sub-tests, but tests are specified", 
 					cmd->command->identifier, sieve_command_type_name(cmd->command));
 				break;
-			case SCT_NONE:
+
+			case SCT_NONE: /* Unknown command */
+
+				/* Is it perhaps a tag for which the ':' was omitted ? */
+				if ( 	sieve_validator_find_tag_by_identifier
+					(valdtr, cmd, test->identifier) != NULL ) {
+					sieve_command_validate_error(valdtr, cmd, 
+						"missing colon ':' before ':%s' tag in %s %s", test->identifier, 
+						cmd->command->identifier, sieve_command_type_name(cmd->command));
+					break;
+				} 
+				/* Fall through */
+			
 			case SCT_COMMAND:
 				sieve_command_validate_error(valdtr, cmd, 
 					"missing semicolon ';' after %s %s", 
@@ -1015,14 +1043,20 @@ static bool sieve_validate_command
 				command->pre_validate(valdtr, ctx) ) {
 		
 				/* Check syntax */
-				if ( 
-					!sieve_validate_command_arguments(valdtr, ctx) ||
- 					!sieve_validate_command_subtests
+				if ( !sieve_validate_command_arguments(valdtr, ctx) ) {
+					result = FALSE;
+
+					/* A missing ':' causes a tag to become a test. This can be the cause
+					 * of the arguments validation failing. Therefore we must produce an
+					 * error for the sub-tests as well if appropriate.
+					 */
+					(void)sieve_validate_command_subtests(valdtr, ctx, command->subtests);
+				} else if (
+					!sieve_validate_command_subtests
  						(valdtr, ctx, command->subtests) || 
  					(ast_type == SAT_COMMAND && !sieve_validate_command_block
  						(valdtr, ctx, command->block_allowed, 
- 							command->block_required)) ) 
- 				{
+ 							command->block_required)) ) {
  					result = FALSE;
  				} else {
 					/* Call command validation function if specified */
diff --git a/tests/compile/errors.svtest b/tests/compile/errors.svtest
index 6dfa56b70..8899cf923 100644
--- a/tests/compile/errors.svtest
+++ b/tests/compile/errors.svtest
@@ -223,7 +223,7 @@ test "Stop errors (FIXME: count only)" {
         test_fail "compile should have failed.";
     }
 
-    if not test_error :count "eq" :comparator "i;ascii-numeric" "8" {
+    if not test_error :count "eq" :comparator "i;ascii-numeric" "9" {
         test_fail "wrong number of errors reported";
     }
 }
@@ -322,7 +322,7 @@ test "Typos" {
 		test_fail "compile should have failed.";
 	}
 
-	if not test_error :count "eq" :comparator "i;ascii-numeric" "4" {
+	if not test_error :count "eq" :comparator "i;ascii-numeric" "6" {
 		test_fail "wrong number of errors reported";
 	}
 
@@ -340,6 +340,16 @@ test "Typos" {
 		"missing semicolon * fileinto *" {
 		test_fail "error 3 is invalid";
 	}
+
+	if not test_error :index 4 :matches
+		"*address test requires 2 * 0 * specified" {
+		test_fail "error 4 is invalid";
+	}
+
+	if not test_error :index 5 :matches
+		"missing colon *matches* tag * address test" {
+		test_fail "error 5 is invalid";
+	}
 }
 
 
diff --git a/tests/compile/errors/typos.sieve b/tests/compile/errors/typos.sieve
new file mode 100644
index 000000000..cdc8b252c
--- /dev/null
+++ b/tests/compile/errors/typos.sieve
@@ -0,0 +1,29 @@
+/*
+ * This test is primarily meant to check the compiler's handing of typos
+ * at various locations.
+ */
+
+require "fileinto";
+
+/* 
+ * Missing semicolon 
+ */
+
+fileinto "frop"
+keep;
+
+/* Other situations */
+
+fileinto "frup" 
+true;
+
+fileinto "friep" 
+snot;
+
+/*
+ * Forgot tag colon
+ */ 
+
+if address matches "from" "*frop*" {
+	stop;
+}
-- 
GitLab