[Qemu-devel] Re: [PATCH 08/11] json-lexer: reset the lexer state on an invalid token

[Anthony Liguori]

On 03/14/2011 02:22 PM, Luiz Capitulino wrote:
> On Fri, 11 Mar 2011 15:00:46 -0600
> Anthony Liguori<address@hidden>  wrote:
>
> > Not everything handles errors from json parsing gracefully.  By at least
> > resetting the lexer, we'll start generating valid tokens again and hopefully
> > recover the stream.
> >
> > Signed-off-by: Anthony Liguori<address@hidden>
> >
> > diff --git a/json-lexer.c b/json-lexer.c
> > index c736f42..834d7af 100644
> > --- a/json-lexer.c
> > +++ b/json-lexer.c
> > @@ -303,6 +303,9 @@ static int json_lexer_feed_char(JSONLexer *lexer, char ch)
> >               new_state = IN_START;
> >               break;
> >           case ERROR:
> > +            QDECREF(lexer->token);
> > +            lexer->token = qstring_new();
> > +            new_state = IN_START;
> >               return -EINVAL;
> This makes the parser accept broken input like:
>
>    { "execute": xxxxx }
>    {"return": {}}

This is a bug in the current QMP server.  Here's how my new QMP server 
responds:

{"QMP": {"version": {"qemu": {"micro": 50, "minor": 13, "major": 0}, 
"package": ""}, "capabilities": []}}
{"error": {"class": "JSONParseError", "data": {"message": "Missing value 
in dict"}}}

>    { "execute": _ }
>    {"return": {}}

Likewise, the new QMP server does not respond to this at all (which 
confuses me TBH).

> Today, it handles this kind of input correctly:
>
>    { "execute": xxxxx }
>    {"error": {"class": "JSONParsing", "desc": "Invalid JSON syntax", "data": 
> {}}}

The parser rejects this verses trying to get what it can out of it and 
passing that to QMP.  The idea here is to be more graceful in dealing 
with bad input and trying to recover.

> Although it also accepts broken stuff today, like:
>
>   { "execute": ___"query-block" }

This is really the server, not the parser.  The new server doesn't 
accept this.

I guess QMP today just ignores the incoming QObject in capabilities mode 
and always returns {}.  You'll see the same thing with:

{ "execute": "not-a-valid-command" }
{"return": {}}

But once you're in command mode, it does the right thing.

Regards,

Anthony Liguori

> >           default:
> >               break;