From 25db5e1a0533892e75d2312ea4c2effc76b40fb9 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Tue, 9 Dec 2008 19:06:27 +0100
Subject: [PATCH] Compiler now warns about syntactically invalid header field
 names.

---
 TODO                                         |  1 -
 src/lib-sieve/rfc2822.c                      |  6 ++-
 src/lib-sieve/rfc2822.h                      |  2 +
 src/lib-sieve/sieve-commands.c               | 39 +++++++++++++++++++-
 src/lib-sieve/sieve-commands.h               |  7 ++++
 src/lib-sieve/tst-address.c                  | 29 ++++++++-------
 src/lib-sieve/tst-exists.c                   | 11 ++++--
 src/lib-sieve/tst-header.c                   | 27 ++++++++------
 tests/compile/warnings/invalid-headers.sieve | 11 ++++++
 9 files changed, 100 insertions(+), 33 deletions(-)
 create mode 100644 tests/compile/warnings/invalid-headers.sieve

diff --git a/TODO b/TODO
index 2e721ee52..6a901579d 100644
--- a/TODO
+++ b/TODO
@@ -28,7 +28,6 @@ Next (in order of descending priority/precedence):
   common implementation. 
 * Implement executing an arbitrary number of scripts sequentially, acting on the 
   same set of result actions (multiscript).
-* Warn about the use of syntactically invalid header names. 
 * Implement dropping errors in the user's mailbox as a mail message.
 * Add normalize() method to comparators to normalize the string before matching
   (for efficiency).
diff --git a/src/lib-sieve/rfc2822.c b/src/lib-sieve/rfc2822.c
index 00fa53901..329432a6c 100644
--- a/src/lib-sieve/rfc2822.c
+++ b/src/lib-sieve/rfc2822.c
@@ -17,13 +17,15 @@ bool rfc2822_header_field_name_verify
 
 	/* field-name   =   1*ftext
 	 * ftext        =   %d33-57 /               ; Any character except
-   *                  %d59-126                ;  controls, SP, and
-   *                                          ;  ":".
+	 *                  %d59-126                ;  controls, SP, and
+	 *                                          ;  ":".
 	 */
 	 
 	while ( p < pend ) {
 		if ( *p < 33 || *p == ':' )
 			return FALSE;
+
+		p++;
 	}	
 	
 	return TRUE;
diff --git a/src/lib-sieve/rfc2822.h b/src/lib-sieve/rfc2822.h
index e4b8fe193..3748c950f 100644
--- a/src/lib-sieve/rfc2822.h
+++ b/src/lib-sieve/rfc2822.h
@@ -4,6 +4,8 @@
 #ifndef __RFC2822_H
 #define __RFC2822_H
 
+#include "lib.h"
+
 bool rfc2822_header_field_name_verify
 	(const char *field_name, unsigned int len);
 
diff --git a/src/lib-sieve/sieve-commands.c b/src/lib-sieve/sieve-commands.c
index c2976d5b1..4dbd914be 100644
--- a/src/lib-sieve/sieve-commands.c
+++ b/src/lib-sieve/sieve-commands.c
@@ -1,11 +1,17 @@
 /* Copyright (c) 2002-2008 Dovecot Sieve authors, see the included COPYING file
  */
 
+#include "lib.h"
+#include "str.h"
+#include "str-sanitize.h"
+
+#include "rfc2822.h"
+
+#include "sieve-common.h"
 #include "sieve-ast.h"
 #include "sieve-validator.h"
 #include "sieve-generator.h"
 #include "sieve-binary.h"
-
 #include "sieve-commands.h"
 #include "sieve-code.h"
 #include "sieve-interpreter.h"
@@ -267,3 +273,34 @@ bool sieve_command_block_exits_unconditionally
 {
 	return ( cmd->block_exit_command != NULL );
 }
+
+/*
+ * Command utility functions
+ */
+
+/* NOTE: this may be moved */
+
+static int _verify_header_name_item
+(void *context, struct sieve_ast_argument *header)
+{
+	struct sieve_validator *valdtr = (struct sieve_validator *) context;
+	string_t *name = sieve_ast_argument_str(header);
+
+	if ( sieve_argument_is_string_literal(header) &&
+		!rfc2822_header_field_name_verify(str_c(name), str_len(name)) ) {
+		sieve_argument_validate_warning
+			(valdtr, header, "specified header field name '%s' is invalid",
+				str_sanitize(str_c(name), 80));
+
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+bool sieve_command_verify_headers_argument
+(struct sieve_validator *valdtr, struct sieve_ast_argument *headers)
+{	
+	return ( sieve_ast_stringlist_map
+		(&headers, (void *) valdtr, _verify_header_name_item) >= 0 );
+}
diff --git a/src/lib-sieve/sieve-commands.h b/src/lib-sieve/sieve-commands.h
index 888baa000..bcfd6c89c 100644
--- a/src/lib-sieve/sieve-commands.h
+++ b/src/lib-sieve/sieve-commands.h
@@ -196,4 +196,11 @@ extern const struct sieve_command tst_size;
 extern const struct sieve_command *sieve_core_tests[];
 extern const unsigned int sieve_core_tests_count;
 
+/*
+ * Command utility functions
+ */
+
+bool sieve_command_verify_headers_argument
+(struct sieve_validator *valdtr, struct sieve_ast_argument *headers);
+
 #endif /* __SIEVE_COMMANDS_H */
diff --git a/src/lib-sieve/tst-address.c b/src/lib-sieve/tst-address.c
index e6d01d0bb..78099c82a 100644
--- a/src/lib-sieve/tst-address.c
+++ b/src/lib-sieve/tst-address.c
@@ -27,9 +27,9 @@
  */
 
 static bool tst_address_registered
-	(struct sieve_validator *validator, struct sieve_command_registration *cmd_reg);
+	(struct sieve_validator *valdtr, struct sieve_command_registration *cmd_reg);
 static bool tst_address_validate
-	(struct sieve_validator *validator, struct sieve_command_context *tst);
+	(struct sieve_validator *valdtr, struct sieve_command_context *tst);
 static bool tst_address_generate
 	(const struct sieve_codegen_env *cgenv, struct sieve_command_context *ctx);
 
@@ -68,12 +68,12 @@ const struct sieve_operation tst_address_operation = {
  */
 
 static bool tst_address_registered
-	(struct sieve_validator *validator, struct sieve_command_registration *cmd_reg) 
+	(struct sieve_validator *valdtr, struct sieve_command_registration *cmd_reg) 
 {
 	/* The order of these is not significant */
-	sieve_comparators_link_tag(validator, cmd_reg, SIEVE_AM_OPT_COMPARATOR );
-	sieve_address_parts_link_tags(validator, cmd_reg, SIEVE_AM_OPT_ADDRESS_PART);
-	sieve_match_types_link_tags(validator, cmd_reg, SIEVE_AM_OPT_MATCH_TYPE);
+	sieve_comparators_link_tag(valdtr, cmd_reg, SIEVE_AM_OPT_COMPARATOR );
+	sieve_address_parts_link_tags(valdtr, cmd_reg, SIEVE_AM_OPT_ADDRESS_PART);
+	sieve_match_types_link_tags(valdtr, cmd_reg, SIEVE_AM_OPT_MATCH_TYPE);
 
 	return TRUE;
 }
@@ -129,25 +129,28 @@ static int _header_is_allowed
 }
 
 static bool tst_address_validate
-	(struct sieve_validator *validator, struct sieve_command_context *tst) 
+	(struct sieve_validator *valdtr, struct sieve_command_context *tst) 
 {
 	struct sieve_ast_argument *arg = tst->first_positional;
 	struct sieve_ast_argument *header;
 		
 	if ( !sieve_validate_positional_argument
-		(validator, tst, arg, "header list", 1, SAAT_STRING_LIST) ) {
+		(valdtr, tst, arg, "header list", 1, SAAT_STRING_LIST) ) {
 		return FALSE;
 	}
 	
-	if ( !sieve_validator_argument_activate(validator, tst, arg, FALSE) )
+	if ( !sieve_validator_argument_activate(valdtr, tst, arg, FALSE) )
 		return FALSE;
 
+	if ( !sieve_command_verify_headers_argument(valdtr, arg) )
+        return FALSE;
+
 	/* Check if supplied header names are allowed
 	 *   FIXME: verify dynamic header names at runtime 
 	 */
 	header = arg;
 	if ( !sieve_ast_stringlist_map(&header, NULL, _header_is_allowed) ) {		
-		sieve_argument_validate_error(validator, header, 
+		sieve_argument_validate_error(valdtr, header, 
 			"specified header '%s' is not allowed for the address test", 
 			str_sanitize(sieve_ast_strlist_strc(header), 64));
 		return FALSE;
@@ -158,16 +161,16 @@ static bool tst_address_validate
 	arg = sieve_ast_argument_next(arg);
 	
 	if ( !sieve_validate_positional_argument
-		(validator, tst, arg, "key list", 2, SAAT_STRING_LIST) ) {
+		(valdtr, tst, arg, "key list", 2, SAAT_STRING_LIST) ) {
 		return FALSE;
 	}
 
-	if ( !sieve_validator_argument_activate(validator, tst, arg, FALSE) )
+	if ( !sieve_validator_argument_activate(valdtr, tst, arg, FALSE) )
 		return FALSE;
 	
 	/* Validate the key argument to a specified match type */
 	return sieve_match_type_validate
-		(validator, tst, arg, &is_match_type, &i_ascii_casemap_comparator); 
+		(valdtr, tst, arg, &is_match_type, &i_ascii_casemap_comparator); 
 }
 
 /* 
diff --git a/src/lib-sieve/tst-exists.c b/src/lib-sieve/tst-exists.c
index 9bcb30f21..fa25143da 100644
--- a/src/lib-sieve/tst-exists.c
+++ b/src/lib-sieve/tst-exists.c
@@ -17,7 +17,7 @@
  */
 
 static bool tst_exists_validate
-	(struct sieve_validator *validator, struct sieve_command_context *tst);
+	(struct sieve_validator *valdtr, struct sieve_command_context *tst);
 static bool tst_exists_generate
 	(const struct sieve_codegen_env *cgenv, struct sieve_command_context *ctx);
 
@@ -56,16 +56,19 @@ const struct sieve_operation tst_exists_operation = {
  */
 
 static bool tst_exists_validate
-  (struct sieve_validator *validator, struct sieve_command_context *tst) 
+  (struct sieve_validator *valdtr, struct sieve_command_context *tst) 
 {
 	struct sieve_ast_argument *arg = tst->first_positional;
 		
 	if ( !sieve_validate_positional_argument
-		(validator, tst, arg, "header names", 1, SAAT_STRING_LIST) ) {
+		(valdtr, tst, arg, "header names", 1, SAAT_STRING_LIST) ) {
 		return FALSE;
 	}
 	
-	return sieve_validator_argument_activate(validator, tst, arg, FALSE);
+	if ( !sieve_validator_argument_activate(valdtr, tst, arg, FALSE) )
+		return FALSE;
+
+	return sieve_command_verify_headers_argument(valdtr, arg);
 }
 
 /* 
diff --git a/src/lib-sieve/tst-header.c b/src/lib-sieve/tst-header.c
index fe8db5783..8023d03cc 100644
--- a/src/lib-sieve/tst-header.c
+++ b/src/lib-sieve/tst-header.c
@@ -21,9 +21,9 @@
  */
 
 static bool tst_header_registered
-	(struct sieve_validator *validator, struct sieve_command_registration *cmd_reg);
+	(struct sieve_validator *valdtr, struct sieve_command_registration *cmd_reg);
 static bool tst_header_validate
-	(struct sieve_validator *validator, struct sieve_command_context *tst);
+	(struct sieve_validator *valdtr, struct sieve_command_context *tst);
 static bool tst_header_generate
 	(const struct sieve_codegen_env *cgenv, struct sieve_command_context *ctx);
 
@@ -62,11 +62,11 @@ const struct sieve_operation tst_header_operation = {
  */
 
 static bool tst_header_registered
-	(struct sieve_validator *validator, struct sieve_command_registration *cmd_reg) 
+	(struct sieve_validator *valdtr, struct sieve_command_registration *cmd_reg) 
 {
 	/* The order of these is not significant */
-	sieve_comparators_link_tag(validator, cmd_reg, SIEVE_MATCH_OPT_COMPARATOR);
-	sieve_match_types_link_tags(validator, cmd_reg, SIEVE_MATCH_OPT_MATCH_TYPE);
+	sieve_comparators_link_tag(valdtr, cmd_reg, SIEVE_MATCH_OPT_COMPARATOR);
+	sieve_match_types_link_tags(valdtr, cmd_reg, SIEVE_MATCH_OPT_MATCH_TYPE);
 
 	return TRUE;
 }
@@ -74,33 +74,36 @@ static bool tst_header_registered
 /* 
  * Validation 
  */
-
+ 
 static bool tst_header_validate
-	(struct sieve_validator *validator, struct sieve_command_context *tst) 
+	(struct sieve_validator *valdtr, struct sieve_command_context *tst) 
 { 		
 	struct sieve_ast_argument *arg = tst->first_positional;
 	
 	if ( !sieve_validate_positional_argument
-		(validator, tst, arg, "header names", 1, SAAT_STRING_LIST) ) {
+		(valdtr, tst, arg, "header names", 1, SAAT_STRING_LIST) ) {
 		return FALSE;
 	}
 	
-	if ( !sieve_validator_argument_activate(validator, tst, arg, FALSE) )
+	if ( !sieve_validator_argument_activate(valdtr, tst, arg, FALSE) )
+		return FALSE;
+
+	if ( !sieve_command_verify_headers_argument(valdtr, arg) )
 		return FALSE;
 	
 	arg = sieve_ast_argument_next(arg);
 
 	if ( !sieve_validate_positional_argument
-		(validator, tst, arg, "key list", 2, SAAT_STRING_LIST) ) {
+		(valdtr, tst, arg, "key list", 2, SAAT_STRING_LIST) ) {
 		return FALSE;
 	}
 	
-	if ( !sieve_validator_argument_activate(validator, tst, arg, FALSE) )
+	if ( !sieve_validator_argument_activate(valdtr, tst, arg, FALSE) )
 		return FALSE;
 
 	/* Validate the key argument to a specified match type */
 	return sieve_match_type_validate
-		(validator, tst, arg, &is_match_type, &i_ascii_casemap_comparator);
+		(valdtr, tst, arg, &is_match_type, &i_ascii_casemap_comparator);
 }
 
 /*
diff --git a/tests/compile/warnings/invalid-headers.sieve b/tests/compile/warnings/invalid-headers.sieve
new file mode 100644
index 000000000..e82c4744a
--- /dev/null
+++ b/tests/compile/warnings/invalid-headers.sieve
@@ -0,0 +1,11 @@
+if header "from:" "frop@rename-it.nl" {
+	stop;
+}
+
+if address "from:" "frop@rename-it.nl" {
+	stop;
+}
+
+if exists "from:" {
+	stop;
+}
-- 
GitLab