[PATCH 03/17] qapi: Fix crash on stray double quote character

Markus Armbruster posted 17 patches 2 years, 9 months ago
[PATCH 03/17] qapi: Fix crash on stray double quote character
Posted by Markus Armbruster 2 years, 9 months ago
When the lexer chokes on a stray character, its shows the characters
until the next structural character in the error message.  It uses a
regular expression to match a non-empty string of non-structural
characters.  Bug: the regular expression treats '"' as structural.
When the lexer chokes on '"', the match fails, and trips
must_match()'s assertion.  Fix the regular expression.

Fixes: 14c32795024c (qapi: Improve reporting of lexical errors)
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/parser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 878f90b458..7b49d3ab05 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -346,7 +346,7 @@ def accept(self, skip_comment: bool = True) -> None:
             elif not self.tok.isspace():
                 # Show up to next structural, whitespace or quote
                 # character
-                match = must_match('[^[\\]{}:,\\s\'"]+',
+                match = must_match('[^[\\]{}:,\\s\']+',
                                    self.src[self.cursor-1:])
                 raise QAPIParseError(self, "stray '%s'" % match.group(0))
 
-- 
2.39.2
Re: [PATCH 03/17] qapi: Fix crash on stray double quote character
Posted by Juan Quintela 2 years, 9 months ago
Markus Armbruster <armbru@redhat.com> wrote:
> When the lexer chokes on a stray character, its shows the characters
> until the next structural character in the error message.

You have a problem

> It uses a regular expression

You use regular expresions.

Now you have two problems.

Yes, I had to do it.

> to match a non-empty string of non-structural
> characters.  Bug: the regular expression treats '"' as structural.
> When the lexer chokes on '"', the match fails, and trips
> must_match()'s assertion.  Fix the regular expression.
>
> Fixes: 14c32795024c (qapi: Improve reporting of lexical errors)
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>
Re: [PATCH 03/17] qapi: Fix crash on stray double quote character
Posted by Markus Armbruster 2 years, 9 months ago
Juan Quintela <quintela@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> wrote:
>> When the lexer chokes on a stray character, its shows the characters
>> until the next structural character in the error message.
>
> You have a problem
>
>> It uses a regular expression
>
> You use regular expresions.
>
> Now you have two problems.
>
> Yes, I had to do it.

:)

Regular expressions go bad when pushed too far.

Lexers are a perfect fit for them.  However, the bug I fix here isn't in
the lexer proper, it's in "make the error message nice" code.

>> to match a non-empty string of non-structural
>> characters.  Bug: the regular expression treats '"' as structural.
>> When the lexer chokes on '"', the match fails, and trips
>> must_match()'s assertion.  Fix the regular expression.
>>
>> Fixes: 14c32795024c (qapi: Improve reporting of lexical errors)
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>

Thanks!