Bug 580449

This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux major release.  Product Management has requested further
review of this request by Red Hat Engineering, for potential inclusion in a Red
Hat Enterprise Linux Major release.  This request is not yet committed for
inclusion.

input unknow_cmd mixed with _, no response for the command
{"execute":yy_uu}

Looking into this issue, should fix it soon.

FYI: Not all bad inputs cause this, technically what happens is that we handle errors from the JSON parser but we don't handle errors from the JSON lexer.

So, an input like:

{ "execute": xxxxx }

Gets handled as expected, but your input:

{ "execute": _ }

Doesn't. The best solution is probably to just kill the connection.

Reverting Status and Devel Whiteboard, as the patches posted for this one fix a different BZ. This one is still an open issue.

The fix for bug #585009 has changed this behavior. Now, instead of stop responding the server just ignores bad characters, so things like this just
works:

  { "execute": ___"query-block" }

This makes me think this is like bug #586233 and maybe deserves the same destiny (ie. be moved to rhel6.1).

Paolo, you've said you were going to work on this, any update? Can I assign this to you?


For reference, here goes the two possible ways to fix this:

1. Just drop the connection

When the lexer sees a bad character, it returns an error but the monitor code doesn't do anything about it (monitor.c:4342). The best solution would be to
just drop the connection.

However, it seems to me that our qemu-char implementation doesn't has an easy way to do this... Possibly, we would have to write the proper interface for it, which is beyond rhel6.

2. Pass an error token to the parser

We can't drop the connection, but we do handle parser errors just fine (monitor.c:4259). So, a different solution would be to change the lexer to pass an error token to the parser and then it could return an error as soon as it sees the error token.

I think this is what Paolo was/is going to do, but I'm not sure whether it's going to be accepted upstream or how easy this is.

Right now we're accepting only blockers for rhel6, as I believe this is not a release blocker (ie. it doesn't prevent libvirt from talking to qemu) we can move it to rhel6.1.

Bug #617082 is possibly related to this one too.

*** Bug 684110 has been marked as a duplicate of this bug. ***

This won't be fixed for RHEL6 because it's a minor bug, libvirt would have to send broken json to qemu to trigger this.

But this bug exists upstream and will be fixed there in the near future. Will move this BZ to RHEL7.0 so that we can keep track of it.

For the first example

{"execute": "qmp_capabilities
{"execute": "qmp_capabilities"}

the only thing we could do is to disallow a newline inside a quoted string.  Newlines in quoted strings are not valid JSON, but we allowed them so far, so it would be a small backwards incompatibility.

For the second example

{"execute": "qmp_capabilities"
{"execute": "qmp_capabilities"}

there are multiple ways to recover.  You could add both a } or a comma before the second open brace.  Hence, I think this is not recoverable.  The only way would be to add some kind of synchronization sequence, for example a NUL character would force the beginning of a new message.

I've tested this bug and here is how it works now:

1. example:

qmp server is waiting for end of string. If you end string on the new line and also end the object like this:

{"execute": "qmp_capabilities
"}

it correctly emit error response:
{"error": {"class": "CommandNotFound", "desc": "The command qmp_capabilities\r\n has not been found", "data": {"name": "qmp_capabilities\r\n"}}}

2. example:

qmp server is waiting for the end of object or another atribute.

Possible way how to handle this issue could be that we allow command could by only on 1 line or we could end command after we receive 2 new lines.

*** Bug 912219 has been marked as a duplicate of this bug. ***

See bug 912219 for some additional (and sometimes duplicated) examples of what is mentioned in comment 19.

*** This bug has been marked as a duplicate of bug 870311 ***