[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Anthony Liguori
Subject: [Qemu-devel] Re: [PATCH 08/11] json-lexer: reset the lexer state on an invalid token
Date: Mon, 14 Mar 2011 15:30:33 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8

On 03/14/2011 03:12 PM, Luiz Capitulino wrote:
On Mon, 14 Mar 2011 14:43:48 -0500
Anthony Liguori<address@hidden>  wrote:

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;
           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

{"QMP": {"version": {"qemu": {"micro": 50, "minor": 13, "major": 0},
"package": ""}, "capabilities": []}}
{"error": {"class": "JSONParseError", "data": {"message": "Missing value
in dict"}}}
How do you handle it? Do you check the return of json_message_parser_feed()?

If that's the case, then the real problem in the current server is that we
use qemu's chardev interface and its read handler doesn't allow for
signaling errors. I did not consider not using it.

By looking at your branch I have the impression you wrote your own stuff,
am I right? If yes, doesn't it duplicate the chardev implementation?

No, that test was with the chardev interface. There is both a chardev server and a unix domain socket server.

I'm not really sure why the current server isn't working correctly. I'd have to investigate.

    { "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.
I'm all for trying to recover, but we can't have varied responses for
bad input. It seems easier to just fail.

I think we need to make sure that we don't ever succeed in the face of bad input, right?

So far, none of the test cases (against the new QMP server) succeed given bad input.

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.
I can't reproduce it w/o this series applied:

{"QMP": {"version": {"qemu": {"micro": 50, "minor": 14, "major": 0}, "package": ""}, 
"capabilities": []}}
{ "execute": "not-a-valid-command" }
{"error": {"class": "CommandNotFound", "desc": "The command not-a-valid-command has not been found", 
"data": {"name": "not-a-valid-command"}}}

Curious, maybe I'm remembering this wrong then.  Let me dig in a big.


Anthony Liguori

reply via email to

[Prev in Thread] Current Thread [Next in Thread]