Repository: tonioo/sievelib

Issue:
Parser does insufficient bracket matching
The following script crashes the parser instead of generating a parse error:

```}```

Upon investigation, the problem seems to be that the parser doesn't always check if the current context allows for closing a bracket (either of `)]}`), and often just assumes we're in the right context when one of them is encountered.

A solution could be for the parser to maintain a stack of open brackets (e.g. a list `['(', '{', '[']`) and `push` to it on each opening bracket, and when closing one, `pop`s the last bracket and confirm it's the same as the one we're closing, otherwise issuing a parse error.

I shall try to implement such a solution and file it as a PR. But maybe another solution would be preferred?

Cf. derula/strainer#15

Discussion:

Patch:
diff --git a/sievelib/parser.py b/sievelib/parser.py
index a92f5f1..219ca94 100755
--- a/sievelib/parser.py
+++ b/sievelib/parser.py
@@ -142,7 +142,7 @@ class Parser(object):
         self.__curcommand = None
         self.__curstringlist = None
         self.__expected = None
-        self.__opened_blocks = 0
+        self.__expected_brackets = []
         RequireCommand.loaded_extensions = []
 
     def __set_expected(self, *args, **kwargs):
@@ -153,6 +153,28 @@ class Parser(object):
         """
         self.__expected = args
 
+    def __push_expected_bracket(self, ttype, tvalue):
+        """Append a new expected bracket.
+
+        Next time a bracket is closed, it must match the one provided here.
+        """
+        self.__expected_brackets.append((ttype, tvalue))
+
+    def __pop_expected_bracket(self, ttype, tvalue):
+        """Drop the last expected bracket.
+
+        If the given bracket doesn't match the dropped expected bracket,
+        or if no bracket is expected at all, a ParseError will be raised.
+        """
+        try:
+            etype, evalue = self.__expected_brackets.pop()
+        except IndexError:
+            raise ParseError("unexpected closing bracket %s (none opened)" %
+                             (tvalue,))
+        if ttype != etype:
+            raise ParseError("unexpected closing bracket %s (expected %s)" %
+                             (tvalue, evalue))
+
     def __up(self, onlyrecord=False):
         """Return to the current command's parent
 
@@ -251,6 +273,7 @@ class Parser(object):
             self.__set_expected("string")
             return True
         if ttype == "right_bracket":
+            self.__pop_expected_bracket(ttype, tvalue)
             self.__curcommand.check_next_arg("stringlist", self.__curstringlist)
             self.__cstate = self.__arguments
             return self.__check_command_completion()
@@ -275,6 +298,7 @@ class Parser(object):
             return self.__curcommand.check_next_arg(ttype, tvalue.decode("ascii"))
 
         if ttype == "left_bracket":
+            self.__push_expected_bracket("right_bracket", b'}')
             self.__cstate = self.__stringlist
             self.__curstringlist = []
             self.__set_expected("string")
@@ -314,6 +338,7 @@ class Parser(object):
             return self.__check_command_completion(testsemicolon=False)
 
         if ttype == "left_parenthesis":
+            self.__push_expected_bracket("right_parenthesis", b')')
             self.__set_expected("identifier")
             return True
 
@@ -322,6 +347,7 @@ class Parser(object):
             return True
 
         if ttype == "right_parenthesis":
+            self.__pop_expected_bracket(ttype, tvalue)
             self.__up()
             return True
 
@@ -348,8 +374,8 @@ class Parser(object):
         """
         if self.__cstate is None:
             if ttype == "right_cbracket":
+                self.__pop_expected_bracket(ttype, tvalue)
                 self.__up()
-                self.__opened_blocks -= 1
                 self.__cstate = None
                 return True
 
@@ -376,7 +402,7 @@ class Parser(object):
             return True
 
         if ttype == "left_cbracket":
-            self.__opened_blocks += 1
+            self.__push_expected_bracket("right_cbracket", b'}')
             self.__cstate = None
             return True
 
@@ -438,8 +464,8 @@ class Parser(object):
                         % (tvalue.decode(), text.decode()[self.lexer.pos])
                     )
                     raise ParseError(msg)
-            if self.__opened_blocks:
-                self.__set_expected("right_cbracket")
+            if self.__expected_brackets:
+                self.__set_expected(self.__expected_brackets[-1][0])
             if self.__expected is not None:
                 raise ParseError("end of script reached while %s expected" %
                                  "|".join(self.__expected))


Test patch:
diff --git a/sievelib/tests/test_parser.py b/sievelib/tests/test_parser.py
index 8890eac..f41cf86 100644
--- a/sievelib/tests/test_parser.py
+++ b/sievelib/tests/test_parser.py
@@ -589,6 +589,16 @@ if header :is "Sender" "me@example.com" {
 
 """)
 
+    def test_nonopened_parenthesis(self):
+        self.compilation_ko(b"""
+if header :is "Sender" "me@example.com") {
+    discard;
+}
+""")
+
+    def test_nonopened_block2(self):
+        self.compilation_ko(b"""}""")
+
     def test_unknown_token(self):
         self.compilation_ko(b"""
 if header :is "Sender" "Toto" & header :contains "Cc" "Tata" {
@@ -599,6 +609,9 @@ if header :is "Sender" "Toto" & header :contains "Cc" "Tata" {
     def test_empty_string_list(self):
         self.compilation_ko(b"require [];")
 
+    def test_unopened_string_list(self):
+        self.compilation_ko(b'require "fileinto"];')
+
     def test_unclosed_string_list(self):
         self.compilation_ko(b'require ["toto", "tata";')
 
@@ -834,7 +847,7 @@ class VariablesCommands(SieveTest):
         self.compilation_ok(b"""require ["variables"];
 
 set "matchsub" "testsubject";
-        
+
 if allof (
   header :contains ["Subject"] "${header}"
 )