diff --git a/src/lib-sieve/sieve-error.c b/src/lib-sieve/sieve-error.c index 44bb0cc400477a4d3e98bd3acfa539fd1d79e643..a2247787e5dafc501aa81007611b462e12911f34 100644 --- a/src/lib-sieve/sieve-error.c +++ b/src/lib-sieve/sieve-error.c @@ -168,6 +168,12 @@ void sieve_error_handler_unref(struct sieve_error_handler **ehandler) *ehandler = NULL; } +void sieve_error_handler_reset(struct sieve_error_handler *ehandler) +{ + ehandler->errors = 0; + ehandler->warnings = 0; +} + /* Output errors directly to stderror (merge this with logfile below?) */ static void sieve_stderr_verror diff --git a/src/lib-sieve/sieve-error.h b/src/lib-sieve/sieve-error.h index e3a02ac630e9f46ffe0af8b7fa3888917392e7c3..5d25c013859e383d451d78750ec8e3c80d02c633 100644 --- a/src/lib-sieve/sieve-error.h +++ b/src/lib-sieve/sieve-error.h @@ -121,6 +121,8 @@ bool sieve_errors_more_allowed(struct sieve_error_handler *ehandler); void sieve_error_handler_ref(struct sieve_error_handler *ehandler); void sieve_error_handler_unref(struct sieve_error_handler **ehandler); +void sieve_error_handler_reset(struct sieve_error_handler *ehandler); + /* * Error handlers */ diff --git a/src/lib-sieve/sieve-validator.c b/src/lib-sieve/sieve-validator.c index 01f81bb4a0cb47137fed463a29a1d599c0eca140..f6f38ff8adda2969595488b7361393364c3950b6 100644 --- a/src/lib-sieve/sieve-validator.c +++ b/src/lib-sieve/sieve-validator.c @@ -826,6 +826,7 @@ static bool sieve_validate_command enum sieve_ast_type ast_type = sieve_ast_node_type(cmd_node); bool result = TRUE; struct sieve_command_registration *cmd_reg; + const struct sieve_command *command = NULL; i_assert( ast_type == SAT_TEST || ast_type == SAT_COMMAND ); @@ -835,7 +836,7 @@ static bool sieve_validate_command (valdtr, cmd_node->identifier); if ( cmd_reg != NULL && cmd_reg->command != NULL ) { - const struct sieve_command *command = cmd_reg->command; + command = cmd_reg->command; /* Identifier = "" when the command was previously marked as unknown */ if ( *(command->identifier) != '\0' ) { @@ -899,14 +900,17 @@ static bool sieve_validate_command * Descend further into the AST */ - /* Tests */ - if ( result || sieve_errors_more_allowed(valdtr->ehandler) ) - result = sieve_validate_test_list(valdtr, cmd_node) && result; + if ( command != NULL ) { + /* Tests */ + if ( command->subtests > 0 && + (result || sieve_errors_more_allowed(valdtr->ehandler)) ) + result = sieve_validate_test_list(valdtr, cmd_node) && result; - /* Command block */ - if ( ast_type == SAT_COMMAND && - (result || sieve_errors_more_allowed(valdtr->ehandler)) ) - result = sieve_validate_block(valdtr, cmd_node) && result; + /* Command block */ + if ( command->block_allowed && ast_type == SAT_COMMAND && + (result || sieve_errors_more_allowed(valdtr->ehandler)) ) + result = sieve_validate_block(valdtr, cmd_node) && result; + } return result; } diff --git a/src/testsuite/tests/compile/errors.svtest b/src/testsuite/tests/compile/errors.svtest index ac841bd80db8c0278fe034da88110fd2ef3df638..28e709856050bcaecd8f1f1bc58b569ccb85837e 100644 --- a/src/testsuite/tests/compile/errors.svtest +++ b/src/testsuite/tests/compile/errors.svtest @@ -3,64 +3,157 @@ require "vnd.dovecot.testsuite"; require "relational"; require "comparator-i;ascii-numeric"; +/* + * Errors triggered in the compiled scripts are pretty reduntant over the + * tested commands, but we want to be thorough. + */ + +/* + * Header test + */ + test "Header errors" { if test_compile "errors/header.sieve" { test_fail "compile should have failed."; } - if not test_error :count "eq" :comparator "i;ascii-numeric" "11" { + if not test_error :count "eq" :comparator "i;ascii-numeric" "10" { test_fail "wrong number of errors reported"; } if not test_error :index 1 :matches - "unknown * ':isnot' for * header test *" { + "unknown * ':all' for * header test *" { test_fail "error 1 is invalid"; } - if not test_error :index 2 :matches - ":comparator * requires one string argument, * number *" { + if not test_error :index 2 :matches + "*header test * string list * 1 (header names), but * number *" { test_fail "error 2 is invalid"; } if not test_error :index 3 :matches - "unknown tagged argument ':all' * header test *" { + "*header test * string list * 2 (key list), * number *" { test_fail "error 3 is invalid"; - } + } - if not test_error :index 4 :matches - "*header test * string list * 2 (key list), * number *" { + if not test_error :index 4 :matches + "* unexpected tagged argument ':tag' while *" { test_fail "error 4 is invalid"; } if not test_error :index 5 :matches - "*header test expects * string list * 1 (header names), but * number *" { + "* header test requires 2 *, but 1 *" { test_fail "error 5 is invalid"; } if not test_error :index 6 :matches - "* unexpected tagged argument ':tag' while *" { + "* header test requires 2 *, but 0 *" { test_fail "error 6 is invalid"; } if not test_error :index 7 :matches - "* header test requires 2 *, but 1 *" { + "*header test accepts no sub-tests* are specified*" { test_fail "error 7 is invalid"; } if not test_error :index 8 :matches - "* header test requires 2 *, but 0 *" { + "* use test 'header' * command*" { test_fail "error 8 is invalid"; } if not test_error :index 9 :matches - "*unknown * ':hufter' * header test *" { + "* use test 'header' * command*" { test_fail "error 9 is invalid"; } - if not test_error :index 10 :matches - "*header test accepts no sub-tests, but tests * anyway" { - test_fail "error 10 is invalid"; + if test_error :index 4 :contains "radish" { + test_fail "error test matched nonsense"; } } +/* + * Address test + */ + + +test "Address errors" { + if test_compile "errors/address.sieve" { + test_fail "compile should have failed."; + } + + if not test_error :count "eq" :comparator "i;ascii-numeric" "9" { + test_fail "wrong number of errors reported"; + } + + if not test_error :index 1 :matches + "*unknown * ':nonsense' * address test*" { + test_fail "error 1 is invalid"; + } + + if not test_error :index 2 :matches + "*address test expects *string list * 1 (header list),* number * found*" { + test_fail "error 2 is invalid"; + } + + if not test_error :index 3 :matches + "*address test expects *string list * 2 (key list),* number * found*" { + test_fail "error 3 is invalid"; + } + if not test_error :index 4 :matches + "*unexpected *':is' * address test*" { + test_fail "error 4 is invalid"; + } + + if not test_error :index 5 :matches + "*address test * 2 positional arg*, but 1*" { + test_fail "error 5 is invalid"; + } + + if not test_error :index 6 :matches + "*address test * 2 positional arg*, but 0*" { + test_fail "error 6 is invalid"; + } + + if not test_error :index 7 :matches + "*'frop' *not allowed *address test*" { + test_fail "error 7 is invalid"; + } + + if not test_error :index 8 :matches + "*'frop' *not allowed *address test*" { + test_fail "error 8 is invalid"; + } + + if test_error :index 23 :contains "radish" { + test_fail "error test matched nonsense"; + } +} + +/* + * If command + */ + +test "If errors (FIXME: count only)" { + if test_compile "errors/if.sieve" { + test_fail "compile should have failed."; + } + + if not test_error :count "eq" :comparator "i;ascii-numeric" "12" { + test_fail "wrong number of errors reported"; + } +} + +/* + * Require command + */ + +test "Require errors (FIXME: count only)" { + if test_compile "errors/require.sieve" { + test_fail "compile should have failed."; + } + + if not test_error :count "eq" :comparator "i;ascii-numeric" "12" { + test_fail "wrong number of errors reported"; + } +} diff --git a/src/testsuite/tests/compile/errors/address.sieve b/src/testsuite/tests/compile/errors/address.sieve new file mode 100644 index 0000000000000000000000000000000000000000..917da1f8e6f94bf9c4c18554f0a45b900e536aa2 --- /dev/null +++ b/src/testsuite/tests/compile/errors/address.sieve @@ -0,0 +1,71 @@ +require "comparator-i;ascii-numeric"; + +/* + * Address test errors + * + * Total count: 8 (+1 = 9) + */ + +/* + * Command structure + */ + +# Invalid tag +if address :nonsense :comparator "i;ascii-casemap" :localpart "From" "nico" { + discard; +} + +# Invalid first argument +if address :is :comparator "i;ascii-numeric" :localpart 45 "nico" { + discard; +} + +# Invalid second argument +if address :is :comparator "i;ascii-numeric" :localpart "From" 45 { + discard; +} + +# Invalid second argument +if address :comparator "i;ascii-numeric" :localpart "From" :is { + discard; +} + +# Missing second argument +if address :is :comparator "i;ascii-numeric" :localpart "From" { + discard; +} + +# Missing arguments +if address :is :comparator "i;ascii-numeric" :localpart { + discard; +} + +# Not an error +if address :localpart :is :comparator "i;ascii-casemap" "from" ["frop", "frop"] { + discard; +} + +/* + * Specified headers must contain addresses + */ + +# Invalid header +if address :is "frop" "frml" { + keep; +} + +# Not an error +if address :is "reply-to" "frml" { + keep; +} + +# Invalid header (#2) +if address :is ["to", "frop"] "frml" { + keep; +} + +# Not an error +if address :is ["to", "reply-to"] "frml" { + keep; +} + diff --git a/src/testsuite/tests/compile/errors/header.sieve b/src/testsuite/tests/compile/errors/header.sieve index ac6108ba131741a96fe3432239da621b7e5975bf..1c87f94252bd6c77709be7990c7e5ab7eddbb631 100644 --- a/src/testsuite/tests/compile/errors/header.sieve +++ b/src/testsuite/tests/compile/errors/header.sieve @@ -1,44 +1,57 @@ require "comparator-i;ascii-numeric"; -if header :isnot :comparator "i;ascii-casemap" "From" "nico" { - discard; -} +/* + * Compile errors for the header test + * + * Total errors: 9 (+1 validation failed msg = 10) + */ -if header :is :comparator 45 "From" "nico" { - discard; +# Unknown tagged argument +if header :all :comparator "i;ascii-casemap" "From" "nico" { + keep; } -if header :all :comparator "i;ascii-numeric" { - keep; +# Wrong first argument +if header :is :comparator "i;ascii-numeric" 45 "nico" { + keep; } +# Wrong second argument if header :is :comparator "i;ascii-numeric" "From" 45 { discard; } -if header :is :comparator "i;ascii-numeric" 45 "nico" { - discard; -} - +# Wrong second argument if header :is :comparator "i;ascii-numeric" "From" :tag { - discard; + stop; } +# Missing second argument if header :is :comparator "i;ascii-numeric" "From" { - discard; + stop; } +# Missing arguments if header :is :comparator "i;ascii-numeric" { - discard; + keep; } +# Not an error if header :is :comparator "i;ascii-casemap" "frop" ["frop", "frop"] { discard; } -if header :hufter :is :comparator "i;ascii-casemap" "frop" ["frop", "frop"] { - discard; +# Spurious sub-test +if header "frop" "frop" true { + discard; } -if header "frop" "frop" true { +# Test used as command with block +header "frop" "frop" { + discard; } + +# Test used as command +header "frop" "frop"; + + diff --git a/src/testsuite/tests/compile/errors/if.sieve b/src/testsuite/tests/compile/errors/if.sieve new file mode 100644 index 0000000000000000000000000000000000000000..fa741c9c6716d24984e05fe599edc58b676dfffa --- /dev/null +++ b/src/testsuite/tests/compile/errors/if.sieve @@ -0,0 +1,78 @@ +/* + * If command errors + * + * Total errors: 11 (+1 = 12) + */ + +# Spurious argument +if "frop" true {} + +# Spurious argument +elsif "frop" true {} + +# Spurious string list +if [ "false", "false", "false" ] false { + stop; +} + +# No block +if true; + +# No test +if { + keep; +} + +# Spurious test list +if ( false, false, true ) { + keep; +} + +stop; + +# If-less else +else { + keep; +} + +# Not an error +if true { + keep; +} + +stop; + +# If-less if structure (should produce only one error) +elsif true { + keep; +} +elsif true { + keep; +} +else { +} + +# Elsif after else +if true { + keep; +} else { + stop; +} elsif true { + stop; +} + +# If used as test +if if true { +} + +# Else if in stead of elsif + +if true { + stop; +} else if false { + keep; +} + + + + diff --git a/src/testsuite/tests/compile/errors/require.sieve b/src/testsuite/tests/compile/errors/require.sieve new file mode 100644 index 0000000000000000000000000000000000000000..93508ad58651a4ede3a50476693337653834f981 --- /dev/null +++ b/src/testsuite/tests/compile/errors/require.sieve @@ -0,0 +1,39 @@ +/* + * Require errors + * + * Total errors: 11 (+1 = 12) + */ + +# Not an error +require "fileinto"; + +# Missing argument +require; + +# Too many arguments +require "fileinto" "vacation"; + +# Invalid argument +require 45; + +# Invalid extensions (3 errors) +require ["_frop", "_friep", "_frml"]; + +# Invalid arguments +require "dovecot.test" true; + +# Invalid extension +require "_frop"; + +# Spurious command block +require "fileinto" { + keep; +} + +# Nested require +if true { + require "relional"; +} + +# Require after other command than require +require "copy"; diff --git a/src/testsuite/testsuite-common.c b/src/testsuite/testsuite-common.c index 034829f553ce0c7ec5c32c8c0fc951dd35b28c9b..39a5f65ef4b7d519571046f665d337261cb775ed 100644 --- a/src/testsuite/testsuite-common.c +++ b/src/testsuite/testsuite-common.c @@ -335,6 +335,8 @@ static void testsuite_script_clear_messages(void) ("testsuite_script_messages", 8192); p_array_init(&_testsuite_script_errors, _testsuite_scriptmsg_pool, 128); + + sieve_error_handler_reset(test_script_ehandler); } void testsuite_script_get_error_init(void) @@ -359,10 +361,10 @@ const char *testsuite_script_get_error_next(bool location) static void testsuite_script_init(void) { - testsuite_script_clear_messages(); - test_script_ehandler = _testsuite_script_ehandler_create(); sieve_error_handler_accept_infolog(test_script_ehandler, TRUE); + + testsuite_script_clear_messages(); } bool testsuite_script_compile(const char *script_path)