From aa8e4148dcfb1f9a98cf42dfdfaa87105d0b8a20 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Fri, 21 Nov 2014 01:23:15 +0100
Subject: [PATCH] lib-sieve: metadata extensions: Implemented proper checking
of annotation names everywhere. Also added testsuite items to test syntax.
---
Makefile.am | 1 +
.../plugins/metadata/tst-metadataexists.c | 45 ++++++++++++++++-
tests/extensions/metadata/errors.svtest | 18 +++++++
tests/extensions/metadata/errors/syntax.sieve | 48 +++++++++++++++++++
tests/extensions/metadata/execute.svtest | 29 ++++++++++-
5 files changed, 138 insertions(+), 3 deletions(-)
create mode 100644 tests/extensions/metadata/errors.svtest
create mode 100644 tests/extensions/metadata/errors/syntax.sieve
diff --git a/Makefile.am b/Makefile.am
index 6693f4649..2331e532a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -159,6 +159,7 @@ test_cases = \
tests/extensions/duplicate/execute.svtest \
tests/extensions/duplicate/execute-vnd.svtest \
tests/extensions/metadata/execute.svtest \
+ tests/extensions/metadata/errors.svtest \
tests/extensions/vnd.dovecot/debug/execute.svtest \
tests/deprecated/notify/basic.svtest \
tests/deprecated/notify/mailto.svtest \
diff --git a/src/lib-sieve/plugins/metadata/tst-metadataexists.c b/src/lib-sieve/plugins/metadata/tst-metadataexists.c
index 58808354a..99b7dd035 100644
--- a/src/lib-sieve/plugins/metadata/tst-metadataexists.c
+++ b/src/lib-sieve/plugins/metadata/tst-metadataexists.c
@@ -104,10 +104,41 @@ const struct sieve_operation_def servermetadataexists_operation = {
* Test validation
*/
+struct _validate_context {
+ struct sieve_validator *valdtr;
+ struct sieve_command *tst;
+};
+
+static int tst_metadataexists_annotation_validate
+(void *context, struct sieve_ast_argument *arg)
+{
+ struct _validate_context *valctx =
+ (struct _validate_context *)context;
+
+ if ( sieve_argument_is_string_literal(arg) ) {
+ const char *aname = sieve_ast_strlist_strc(arg);
+ const char *error;
+
+ if ( !imap_metadata_verify_entry_name(aname, &error) ) {
+ char *lcerror = t_strdup_noconst(error);
+ lcerror[0] = i_tolower(lcerror[0]);
+ sieve_argument_validate_warning
+ (valctx->valdtr, arg, "%s test: "
+ "specified annotation name `%s' is invalid: %s",
+ sieve_command_identifier(valctx->tst),
+ str_sanitize(aname, 256), lcerror);
+ }
+ }
+
+ return TRUE; /* Can't check at compile time */
+}
+
static bool tst_metadataexists_validate
(struct sieve_validator *valdtr, struct sieve_command *tst)
{
struct sieve_ast_argument *arg = tst->first_positional;
+ struct sieve_ast_argument *aarg;
+ struct _validate_context valctx;
unsigned int arg_index = 1;
if ( sieve_command_is(tst, metadataexists_test) ) {
@@ -127,7 +158,16 @@ static bool tst_metadataexists_validate
return FALSE;
}
- return sieve_validator_argument_activate(valdtr, tst, arg, FALSE);
+ if ( !sieve_validator_argument_activate(valdtr, tst, arg, FALSE) )
+ return FALSE;
+
+ aarg = arg;
+ memset(&valctx, 0, sizeof(valctx));
+ valctx.valdtr = valdtr;
+ valctx.tst = tst;
+
+ return sieve_ast_stringlist_map
+ (&aarg, (void*)&valctx, tst_metadataexists_annotation_validate);
}
/*
@@ -232,7 +272,8 @@ static int tst_metadataexists_check_annotations
"specified annotation name `%s' is invalid: %s",
(mailbox != NULL ? "metadataexists" : "servermetadataexists"),
str_sanitize(str_c(aname), 256), _lc_error(error));
- continue;
+ all_exist = FALSE;
+ break;;
}
ret = imap_metadata_get(imtrans, str_c(aname), &avalue);
diff --git a/tests/extensions/metadata/errors.svtest b/tests/extensions/metadata/errors.svtest
new file mode 100644
index 000000000..e0cffaafc
--- /dev/null
+++ b/tests/extensions/metadata/errors.svtest
@@ -0,0 +1,18 @@
+require "vnd.dovecot.testsuite";
+
+require "relational";
+require "comparator-i;ascii-numeric";
+
+/*
+ * Invalid syntax
+ */
+
+test "Invalid Syntax" {
+ if test_script_compile "errors/syntax.sieve" {
+ test_fail "compile should have failed";
+ }
+
+ if not test_error :count "eq" :comparator "i;ascii-numeric" "27" {
+ test_fail "wrong number of errors reported";
+ }
+}
diff --git a/tests/extensions/metadata/errors/syntax.sieve b/tests/extensions/metadata/errors/syntax.sieve
new file mode 100644
index 000000000..4b488d136
--- /dev/null
+++ b/tests/extensions/metadata/errors/syntax.sieve
@@ -0,0 +1,48 @@
+require "mboxmetadata";
+require "servermetadata";
+
+# 1-4: Used as a command
+metadata;
+metadataexists;
+servermetadata;
+servermetadataexists;
+
+# 5-8: Used with no argument
+if metadata {}
+if metadataexists {}
+if servermetadata {}
+if servermetadataexists {}
+
+# 9-10: Used with one string argument
+if metadata "frop" { }
+if servermetadata "frop" { }
+if metadataexists "frop" { }
+
+# Used with one number argument
+if metadata 13123123 { }
+if servermetadata 123123 { }
+if metadataexists 123123 { }
+if servermetadataexists 123123 {}
+
+# Used with one string list argument
+if metadata ["frop"] { }
+if servermetadata ["frop"] { }
+if metadataexists ["frop"] { }
+
+# Used with unknown tag
+if metadata :frop "frop" { }
+if servermetadata :frop "frop" { }
+if metadataexists :frop "frop" { }
+if servermetadataexists :frop "frop" {}
+
+# Invalid arguments
+if metadata "/private/frop" "friep" {}
+if servermetadata "INBOX" "/private/frop" "friep" {}
+if metadataexists 23 "/private/frop" {}
+if servermetadataexists "INBOX" "/private/frop" {}
+
+# Invalid annotations
+if metadata "INBOX" "frop" "friep" {}
+if servermetadata "frop" "friep" {}
+if metadataexists "INBOX" ["/private/frop", "/friep"] { }
+if servermetadataexists ["/private/frop", "/friep", "/private/friep"] { }
diff --git a/tests/extensions/metadata/execute.svtest b/tests/extensions/metadata/execute.svtest
index 0a218092a..e3a2b70c8 100644
--- a/tests/extensions/metadata/execute.svtest
+++ b/tests/extensions/metadata/execute.svtest
@@ -35,6 +35,13 @@ test "MetadataExists - All exist" {
}
}
+test "MetadataExists - Invalid" {
+ if metadataexists "INBOX"
+ ["/shared/frop", "/friep", "/private/frml"] {
+ test_fail "metadataexists accepted invalid annotation name";
+ }
+}
+
test "Metadata" {
if not metadata :is "INBOX" "/private/frop" "FROP!" {
test_fail "invalid metadata value for /private/frop";
@@ -58,6 +65,12 @@ test "Metadata" {
}
}
+test "Metadata - Invalid" {
+ if metadata :contains "INBOX" "/frop" "" {
+ test_fail "erroneously found a value for \"/frop\"";
+ }
+}
+
test "ServermetadataExists - None exist" {
if servermetadataexists "/private/frop" {
test_fail "servermetadataexists confirms existence of unknown annotation";
@@ -86,7 +99,14 @@ test "ServermetadataExists - One exists" {
test "ServermetadataExists - All exist" {
if not servermetadataexists
["/private/frop", "/private/friep", "/private/frml"] {
- test_fail "metadataexists fails to recognize annotations";
+ test_fail "servermetadataexists fails to recognize annotations";
+ }
+}
+
+test "ServermetadataExists - Invalid" {
+ if servermetadataexists
+ ["frop", "/private/friep", "/private/frml"] {
+ test_fail "servermetadataexists accepted invalid annotation name";
}
}
@@ -112,3 +132,10 @@ test "Servermetadata" {
test_fail "unexpected match for /private/frml";
}
}
+
+test "Servermetadata - Invalid" {
+ if servermetadata :contains "/frop" "" {
+ test_fail "erroneously found a value for \"/frop\"";
+ }
+}
+
--
GitLab