From 766303db802b771743e1a5962bbd6eccd8df3719 Mon Sep 17 00:00:00 2001 From: Stephan Bosch <stephan@rename-it.nl> Date: Sat, 23 Jan 2010 00:37:21 +0100 Subject: [PATCH] Fixed bugs in parser recovery and added corresponding tests in the test suite. --- Makefile.am | 1 + src/lib-sieve/sieve-parser.c | 64 ++++++++++++------- tests/compile/recover.svtest | 50 +++++++++++++++ tests/compile/recover/commands-endblock.sieve | 24 +++++++ .../compile/recover/commands-semicolon.sieve | 7 ++ tests/compile/recover/tests-endcomma.sieve | 10 +++ 6 files changed, 133 insertions(+), 23 deletions(-) create mode 100644 tests/compile/recover.svtest create mode 100644 tests/compile/recover/commands-endblock.sieve create mode 100644 tests/compile/recover/commands-semicolon.sieve create mode 100644 tests/compile/recover/tests-endcomma.sieve diff --git a/Makefile.am b/Makefile.am index db4511703..37f885528 100644 --- a/Makefile.am +++ b/Makefile.am @@ -39,6 +39,7 @@ test_cases = \ tests/compile/compile.svtest \ tests/compile/errors.svtest \ tests/compile/warnings.svtest \ + tests/compile/recover.svtest \ tests/execute/errors.svtest \ tests/execute/actions.svtest \ tests/execute/smtp.svtest \ diff --git a/src/lib-sieve/sieve-parser.c b/src/lib-sieve/sieve-parser.c index c78efc8e9..d4319336b 100644 --- a/src/lib-sieve/sieve-parser.c +++ b/src/lib-sieve/sieve-parser.c @@ -157,9 +157,13 @@ static int sieve_parse_arguments necessarily error-free state */ /* Parse arguments */ - while ( arg_present && result > 0 && - (parser->valid || sieve_errors_more_allowed(parser->ehandler)) ) { + while ( arg_present && result > 0 ) { struct sieve_ast_argument *arg; + + if ( !parser->valid && !sieve_errors_more_allowed(parser->ehandler) ) { + result = 0; + break; + } switch ( sieve_lexer_current_token(lexer) ) { @@ -184,10 +188,14 @@ static int sieve_parse_arguments sieve_lexer_skip_token(lexer); - while ( !add_failed && sieve_lexer_current_token(lexer) == STT_COMMA && - (parser->valid || sieve_errors_more_allowed(parser->ehandler)) ) { - + while ( !add_failed && sieve_lexer_current_token(lexer) == STT_COMMA ) { sieve_lexer_skip_token(lexer); + + /* Check parser status */ + if ( !parser->valid && !sieve_errors_more_allowed(parser->ehandler) ) { + result = sieve_parser_recover(parser, STT_RSQUARE); + break; + } if ( sieve_lexer_current_token(lexer) == STT_STRING ) { /* Add the string to the list */ @@ -339,10 +347,15 @@ static int sieve_parse_arguments if ( (result=sieve_parse_arguments(parser, test, depth+1)) > 0 ) { /* More tests ? */ - while ( sieve_lexer_current_token(lexer) == STT_COMMA && - (parser->valid && sieve_errors_more_allowed(parser->ehandler)) ) { + while ( sieve_lexer_current_token(lexer) == STT_COMMA ) { sieve_lexer_skip_token(lexer); - + + /* Check parser status */ + if ( !parser->valid && !sieve_errors_more_allowed(parser->ehandler) ) { + result = sieve_parser_recover(parser, STT_RBRACKET); + break; + } + /* Test starts with identifier */ if ( sieve_lexer_current_token(lexer) == STT_IDENTIFIER ) { test = sieve_ast_test_create @@ -355,7 +368,6 @@ static int sieve_parse_arguments /* Parse test arguments, which may include more tests (recurse) */ if ( (result=sieve_parse_arguments(parser, test, depth+1)) <= 0 ) { if ( result < 0 ) return result; - result = sieve_parser_recover(parser, STT_RBRACKET); break; } @@ -430,12 +442,20 @@ static int sieve_parse_commands int result = TRUE; while ( result > 0 && - sieve_lexer_current_token(lexer) == STT_IDENTIFIER && - (parser->valid || sieve_errors_more_allowed(parser->ehandler)) ) { - struct sieve_ast_node *command = - sieve_ast_command_create - (block, sieve_lexer_token_ident(lexer), - sieve_lexer_current_line(parser->lexer)); + sieve_lexer_current_token(lexer) == STT_IDENTIFIER ) { + struct sieve_ast_node *command; + + /* Check parser status */ + if ( !parser->valid && !sieve_errors_more_allowed(parser->ehandler) ) { + result = sieve_parser_recover(parser, STT_SEMICOLON); + break; + } + + /* Create command node */ + command = sieve_ast_command_create + (block, sieve_lexer_token_ident(lexer), + sieve_lexer_current_line(parser->lexer)); + sieve_lexer_skip_token(lexer); if ( command == NULL ) { sieve_parser_error(parser, @@ -444,11 +464,6 @@ static int sieve_parse_commands return -1; } - /* Defined state */ - result = TRUE; - - sieve_lexer_skip_token(lexer); - result = sieve_parse_arguments(parser, command, 1); /* Check whether the command is properly terminated @@ -471,7 +486,9 @@ static int sieve_parse_commands } /* Don't bother to continue if we are not in a defined state */ - if ( result <= 0 ) return result; + if ( result <= 0 ) { + return result; + } switch ( sieve_lexer_current_token(lexer) ) { @@ -510,7 +527,7 @@ static int sieve_parse_commands } else { if ( result < 0 ) return result; - if ( (result=sieve_parser_recover(parser, STT_RCURLY)) == 0 ) + if ( (result=sieve_parser_recover(parser, STT_RCURLY)) > 0 ) sieve_lexer_skip_token(lexer); } @@ -643,8 +660,9 @@ static int sieve_parser_recover /* Special case: COMMAND */ if (end_token == STT_SEMICOLON && - sieve_lexer_current_token(lexer) == STT_LCURLY) + sieve_lexer_current_token(lexer) == STT_LCURLY) { return TRUE; + } /* End not found before eof or end of surrounding grammatical structure */ diff --git a/tests/compile/recover.svtest b/tests/compile/recover.svtest new file mode 100644 index 000000000..f6010a022 --- /dev/null +++ b/tests/compile/recover.svtest @@ -0,0 +1,50 @@ +require "vnd.dovecot.testsuite"; + +require "relational"; +require "comparator-i;ascii-numeric"; + +/* + * Test parser's recover capability + */ + +/* + * Commands + */ + +/* Missing semicolon */ + +test "Missing semicolons" { + if test_script_compile "recover/commands-semicolon.sieve" { + test_fail "compile should have failed."; + } + + if not test_error :count "eq" :comparator "i;ascii-numeric" "3" { + test_fail "wrong number of errors reported"; + } +} + +/* End of block recovery*/ + +test "Missing semicolon at end of block" { + if test_script_compile "recover/commands-endblock.sieve" { + test_fail "compile should have failed."; + } + + if not test_error :count "eq" :comparator "i;ascii-numeric" "4" { + test_fail "wrong number of errors reported"; + } +} + +/* + * Tests + */ + +test "Spurious comma at end of test list" { + if test_script_compile "recover/tests-endcomma.sieve" { + test_fail "compile should have failed."; + } + + if not test_error :count "eq" :comparator "i;ascii-numeric" "3" { + test_fail "wrong number of errors reported"; + } +} diff --git a/tests/compile/recover/commands-endblock.sieve b/tests/compile/recover/commands-endblock.sieve new file mode 100644 index 000000000..2868c5413 --- /dev/null +++ b/tests/compile/recover/commands-endblock.sieve @@ -0,0 +1,24 @@ +if true { + if true { + keep + } +} + +if true { + keep, + keep +} + +if true { + if anyof(true,true,false) { + keep; + } +} + +if true { + if anyof(true,true,false) { + keep; + discard + } +} + diff --git a/tests/compile/recover/commands-semicolon.sieve b/tests/compile/recover/commands-semicolon.sieve new file mode 100644 index 000000000..225403cac --- /dev/null +++ b/tests/compile/recover/commands-semicolon.sieve @@ -0,0 +1,7 @@ +keep; +discard; +keep +redirect "frop@frop.nl"; +discard; +keep +redirect "frml@frop.nl"; diff --git a/tests/compile/recover/tests-endcomma.sieve b/tests/compile/recover/tests-endcomma.sieve new file mode 100644 index 000000000..6daf11789 --- /dev/null +++ b/tests/compile/recover/tests-endcomma.sieve @@ -0,0 +1,10 @@ +if true { +if true { +if anyof(true,true,true,) { +}}} + +if true { +if anyof(true,true) { +if anyof(true,true,true,) { +if anyof(true,true,true) { +}}}} -- GitLab