| Message ID | 1374842387-17146-7-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/26/2013 06:39 AM, Markus Armbruster wrote: > Fixes at least the following parser bugs: > > * accepts any token in place of a colon > > * treats comma as optional > > * crashes when closing braces or brackets are missing > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi.py | 40 ++++++++++++++++++++++++++--------- > tests/qapi-schema/missing-colon.err | 1 + > tests/qapi-schema/missing-colon.exit | 2 +- > tests/qapi-schema/missing-colon.out | 3 --- > tests/qapi-schema/missing-comma.err | 1 + > tests/qapi-schema/missing-comma.exit | 2 +- > tests/qapi-schema/missing-comma.out | 3 --- > tests/qapi-schema/unclosed-object.err | 2 +- > 8 files changed, 35 insertions(+), 19 deletions(-) > > > def get_values(self): > expr = [] > - while self.tok != ']': > + if self.tok == ']': > + self.accept() > + return expr > + if not self.tok in [ '{', '[', "'" ]: > + raise QAPISchemaError(self, 'Expected "{", "[", "]" or string') JSON allows primitives here, as in [ 1 ]; but I agree that for the purposes of our schema we will always be taking a string or complex object whenever we have a list. Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@redhat.com> writes: > On 07/26/2013 06:39 AM, Markus Armbruster wrote: >> Fixes at least the following parser bugs: >> >> * accepts any token in place of a colon >> >> * treats comma as optional >> >> * crashes when closing braces or brackets are missing >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> scripts/qapi.py | 40 ++++++++++++++++++++++++++--------- >> tests/qapi-schema/missing-colon.err | 1 + >> tests/qapi-schema/missing-colon.exit | 2 +- >> tests/qapi-schema/missing-colon.out | 3 --- >> tests/qapi-schema/missing-comma.err | 1 + >> tests/qapi-schema/missing-comma.exit | 2 +- >> tests/qapi-schema/missing-comma.out | 3 --- >> tests/qapi-schema/unclosed-object.err | 2 +- >> 8 files changed, 35 insertions(+), 19 deletions(-) >> > >> >> def get_values(self): >> expr = [] >> - while self.tok != ']': >> + if self.tok == ']': >> + self.accept() >> + return expr >> + if not self.tok in [ '{', '[', "'" ]: >> + raise QAPISchemaError(self, 'Expected "{", "[", "]" or string') > > JSON allows primitives here, as in [ 1 ]; but I agree that for the > purposes of our schema we will always be taking a string or complex > object whenever we have a list. The lexer doesn't recognize any atoms but strings. If we change that, the syntax error messages need to be reviewed (not just this one). > Reviewed-by: Eric Blake <eblake@redhat.com>
On 07/26/2013 01:35 PM, Markus Armbruster wrote: >>> + if not self.tok in [ '{', '[', "'" ]: >>> + raise QAPISchemaError(self, 'Expected "{", "[", "]" or string') >> >> JSON allows primitives here, as in [ 1 ]; but I agree that for the >> purposes of our schema we will always be taking a string or complex >> object whenever we have a list. > > The lexer doesn't recognize any atoms but strings. If we change that, > the syntax error messages need to be reviewed (not just this one). Nah, I'm fine leaving the lexer as-is. It's okay that we parse only a subset of JSON, as long as our subset is expressive enough for our needs in QAPI. As it is, technically, we aren't QUITE parsing JSON, because our schema strings are marked with '' instead of "". And as long as we are extending our parser to take something slightly different than JSON, maybe we should teach it to tolerate trailing commas? On the other hand, the further we diverge from JSON, the more likely we are to have to maintain the parser ourselves instead of being able to reuse someone else's code.
diff --git a/scripts/qapi.py b/scripts/qapi.py index f72a0e3..a7feccb 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -91,24 +91,42 @@ class QAPISchema: def get_members(self): expr = OrderedDict() - while self.tok != '}': + if self.tok == '}': + self.accept() + return expr + if self.tok != "'": + raise QAPISchemaError(self, 'Expected string or "}"') + while True: key = self.val self.accept() - self.accept() # : + if self.tok != ':': + raise QAPISchemaError(self, 'Expected ":"') + self.accept() expr[key] = self.get_expr() - if self.tok == ',': + if self.tok == '}': self.accept() - self.accept() - return expr + return expr + if self.tok != ',': + raise QAPISchemaError(self, 'Expected "," or "}"') + self.accept() + if self.tok != "'": + raise QAPISchemaError(self, 'Expected string') def get_values(self): expr = [] - while self.tok != ']': + if self.tok == ']': + self.accept() + return expr + if not self.tok in [ '{', '[', "'" ]: + raise QAPISchemaError(self, 'Expected "{", "[", "]" or string') + while True: expr.append(self.get_expr()) - if self.tok == ',': + if self.tok == ']': self.accept() - self.accept() - return expr + return expr + if self.tok != ',': + raise QAPISchemaError(self, 'Expected "," or "]"') + self.accept() def get_expr(self): if self.tok == '{': @@ -117,9 +135,11 @@ class QAPISchema: elif self.tok == '[': self.accept() expr = self.get_values() - else: + elif self.tok == "'": expr = self.val self.accept() + else: + raise QAPISchemaError(self, 'Expected "{", "[" or string') return expr def parse_schema(fp): diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err index e69de29..9f2a355 100644 --- a/tests/qapi-schema/missing-colon.err +++ b/tests/qapi-schema/missing-colon.err @@ -0,0 +1 @@ +<stdin>:1:10: Expected ":" diff --git a/tests/qapi-schema/missing-colon.exit b/tests/qapi-schema/missing-colon.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/missing-colon.exit +++ b/tests/qapi-schema/missing-colon.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/missing-colon.out b/tests/qapi-schema/missing-colon.out index e67068c..e69de29 100644 --- a/tests/qapi-schema/missing-colon.out +++ b/tests/qapi-schema/missing-colon.out @@ -1,3 +0,0 @@ -[OrderedDict([('enum', None), ('data', ['good', 'bad', 'ugly'])])] -[None] -[] diff --git a/tests/qapi-schema/missing-comma.err b/tests/qapi-schema/missing-comma.err index e69de29..b0121b5 100644 --- a/tests/qapi-schema/missing-comma.err +++ b/tests/qapi-schema/missing-comma.err @@ -0,0 +1 @@ +<stdin>:2:3: Expected "," or "}" diff --git a/tests/qapi-schema/missing-comma.exit b/tests/qapi-schema/missing-comma.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/missing-comma.exit +++ b/tests/qapi-schema/missing-comma.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/missing-comma.out b/tests/qapi-schema/missing-comma.out index e3bd904..e69de29 100644 --- a/tests/qapi-schema/missing-comma.out +++ b/tests/qapi-schema/missing-comma.out @@ -1,3 +0,0 @@ -[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])] -['Status'] -[] diff --git a/tests/qapi-schema/unclosed-object.err b/tests/qapi-schema/unclosed-object.err index f9a9c2a..cc11626 100644 --- a/tests/qapi-schema/unclosed-object.err +++ b/tests/qapi-schema/unclosed-object.err @@ -1 +1 @@ -Crashed: <type 'exceptions.IndexError'> +<stdin>:1:19: Expected "," or "]"
Fixes at least the following parser bugs: * accepts any token in place of a colon * treats comma as optional * crashes when closing braces or brackets are missing Signed-off-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi.py | 40 ++++++++++++++++++++++++++--------- tests/qapi-schema/missing-colon.err | 1 + tests/qapi-schema/missing-colon.exit | 2 +- tests/qapi-schema/missing-colon.out | 3 --- tests/qapi-schema/missing-comma.err | 1 + tests/qapi-schema/missing-comma.exit | 2 +- tests/qapi-schema/missing-comma.out | 3 --- tests/qapi-schema/unclosed-object.err | 2 +- 8 files changed, 35 insertions(+), 19 deletions(-)